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

From: Jeff King <peff@peff.net>
Date: Jul 16, 2018, 21:45 UTC
Message-ID: <20180716214539.GL25189@sigill.intra.peff.net>
In-Reply-To: <20180716214003.GH11513@aiede.svl.corp.google.com>

On Mon, Jul 16, 2018 at 02:40:03PM -0700, Jonathan Nieder wrote:

Show 13 quoted lines

> > 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!

Right. That's the mysterious "-1" error code, and I agree that part is confusing.

> 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).

I don't think any command should report failure of its _own_ operation if "gc --auto" failed. And grepping around the source code shows that we typically ignore it.

> 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.

What I was trying to say earlier is that we _did_ build this rate-limiting, and I think it is a bug that the non-daemon case does not rate-limit (but nobody noticed, because the default is daemonizing).

So the fix is not "rip out the rate-limiting in daemon mode", but rather "extend it to the non-daemon case".

-Peff