Re: [PATCH 2/2] read-cache: fix incorrect count and progress bar stalling

Am 09.06.21 um 00:12 schrieb Ævar Arnfjörð Bjarmason:

On Tue, Jun 08 2021, René Scharfe wrote:

quoted

I wonder (only in a semi-curious way, though) if we can detect
off-by-one errors by adding an assertion to display_progress() that
requires the first update to have the value 0, and in stop_progress()
one that requires the previous display_progress() call to have a value
equal to the total number of work items.  Not sure it'd be worth the
hassle..

That's intentional. We started eating 3 apples, got to one, but now our
house is on fire and we're eating no more apples today, even if we
planned to eat 3 when we sat down.

The progress bar reflects this unexpected but recoverable state:

    $ perl -wE 'for (0..1) { say "update"; say "progress $_" }' |
      ./helper/test-tool progress --total=3 Apples 2>&1 |
      cat -v | perl -pe 's/\^M\K/\n/g'
    Apples:   0% (0/3)^M
    Apples:  33% (1/3)^M
    Apples:  33% (1/3), done.

We're at 1/3, but we're done. No more apples.

This isn't just some hypothetical, e.g. consider neeing to unlink() or
remove files/directories one at a time in a directory and getting the
estimated number from st_nlink (yeah yeah, unportable, but it was the
first thing I thought of).

We might think we're processing 10 entries, but another other processes
might make our progress bar end at more or less than the 100% we
expected. That's OK, not something we should invoke BUG() about.

It doesn't have to be a BUG; a warning would suffice.  And I hope not
finishing the expected number of items due to a catastrophic event is
rare enough that an additional warning wouldn't cause too much pain.

Loops that *regularly* end early are not a good fit for progress
percentages, I think.

Similarly, the n=0 being distinguishable from the first
display_progress() is actually useful in practice. It's something I've
seen git.git emit (not recently, I patched the relevant code to emit
more granular progress).

It's useful to know that we're stalling on the setup code before the
for-loop, not on the first item.

Hmm, preparations that take a noticeable time might deserve their own
progress line.

Anyway, if no guard rails can be built then we have to rely on our math
skills alone.  Off-by-one errors may look silly, but are no joke -- they
are surprisingly easy to make.

René