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

----- Original Message ----- > 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 Agreed, I will rename it. >> >> 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). > Yes but I am not sure this should happen right there. All the other service installs it's own jobs, so maybe SessionDataContainer should do so as well. It would look more consistent. >> 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 >