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

From: Jeff King <peff@peff.net>
Date: Jul 16, 2018, 21:21 UTC
Message-ID: <20180716212159.GH25189@sigill.intra.peff.net>
In-Reply-To: <CABPp-BEpCF9FE7eJwZWjY+bMsjDQnnDaSrHO+e3DtDDsR-=7Hg@mail.gmail.com>

On Mon, Jul 16, 2018 at 02:09:43PM -0700, Elijah Newren wrote:

Show 19 quoted lines

> >> Um, except it doesn't actually do that.  The testcase I provided shows
> >> that it leaves around 10000 objects that are totally deletable and
> >> were only previously referenced by reflog entries -- entries that gc
> >> removed without removing the corresponding objects.
> >
> > What's your definition of "totally deletable"?
> 
> The point of gc is to: expire reflogs, repack referenced objects, and
> delete loose objects that (1) are no longer referenced and (2) are
> "old enough".
> 
> The "old enough" bit is the special part here.  Before the gc, those
> objects were referenced (only by old reflog entries) and were found in
> a pack.  git currently writes those objects out to disk and gives them
> the age of the packfile they are contained in, which I think is the
> source of the bug.  We have a reference for those objects from the
> reflogs, know they aren't referenced anywhere else, so those objects
> to me are the age of those reflog entries: 90 days.  As such, they are
> "old enough" and should be deleted.

OK, I see what you're saying, but...

Show 6 quoted lines

> I never got around to fixing it properly, though, because 'git prune'
> is such a handy workaround that for now.  Having people nuke all their
> loose objects is a bit risky, but the only other workaround people had
> was to re-clone (waiting the requisite 20+ minutes for the repo to
> download) and throw away their old clone.  (Though some people even
> went that route, IIRC.)

If we were to delete those objects, wouldn't it be exactly the same as running "git prune"? Or setting your gc.pruneExpire to "now" or even "5 minutes"? Or are you concerned with taking other objects along for the ride that weren't part of old reflogs? I think that's a valid concern, but it's also an issue for objects which were previously referenced in a reflog, but are part of another current operation.

Also, what do you do if there weren't reflogs in the repo? Or the reflogs were deleted (e.g., because branch deletion drops the associated reflog entirely)?

> With big repos, it's easy to get into situations where there are well
> more than 10000 objects satisfying these conditions.  In fact, it's
> not all that rare that the repo has far more loose objects after a git
> gc than before.

Yes, this is definitely a wart and I think is worth addressing.

> I totally agree with your general plan to put unreferenced loose
> objects into a pack.  However, I don't think these objects should be
> part of that pack; they should just be deleted instead.

I assume by "these objects" you mean ones which used to be reachable from a reflog, but that reflog entry just expired. I think you'd be sacrificing some race-safety in that case.

If the objects went into a pack under a race-proof scheme, would that actually bother you? Is it the 10,000 objects that's a problem, or is it the horrible I/O from exploding them coupled with the annoying auto-gc behavior?

-Peff