[Engine-devel] Weird behavior of multiple SetVmTicket query
Vojtech Szocs
vszocs at redhat.com
Tue Nov 19 11:22:22 UTC 2013
----- 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: awels at redhat.com, "engine-devel" <engine-devel at ovirt.org>, "Vojtech Szocs" <vszocs at redhat.com>
> Sent: Monday, November 18, 2013 10: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)?
+1
(or in other words, the purpose behind MultipleActionsRunner line 91 -> ThreadPoolUtil.execute(...RunCommands...) -> actions invoked in separate thread and MultipleActionsRunner.Execute immediately returns)
>
> ----
> 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
> >
>
More information about the Engine-devel
mailing list