Re: [Engine-devel] Serial Execution of Async Tasks

From: "Michael Kublin" <mkublin(a)redhat.com&gt; To: "Allon Mureinik" <amureini(a)redhat.com&gt; Cc: "Eduardo Warszawski" <ewarszaw(a)redhat.com&gt;, "Liron Aravot" <laravot(a)redhat.com&gt;, "Maor Lipchuk" <mlipchuk(a)redhat.com&gt;, "engine-devel" <engine-devel(a)ovirt.org&gt; 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?