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

From: "Yair Zaslavsky" <yzaslavs(a)redhat.com&gt; To: engine-devel(a)ovirt.org Sent: Tuesday, February 14, 2012 2:01:41 PM Subject: Re: [Engine-devel] [backend] a little confusion about the quartz jobs 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 _______________________________________________ Engine-devel mailing list Engine-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel