From: "Ayal Baron" <abaron(a)redhat.com>
To: "Allon Mureinik" <amureini(a)redhat.com>, "Michael Kublin"
<mkublin(a)redhat.com>
Cc: "Liron Aravot" <laravot(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>, "Eduardo Warszawski"
<ewarszaw(a)redhat.com>
Sent: Sunday, September 23, 2012 1:10:07 PM
Subject: Re: [Engine-devel] Serial Execution of Async Tasks
----- Original Message -----
>
>
> ----- Original Message -----
> > From: "Michael Kublin" <mkublin(a)redhat.com>
> > To: "Allon Mureinik" <amureini(a)redhat.com>
> > Cc: "Eduardo Warszawski" <ewarszaw(a)redhat.com>, "Liron
Aravot"
> > <laravot(a)redhat.com>, "Maor Lipchuk"
> > <mlipchuk(a)redhat.com>, "engine-devel"
<engine-devel(a)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_Task...
> > >>>>>>>> -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?
What is the private case here exactly?
Every task can have multiple jobs. We've identified several such
places (e.g. live storage migration, move disk, move vm) and I have
no doubt more will popup.
As Allon notes below, task handling is done at CommandBase, if you
think task management should be for storage only, you're welcome to
push it down to StorageHandlingCommandBase (or get rid of
inheritance here altogether).
Interesting , regards cyclic reference no response,
but who cares,
it is difficult to answer , that's why better not to response?
Regards private case:
1. We have command that not creating any task
2. We have command that will create a one task.
3. And we have 3 commands meanwhile which will create more than one task.
I think that 3 is private case and not common? (In the future, I removed too many
lines of code that were preparation for future that never come)
The handling done at CommandBase it means that it is influence all system.
Now regards architecture why I need some handler which will be inside a command
and will call for command methods? Please explain.
> 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.
> >
> >
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
>
http://lists.ovirt.org/mailman/listinfo/engine-devel
>