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

[Laszlo Hornyak]

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