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

["René Scharfe" <l.s.r@web.de>]

Am 09.06.21 um 00:12 schrieb Ævar Arnfjörð Bjarmason:
>
> On Tue, Jun 08 2021, René Scharfe wrote:
>
>> 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é