Re: [PATCH] gc: do not warn about too many loose objects

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 16, 2018, 21:40 UTC
Message-ID: <20180716214003.GH11513@aiede.svl.corp.google.com>
In-Reply-To: <20180716210938.GF25189@sigill.intra.peff.net>

Jeff King wrote:

> On Mon, Jul 16, 2018 at 01:37:53PM -0700, Jonathan Nieder wrote:
>> Jeff King wrote:

Show 13 quoted lines

>>> With the current code, that produces a bunch of annoying warnings for
>>> every commit ("I couldn't gc because the last gc reported a warning").
>>> But after Jonathan's patch, every single commit will do a full gc of the
>>> repository. In this tiny repository that's relatively quick, but in a
>>> real repo it's a ton of CPU and I/O, all for nothing.
>>
>> I see.  Do I understand correctly that if we find a way to print the
>> warning but not error out, that would be good enough for you?
>
> Yes. I thought that's what I proposed originally. ;)
>
> The key thing is that the presence of the warning/log still suppress
> the actual gc for the gc.logExpiry period.

Ah, now I think I see the source of the misunderstanding.

When I initially read the docs, I also assumed that

	If the file gc.log exists, then git gc --auto won’t run unless
	that file is more than gc.logExpiry old.

meant "git gc --auto" would be skipped (and thus silently succeed) when the file is less than gc.logExpiry old. But that assumption was wrong: it errors out!

This makes scripting difficult, since arbitrary commands that incidentally run "git gc --auto" will fail in this state, until gc.log expires (but at that point they'll fail again anyway).

For comparison, in non-daemon mode, there is nothing enforcing the kind of ratelimiting you are talking about. It is an accident of history. If we want this kind of ratelimiting, I'd rather we build it deliberately instead of relying on this accident.

Jonathan