Re: [OE-core] [PATCH 4/5] sstate.bbclass: disable thread lock if we don't have events

Jose Quaresma
2021-08-18T15:51:18Z

First of all, sorry for the inconvenience of this patch that adds many troubles.

I have concluded from the commit history that the lock is introduced
because of the progress bar and I don't know that the debug message
uses the events when it runs inside threads.

Basically I am wrong in my assumption and this patch doesn't make sense,
so I will remove it from the series and send the others that are minor tweaks
and little improvements in the sstate mirror.

Best regards,
José Quaresma

Richard Purdie <richard.purdie@linuxfoundation.org> escreveu no dia
quarta, 11/08/2021 à(s) 20:40:
>
> On Mon, 2021-08-09 at 10:48 +0100, Jose Quaresma wrote:
> > commit f2053844958325496a9387874a8f3182400b71ca
> > 'classes/sstate.bbclass: Enable thread lock when checkstatus'
> > adds a thread lock to don't lose the events from multiple threads
> > that runs on the ThreadPool.
> >
> > commit 1444b8a2ae226829e719d3d184fca27e5940ae0d
> > 'sstate.bbclass: Only show sstate mirror progress bar for >= 100 objects'
> > disable the events if we don't have a minium number of objects.
> >
> > So we can only use the thread lock when we have the events in place.
> >
> > Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
> > ---
> > meta/classes/sstate.bbclass | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> > index 63085a7f3a..948779386d 100644
> > --- a/meta/classes/sstate.bbclass
> > +++ b/meta/classes/sstate.bbclass
> > @@ -969,17 +969,17 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
> > if len(tasklist) >= min_tasks:
> > msg = "Checking sstate mirror object availability"
> > bb.event.fire(bb.event.ProcessStarted(msg, len(tasklist)), d)
> > + bb.event.enable_threadlock()
> >
> >
> >
> >
> > - bb.event.enable_threadlock()
> > pool = oe.utils.ThreadedPool(nproc, len(tasklist),
> > worker_init=checkstatus_init, worker_end=checkstatus_end)
> > for t in tasklist:
> > pool.add_task(checkstatus, t)
> > pool.start()
> > pool.wait_completion()
> > - bb.event.disable_threadlock()
> >
> >
> >
> >
> > if len(tasklist) >= min_tasks:
> > + bb.event.disable_threadlock()
> > bb.event.fire(bb.event.ProcessFinished(msg), d)
> >
> >
>
> This patch series somehow breaks the build in weird ways, typically:
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/59/builds/3789
>
> i.e. the sdk-update from an eSDK somehow fails to get sstate objects from the
> network. Michael and I spent hours trying to figure out what was breaking
> things and whether this was an infrastructure issue with sstate or not :(.
>
> I dropped this series and builds look much happier. I think this patch
> isn't correct as the threadlock isn't guarding against just progress
> events, it guards against any events from threads. There are debug messages
> in the code which turn into events too. As such, when threads are in use,
> the lock is needed.
>
> I therefore suspect it is this change that breaks things in weird ways
> but I haven't confirmed that.
>
> Cheers,
>
> Richard
>

--
Best regards,

José Quaresma