On Thu, Jul 21, 2022 at 06:47:10PM +0200, Florian Obser wrote:
> On 2022-07-21 18:19 +02, Stefan Sperling <stsp@stsp.name> wrote:
> > On Thu, Jul 21, 2022 at 05:46:59PM +0200, Florian Obser wrote:
> >> I can't quite figure out if there is a case where we are not updating
> >> progress even with the fix. Maybe we can just drop bytes_sent_cur and
> >> always update, that's what the code is currently doing anyway.
> >
> > Your fix is fine. ok stsp@
> >
> > Progress looks the same as before because libexec/got-send-pack uses
> > rate-limiting for the progress reports it sends via imsg. These reports
> > are spaced 500 msec apart. In your testing some amount of bytes will
> > always have been sent when another 500 msec have elapsed.
> 
> But is something ticking every 500ms? I can't find that.

There is no timer. The ratelimit struct contains a timestamp from
the previous report. Via got_ratelimit_check() got-send-pack checks
whether it should pass another report through, and the result of this
check will be "yes" if at least 500ms have elapsed since the last check.

> Isn't send_pack_file() in got-send-pack.c waiting however long the
> write() takes? wtotal will always have increased when we call
> send_upload_progress().

What I measured was the total size of progress output after redirecting
it to a file. If you disable the rate-limiting you should see more progress
data generated by got-send-pack than before. Of course, when I did this
I did not have your fix, so my output could have been even larger than it
will be with your fix in place and rate-limiting disabled.

In any case, with rate-limiting in place your fix shouldn't make a big
difference, if at all. But your fix is correct regardless.