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(a)ovirt.org
>
http://lists.ovirt.org/mailman/listinfo/engine-devel
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel