[Engine-devel] [backend] a little confusion about the quartz jobs

[Yair Zaslavsky]

On 02/14/2012 02:21 PM, Mike Kolesnik wrote:
>> hi,
>>
>> I was playing with the quartz jobs in the backend and I thought this
>> is an area where some simplification and/or cleanup would be useful.
>>
>>  - SchedulerUtil interface would be nice to hide quartz from the rest
>>  of the code, but it very rarely used, the clients are bound to it's
>>  single implementation, SchedulerUtilQuartzImpl through it's
>>  getInstance() method.
> 
> I think the whole class name is misleading, since usually when I imagine a utils class, it's a simple class that does some menial work for me in static methods, and not really calls anything else or even has an instance.
+1
> 
> Maybe the class can be renamed to just Scheduler, or ScheduleManager which will be more precise.
> 
>>  - It was designed to be a local EJB, Backend actually expects it to
>>  be injected. (this field is not used)
>>  - when scheduling a job, you call schedule...Job(Object instance,
>>  String methodName, ...) however, it is not the _methodname_ that
>>  the executor will look for
>>  - instead, it will check the OnTimerMethodAnnotation on all the
>>  methods. But this annotation has everywhere the methodName as value
>>  - JobWrapper actually iterates over all the methods to find the one
>>  with the right annotation
>>
>> So a quick simplification could be:
>>  - The annotation is not needed, it could be removed
>>  - JobWrapper could just getMethod(methodName, argClasses) instead of
>>  looking for the annotation in all of the methods
> 
> Sounds good, or maybe just keep the annotation and not the method name in the call/annotation since then if the method name changes it won't break and we can easily locate all jobs by searching for the annotation..
> 
>>  - I am really not for factoryes, but if we want to separate the
>>  interface from the implementation, then probably a
>>  SchedulerUtilFactory could help here. The dummy implementation
>>  would do just the very same thing as the
>>  SchedulerUtilQuartzImpl.getInstance()
>>  - I would remove the reference to SchedulerUtil from Backend as
>>  well, since it is not used. Really _should_ the Backend class do
>>  any scheduling?
> 
> Backend does schedule at least one job in it's Initialize() method..
Yes, we have the DbUsers cache manager that performs periodic checks for
db users against AD/IPA.
This scheduler should start upon @PostConstruct (or any logical equivalent).

> Maybe the class should be injected, but I don't know if that happens so maybe that's why it's unused.
> 
>>
>> Please share your thoughts.
>>
>> Thank you,
>> Laszlo
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel