diff mbox series

sstate: show progress bar again

Message ID 20221116112425.2173010-1-enrico.scholz@sigma-chemnitz.de
State Accepted, archived
Commit eacd52216815e0c75e5f257b6db792afd8b1b6df
Headers show
Series sstate: show progress bar again | expand

Commit Message

Enrico Scholz Nov. 16, 2022, 11:24 a.m. UTC
From: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>

Transition to ThreadPoolExecutor (eb6a6820928472ef194b963b606454e731f9486f)
broke the

| Checking sstate mirror object availability: ...

progress bar because the removed 'thread_worker' was still referenced
in an asynchronous function.  As the result of the future is never
read, the resulting backtrace is silently discarded.

Replace the information given to 'ProcessProgress' by a counter.

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
---
 meta/classes-global/sstate.bbclass | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Jose Quaresma Nov. 16, 2022, 3:36 p.m. UTC | #1
Hi Enrico,

Enrico Scholz via lists.openembedded.org <enrico.scholz=
sigma-chemnitz.de@lists.openembedded.org> escreveu no dia quarta,
16/11/2022 à(s) 11:24:

> From: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
>
> Transition to ThreadPoolExecutor (eb6a6820928472ef194b963b606454e731f9486f)
> broke the
>
> | Checking sstate mirror object availability: ...
>
> progress bar because the removed 'thread_worker' was still referenced
> in an asynchronous function.  As the result of the future is never
> read, the resulting backtrace is silently discarded.
>

I have tested the patch at the time and the progress bar works as expected,
maybe something breaks after that.


>
> Replace the information given to 'ProcessProgress' by a counter.
>
> Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
> ---
>  meta/classes-global/sstate.bbclass | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes-global/sstate.bbclass
> b/meta/classes-global/sstate.bbclass
> index 2c8e7b8cc232..3fabca4a3160 100644
> --- a/meta/classes-global/sstate.bbclass
> +++ b/meta/classes-global/sstate.bbclass
> @@ -925,6 +925,8 @@ sstate_unpack_package () {
>  BB_HASHCHECK_FUNCTION = "sstate_checkhashes"
>
>  def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0,
> summary=True, **kwargs):
> +    import itertools
> +
>      found = set()
>      missed = set()
>
> @@ -1019,7 +1021,7 @@ def sstate_checkhashes(sq_data, d, siginfo=False,
> currentcount=0, summary=True,
>              connection_cache_pool.put(connection_cache)
>
>              if progress:
> -                bb.event.fire(bb.event.ProcessProgress(msg, len(tasklist)
> - thread_worker.tasks.qsize()), d)
> +                bb.event.fire(bb.event.ProcessProgress(msg,
> next(num_tasks_done) + 1), d)


>          tasklist = []
>          for tid in missed:
> @@ -1029,6 +1031,8 @@ def sstate_checkhashes(sq_data, d, siginfo=False,
> currentcount=0, summary=True,
>          if tasklist:
>              nproc = min(int(d.getVar("BB_NUMBER_THREADS")), len(tasklist))
>
> +            ## thread-safe counter
> +            num_tasks_done = itertools.count()
>              progress = len(tasklist) >= 100
>              if progress:
>

Can the num_tasks_done be moved to there and call the first
next(num_tasks_done) so we can remove the +1 on the ProcessProgress

Jose


>                  msg = "Checking sstate mirror object availability"
> --
> 2.38.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#173383):
> https://lists.openembedded.org/g/openembedded-core/message/173383
> Mute This Topic: https://lists.openembedded.org/mt/95064126/5052612
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> quaresma.jose@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Enrico Scholz Nov. 17, 2022, 11:46 a.m. UTC | #2
Jose Quaresma <quaresma.jose@gmail.com> writes:

>> Transition to ThreadPoolExecutor (eb6a6820928472ef194b963b606454e731f9486f)
>> broke he
>>
>> | Checking sstate mirror object availability: ...
>>
>> progress bar because the removed 'thread_worker' was still referenced
>> in an asynchronous function.  As the result of the future is never
>> read, the resulting backtrace is silently discarded.
>
> I have tested the patch at the time and the progress bar works as expected,
> maybe something breaks after that.

Probably, you saw only the full 100% bar produced by
'bb.event.ProcessFinished'

But the patch does

| -        def checkstatus(thread_worker, arg):
| +        def checkstatus(arg):
|  ...
|                  bb.event.fire(bb.event.ProcessProgress(msg, len(tasklist) - thread_worker.tasks.qsize()), d)

This will always raise an exception and 'bb.event.ProcessProgress' will
never be fired.


> Can the num_tasks_done be moved to there and call the first
> next(num_tasks_done) so we can remove the +1 on the ProcessProgress

I will send a v2



Enrico
Jose Quaresma Nov. 22, 2022, 10:46 a.m. UTC | #3
Enrico Scholz <enrico.scholz@sigma-chemnitz.de> escreveu no dia quinta,
17/11/2022 à(s) 11:47:

> Jose Quaresma <quaresma.jose@gmail.com> writes:
>
> >> Transition to ThreadPoolExecutor
> (eb6a6820928472ef194b963b606454e731f9486f)
> >> broke he
> >>
> >> | Checking sstate mirror object availability: ...
> >>
> >> progress bar because the removed 'thread_worker' was still referenced
> >> in an asynchronous function.  As the result of the future is never
> >> read, the resulting backtrace is silently discarded.
> >
> > I have tested the patch at the time and the progress bar works as
> expected,
> > maybe something breaks after that.
>
> Probably, you saw only the full 100% bar produced by
> 'bb.event.ProcessFinished'
>
> But the patch does
>
> | -        def checkstatus(thread_worker, arg):
> | +        def checkstatus(arg):
> |  ...
> |                  bb.event.fire(bb.event.ProcessProgress(msg,
> len(tasklist) - thread_worker.tasks.qsize()), d)
>
> This will always raise an exception and 'bb.event.ProcessProgress' will
> never be fired.
>

I got where the problem came from, this patch serie has been tested on the
autobuilder for quite some time
and had several interactions so in one of them the patch [1] has been
dropped.
[1] https://lists.openembedded.org/g/openembedded-core/message/164564

Anyway your solution is much simpler than mine.

Best regards,
Jose


>
> > Can the num_tasks_done be moved to there and call the first
> > next(num_tasks_done) so we can remove the +1 on the ProcessProgress
>
> I will send a v2
>
>
>
> Enrico
>
diff mbox series

Patch

diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass
index 2c8e7b8cc232..3fabca4a3160 100644
--- a/meta/classes-global/sstate.bbclass
+++ b/meta/classes-global/sstate.bbclass
@@ -925,6 +925,8 @@  sstate_unpack_package () {
 BB_HASHCHECK_FUNCTION = "sstate_checkhashes"
 
 def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True, **kwargs):
+    import itertools
+
     found = set()
     missed = set()
 
@@ -1019,7 +1021,7 @@  def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
             connection_cache_pool.put(connection_cache)
 
             if progress:
-                bb.event.fire(bb.event.ProcessProgress(msg, len(tasklist) - thread_worker.tasks.qsize()), d)
+                bb.event.fire(bb.event.ProcessProgress(msg, next(num_tasks_done) + 1), d)
 
         tasklist = []
         for tid in missed:
@@ -1029,6 +1031,8 @@  def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
         if tasklist:
             nproc = min(int(d.getVar("BB_NUMBER_THREADS")), len(tasklist))
 
+            ## thread-safe counter
+            num_tasks_done = itertools.count()
             progress = len(tasklist) >= 100
             if progress:
                 msg = "Checking sstate mirror object availability"