----- Original Message ----- > From: "Yair Zaslavsky" <yzasl...@redhat.com> > To: engine-devel@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@ovirt.org > >> http://lists.ovirt.org/mailman/listinfo/engine-devel > >> > > _______________________________________________ > > Engine-devel mailing list > > Engine-devel@ovirt.org > > http://lists.ovirt.org/mailman/listinfo/engine-devel > > _______________________________________________ > Engine-devel mailing list > Engine-devel@ovirt.org > http://lists.ovirt.org/mailman/listinfo/engine-devel > _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel