Re: [Patch] Prevent cloning over http from spewing

From: Tay Ray Chuan <rctay89@gmail.com>
Date: Jun 22, 2009, 12:10 UTC
Message-ID: <be6fef0d0906220510r416d66aaoc2171bdcc61ec3a4@mail.gmail.com>
In-Reply-To: <20090611111141.GB4409@coredump.intra.peff.net>

Hi,

note: this time, I haven't anything to show for in my repo; sorry.

On Thu, Jun 11, 2009 at 7:11 PM, Jeff King<peff@peff.net> wrote:

Show 15 quoted lines

> On Wed, Jun 10, 2009 at 10:03:10PM +0800, Tay Ray Chuan wrote:
>
>> > My first complaint is that it is way too long. It wrapped in my
>> > 80-column terminal, causing all sorts of visual confusion.
>>
>> The byte counts can really take up alot of space. Perhaps we should
>> just show the size (MiB) and completed percentage, sans byte counts?
>
> I think that makes sense. Especially because the (X/Y) in git's progress
> output usually refers to the number of _objects_, and there is nothing
> in the output to indicate that it is actually a byte count here.
>
> I think it will need some tweaking of the progress code to show the
> percentage but not the actual byte counts, but it should be a relatively
> simple change.

Ok.

Show 10 quoted lines

>> Fetching of objects and packs take place separately; it doesn't mean
>> that when '0' objects are being fetched, we're definitely fetching
>> something else (eg. packs). Perhaps we should "hide" the "Fetching 0
>> objects" part when the number of simultaneous object fetches is 0?
>
> Yes, though I really wonder if the "fetching" number is all that useful
> even when it is not zero. The _most_ important thing is to show the user
> that something is happening, and we are waiting on the network. And I
> think we largely show that through the "total bytes sent" and throughput
> counters.

Ok also.

My understanding of this point now: if we're fetching non-objects *(like packs, pack indices), we wouldn't display "Fetching X objects" at all, regardless of X. But if we're not fetching any non-objects and we're waiting for loose objects fetching to finish (ie. we're fetching 1 loose object or more), then we would display "Fetching X objects".

Show 10 quoted lines

>> The total number of objects (320) increases as we "walk" the commits;
>> sometimes we need to fetch the "walked" objects, sometimes we don't
>> (eg. in packs we've fetched already). There's no way to know in
>> advance the total; hence, the continually updating of the total. I
>> don't think there's it's a problem; the idea is to let the user be
>> sure that git is active.
>
> Right. But I think we are better off showing simple increasing numbers
> (like bytes or objects transferred) than misleading or inaccurate
> guesses of totals. The latter creates more frustration, I think.

Ok.

Show 5 quoted lines

>> Yeah, we do download packs one at a time (as I said above).
>
> But from what you wrote elsewhere in the message, it sounds like we may
> be downloading a pack _and_ a loose object at the same time. So my
> suggestion doesn't quite work in that case.

In addition to that, it isn't possible to calculate the number of pack indices to be fetched, because we're fetching them as we go along (see http.c::http_get_info_packs() in 'master'), so that's a potential problem. I wonder if you (or anyone reading this) has any suggestions on this?

I still think we can use your idea of one-fetch-one-verify per pack, even though objects are fetched simultaneously.

Here's what a "git clone http://repo" would look like, after incorporating points from our discussion so far:

 Fetching info/refs: Z%, x MiB | y KiB/s, done
 Fetching objects/info/packs: Z%, x MiB | y KiB/s, done
 Fetching pack index (1 of 3): Z%, x MiB | y KiB/s, done
 Fetching pack index (2 of 3): Z%, x MiB | y KiB/s, done
 Fetching pack index (3 of 3): Z%, x MiB | y KiB/s, done
 Fetching pack (1 of 3): Z%, x MiB | y KiB/s, done
 Verifying pack (1 of 3): Z%, (X/Y), done
 Fetching pack (2 of 3): Z%, x MiB | y KiB/s, done
 Verifying pack (2 of 3): Z%, (X/Y), done
 Fetching pack (3 of 3): Z%, x MiB | y KiB/s, done
 Verifying pack (3 of 3): Z%, (X/Y), done
 Fetching 2 objects, done.
 Checking out files, done.

(I made up the last line.)

>> the linux-2.6 repo has only 1 pack (and no http-alternates), so this
>> is weird.
>
> Maybe we are fetching from different places:

Thanks. I realise the weird percentage values you saw were due to out-of-range numeric calculations, so actually, your point didn't count, but thanks anyway.

-- 
Cheers,
Ray Chuan