[Engine-devel] Weird behavior of multiple SetVmTicket query
Vojtech Szocs
vszocs at redhat.com
Thu Nov 28 17:10:21 UTC 2013
----- Original Message -----
> From: "Alexander Wels" <awels at redhat.com>
> To: "Itamar Heim" <iheim at redhat.com>
> Cc: "Oved Ourfalli" <ovedo at redhat.com>, engine-devel at ovirt.org
> Sent: Thursday, November 21, 2013 3:15:40 PM
> Subject: Re: [Engine-devel] Weird behavior of multiple SetVmTicket query
>
> On Thursday, November 21, 2013 04:01:34 PM Itamar Heim wrote:
> > On 11/21/2013 03:28 PM, Alexander Wels wrote:
> > > On Thursday, November 21, 2013 07:18:42 AM Daniel Erez wrote:
> > >> ----- Original Message -----
> > >>
> > >>> From: "Einav Cohen" <ecohen at redhat.com>
> > >>> To: "Livnat Peer" <lpeer at redhat.com>, "Eli Mesika"
> > >>> <emesika at redhat.com>,
> > >>> "Omer Frenkel" <ofrenkel at redhat.com>, "Doron Fediuck"
> > >>> <dfediuck at redhat.com>, "Oved Ourfalli" <ovedo at redhat.com> Cc:
> > >>> "engine-devel" <engine-devel at ovirt.org>
> > >>> Sent: Monday, November 18, 2013 11:19:23 PM
> > >>> Subject: Re: [Engine-devel] Weird behavior of multiple SetVmTicket
> > >>> query
> > >>>
> > >>> engine-core maintainers - this one is mainly for you - see below.
> > >>> the most important question (I think) is: Is there any reason
> > >>> to not introduced a "sync" flavor of RunMultipleActions (i.e.
> > >>> one that will return from the backend only when all actions have
> > >>> been completed, similarly to RunAction)?
> > >>
> > >> There's a couple of years old patch exactly for that [1].
> > >> It should allow both synchronous and asynchronous multiple actions
> > >> according to a flag. Question is when do we want to use the synchronous
> > >> flavor? The reasoning behind the current implementation is to return a
> > >> quick response to the client, as executing multiple vdsm commands can
> > >> potentially require a long wait. Do we need to introduce a sync version
> > >> just for VM console connection flow? IIRC, the flow actually uses
> > >> multiple calls to the singular form (RunAction) rather than
> > >> RunMultipleAction?
> > >>
> > >> [1] http://gerrit.ovirt.org/#/c/3210/
> > >
> > > We recently introduced a patch [1] that on the front-end is smart enough
> > > to
> > > consolidate multiple query/actions into a single
> > > runMultiple(Query/Action)
> > > call to reduce the number http requests to the back-end. In the
> > > SetVMTicket
> > > case it combined two actions into one runMultipleAction call and was
> > > expected that to return once all the actions where complete, which didn't
> > > happen causing some undesired behavior on the client side. This sparked
> > > the investigation that ended in this email.
> > >
> > > So yes I would definitely like a 'synchronous' version of
> > > runMultipleAction so the frontend code can set that flag when combining
> > > multiple actions into a single call. We can only safely do this if we are
> > > combining several runActions as all of those are 'synchronous'.
> > >
> > > [1] http://gerrit.ovirt.org/#/c/17356/
> >
> > btw, how is this going to work with the REST API?
> >
>
> I believe that is one of the outstanding issues with regards to the REST API
> transition. Before this optimization we already had code calling
> runMultiple(Query/Action) and we will have to figure something out for the
> REST
> API there as well. I think at the point we solve that issue we can decide if
> we want to keep the optimization or not.
Yes, UI code currently reflects (and uses) what internal Backend API offers. This is why I mentioned we will eventually have to move away from query/action concept (RPC-method-centric) to REST resource/operation concept (resource-centric).
GenericApiGWTService server-side implementation (aka GWT RPC endpoint) more or less delegates to BackendLocal which exposes methods such as RunMultipleActions by itself. To emulate RunMultipleActions when using REST API, assuming there is no such support provided by REST backend, we'll have to do that via oVirt.js library.
>
> > >>> ----
> > >>> Thanks,
> > >>> Einav
> > >>>
> > >>> ----- Original Message -----
> > >>>
> > >>>> From: "Vojtech Szocs" <vszocs at redhat.com>
> > >>>> To: awels at redhat.com
> > >>>> Cc: "engine-devel" <engine-devel at ovirt.org>
> > >>>> Sent: Tuesday, November 12, 2013 6:40:18 AM
> > >>>> Subject: Re: [Engine-devel] Weird behavior of multiple SetVmTicket
> > >>>> query
> > >>>>
> > >>>> Forwarding this email to engine-devel so that backend maintainers are
> > >>>> aware
> > >>>> of this issue.
> > >>>>
> > >>>> Looking at the code:
> > >>>>
> > >>>> - MultipleActionsRunner#Execute first creates and "validates" all
> > >
> > > commands:
> > >>>> A. the "for" block which iterates over getParameters()
> > >>>>
> > >>>> 1-> validate correlation ID, if OK create and add command,
> > >>>> otherwise
> > >>>> add
> > >>>> returnValue
> > >>>>
> > >>>> B the "if" block which tests getCommands().size()
> > >>>>
> > >>>> 1-> if single command to execute, add its "canDoActionOnly"
> > >>>> returnValue,
> > >>>> which is returnValue with canDoAction but without actual result
> > >>>> object
> > >>>> 2-> if multi commands to execute, execute chunks of max. 10
> > >>>> threads
> > >>>> (sequentially, ThreadPoolUtil.invokeAll returns after all
> > >>>> threads
> > >>>> complete), same returnValue as above
> > >>>>
> > >>>> C. the "if" block which tests canRunActions
> > >>>>
> > >>>> 1-> all commands are executed within SINGLE THREAD due to
> > >>>> ThreadPoolUtil.execute(Runnable), which is kind of weird
> > >>>> comapred
> > >>>> to
> > >>>> how returnValues are prepared (see B2)
> > >>>> 2-> when executing command, code DOES NOT CARE about its
> > >>>> returnValue,
> > >>>> i.e. returnValue was already prepared (see B) and command
> > >>>> execution
> > >>>> should just update it
> > >>>>
> > >>>> The problem (I think) is that C1 starts a different thread (to execute
> > >>>> all
> > >>>> commands) and immediately returns, i.e. code doesn't wait for thread
> > >>>> to
> > >>>> complete. This is why returnValues are observed on frontend as
> > >>>> inconsistent.
> > >>>>
> > >>>> Additionally, we're also mixing of two different things: canDoAction
> > >>>> processing and returnValues processing. IMHO this should be refactored
> > >>>> to
> > >>>> make code easier to read.
> > >>>>
> > >>>> Changes done by Alex (patch attached):
> > >>>>
> > >>>> X1. returnValues changed to Collections.synchronizedList
> > >>>>
> > >>>> -> this means all access to returnValues is now serial
> > >>>> -> iteration over synchronizedList should also be enclosed in
> > >>>>
> > >>>> "synchronized (list)" block, so this:
> > >>>> for (VdcReturnValueBase value : returnValues) ...
> > >>>>
> > >>>> should be this:
> > >>>> synchronized (returnValues) { for (VdcReturnValueBase
> > >>>> value
> > >>>> :
> > >>>> returnValues) ... }
> > >>>>
> > >>>> X2. commented-out original command execution via
> > >>>> ThreadPoolUtil.execute(Runnable)
> > >>>>
> > >>>> -> new RunCommands method invokes all commands each in separate
> > >>>> thread
> > >>>> via ThreadPoolUtil.invokeAll
> > >>>> -> returnValues list is explicitly updated
> > >>>>
> > >>>> Guys, what do you think?
> > >>>>
> > >>>> Vojtech
> > >>>>
> > >>>>
> > >>>> ----- Original Message -----
> > >>>>
> > >>>>> From: "Alexander Wels" <awels at redhat.com>
> > >>>>> To: "Frantisek Kobzik" <fkobzik at redhat.com>
> > >>>>> Cc: "Vojtech Szocs" <vszocs at redhat.com>
> > >>>>> Sent: Monday, November 11, 2013 9:19:08 PM
> > >>>>> Subject: Re: Weird behavior of multiple SetVmTicket query
> > >>>>>
> > >>>>> Hi,
> > >>>>>
> > >>>>> I did some debugging, and the problem is a race condition in the
> > >>>>> MultipleActions class. The whole class seems to have a lot of
> > >>>>> multi-threading
> > >>>>> issues but if I modify the code to wait for the results of the
> > >>>>> actions
> > >>>>> to
> > >>>>> return before returning the return value, everything works fine.
> > >>>>>
> > >>>>> I am attaching a patch that solves the issue at hand, but should not
> > >>>>> be
> > >>>>> considered a real patch. It is just to show the issue is in the
> > >>>>> back-end
> > >>>>> not
> > >>>>> the front-end code. The front-end code is just exposing an issue in
> > >>>>> the
> > >>>>> back-
> > >>>>> end
> > >>>>>
> > >>>>> Alexander
> > >>>>>
> > >>>>> On Monday, November 11, 2013 09:53:22 AM you wrote:
> > >>>>>> Ok, thank you very much!
> > >>>>>>
> > >>>>>> ----- Original Message -----
> > >>>>>> From: "Alexander Wels" <awels at redhat.com>
> > >>>>>> To: "Frantisek Kobzik" <fkobzik at redhat.com>
> > >>>>>> Sent: Monday, November 11, 2013 2:15:43 PM
> > >>>>>> Subject: Re: Weird behavior of multiple SetVmTicket query
> > >>>>>>
> > >>>>>> Frantisek,
> > >>>>>>
> > >>>>>> I had seen this before, let me test and fix it for you, it is very
> > >>>>>> likely
> > >>>>>> my
> > >>>>>> patch broke that.
> > >>>>>>
> > >>>>>> Alexander
> > >>>>>>
> > >>>>>> On Monday, November 11, 2013 03:43:19 AM you wrote:
> > >>>>>>> Hi Alex,
> > >>>>>>>
> > >>>>>>> recently I noticed problems with invoking console for multiple VMs
> > >>>>>>> (select
> > >>>>>>> more VMs in webadmin and then hit the console btn). I was sure it
> > >>>>>>> worked
> > >>>>>>> in
> > >>>>>>> past so i git-bisected the master branch and I discovered that
> > >>>>>>> this
> > >>>>>>> problem
> > >>>>>>> is apparently caused by patch [1]. For single vm console
> > >>>>>>> invocation
> > >>>>>>> it
> > >>>>>>> works fine, but for multiple VMs it doesn't.
> > >>>>>>>
> > >>>>>>> I did some closer investigation with a debugger and it seems that
> > >>>>>>> getSucceeded() on the return val of SetVmTicket command returns
> > >>>>>>> false
> > >>>>>>> in
> > >>>>>>> case of multiple execution despite the fact SetVmTicketCommand
> > >>>>>>> sets
> > >>>>>>> this
> > >>>>>>> value to true. I suppose there is some problem with propagation of
> > >>>>>>> command
> > >>>>>>> return value from BE to FE. The same goes for the encapsulated
> > >>>>>>> returnValue
> > >>>>>>> attribute (it contains value of the generated ticket). When
> > >>>>>>> invoking
> > >>>>>>> multiple consoles it is null (although it has been filled on
> > >>>>>>> backend).
> > >>>>>>> Weird.
> > >>>>>>>
> > >>>>>>> Tomas told me you did some optimizations for multiple command
> > >>>>>>> executions,
> > >>>>>>> do you know it might cause it? Please let me know if you have any
> > >>>>>>> idea...
> > >>>>>>>
> > >>>>>>> Cheers,
> > >>>>>>> F.
> > >>>>>>>
> > >>>>>>> [1]: http://gerrit.ovirt.org/#/c/17356/
> > >>>>
> > >>>> _______________________________________________
> > >>>> 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
> > >
> > > _______________________________________________
> > > 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
>
More information about the Engine-devel
mailing list