[Engine-devel] Serial Execution of Async Tasks

[Allon Mureinik]

----- Original Message -----
> From: "Michael Kublin" <mkublin at redhat.com>
> To: "Allon Mureinik" <amureini at redhat.com>
> Cc: "Eduardo Warszawski" <ewarszaw at redhat.com>, "Liron Aravot" <laravot at redhat.com>, "Maor Lipchuk"
> <mlipchuk at redhat.com>, "engine-devel" <engine-devel at ovirt.org>
> Sent: Sunday, September 23, 2012 12:41:05 PM
> Subject: Re: [Engine-devel] Serial Execution of Async Tasks
> 
> 
> >>>>>>>>>> Hi guys,
> >>>>>>>>>>
> >>>>>>>>>> As you may know the engine currently has the ability to
> >>>>>>>>>> fire
> >>>>>>>>>> an
> >>>>>>>>>> SPM
> >>>>>>>>>> task, and be asynchronously be "woken-up" when it ends.
> >>>>>>>>>> This is great, but we found the for the Live Storage
> >>>>>>>>>> Migration
> >>>>>>>>>> feature we need something a bit complex - the ability to
> >>>>>>>>>> have a
> >>>>>>>>>> series of async tasks in a single control flow.
> >>>>>>>>>>
> >>>>>>>>>> Here's my initial design for this, your comments and
> >>>>>>>>>> criticism
> >>>>>>>>>> would
> >>>>>>>>>> be welcome:
> >>>>>>>>>> http://wiki.ovirt.org/wiki/Features/Serial_Execution_of_Asynchronous_Tasks_Detailed_Design
> >>>>>>>> -successful execution -
> >>>>>>>> * "CommandBase iterates over its SPMAsyncTaskHandlers" -
> >>>>>>>> when?
> >>>>>>> This is the new suggested format of executeCommand(). I'll
> >>>>>>> clarify
> >>>>>>> this too.
> >>>>>>>
> >>>>>>>> * If the second task is an HSM command (vs. SPM command), I
> >>>>>>>> think you
> >>>>>>>> should explain in the design how to handle such flows as
> >>>>>>>> well.
> >>>>>>> HSM commands do not create AsyncTasks, as they do today - I
> >>>>>>> will
> >>>>>>> clarify this.
> >>>>>>>
> >>>>>>>> * Why do we need before task? can you give a concrete
> >>>>>>>> example
> >>>>>>>> of what
> >>>>>>>> would you do in such a method.
> >>>>>>> Basically, /today/, command look like this:
> >>>>>>> executeCommand() {
> >>>>>>>     doStuffInTheDB();
> >>>>>>>     runVdsCommand(someCommand);
> >>>>>>> }
> >>>>>>>
> >>>>>>> endSuccessfully() {
> >>>>>>>     doMoreStuffInTheDB();
> >>>>>>> }
> >>>>>>>
> >>>>>>> endWithFailure() {
> >>>>>>>     doMoreStuffForFailureInTheDB();
> >>>>>>> }
> >>>>>>>
> >>>>>>> In the new design, the entire doStuffInTheDB() should be
> >>>>>>> moved
> >>>>>>> to a
> >>>>>>> breforeTask of the (only) SPMAsyncTaskHandler.
> >>>>>>>
> >>>>>>>>
> >>>>>>>> - I see you added SPMAsyncTaskHandler, any reason not to use
> >>>>>>>> SPMAsyncTasK to manage it own life-cycle?
> >>>>>>> Conserving today's design - The SPMAsyncTaskHandler is the
> >>>>>>> place to
> >>>>>>> add additional, non-SPM, logic around the SPM task execution,
> >>>>>>> like
> >>>>>>> CommandBase allows today.
> >>>>>>>
> >>>>>>>>
> >>>>>>>> - In the life-cycle managed by the SPMAsyncTaskHandler there
> >>>>>>>> is a
> >>>>>>>> step
> >>>>>>>> 'createTask - how to create the async task' can you please
> >>>>>>>> elaborate
> >>>>>>>> what are the options.
> >>>>>>> new [any type of async task]
> 
> (I cleaned thread a little.)
> The following design and it is implementation
> http://gerrit.ovirt.org/#/c/7956/
> is bad.
> I don't see any reason for creating a new SPMAsyncTaskHandler and
> especially in the
> way as it's done in patch.
> The reason are following:
> 1. Performance , increased memory footprint, created CYCLIC
> REFERENCE.
> 2. Readability and robust of code: the code which is written as
> cyclic references is unreadable
>    and difficult for debug.
> 3. Why I need a generic implementation and changes all over whole
> project because of
>    series of async commands, for me it is a private case?
This will occur all over the storage commands (which are the only usages of tasks nowadys).
Moreover, async task handling is done at the Commandbase level (see the end* methods) - instead of hacking it in X different places whenever we need it, I'd prefer doing it once, properly.
> 
>