Re: Race condition between repack and loose-objects maintenance task

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Jul 26 2022, Gregory Szorc wrote:

> On Wed, Jul 20, 2022 at 3:12 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>>
>> On Tue, Jul 19 2022, Gregory Szorc wrote:
>>
>> > On 6/29/2022 8:23 PM, Taylor Blau wrote:
>> >> On Wed, Jun 29, 2022 at 11:19:09PM -0400, Taylor Blau wrote:
>>> [...]
>> > Since the interaction between cruft packfiles and MIDX appears to be
>> > buggy, I think I'm going to leave cruft packfiles disabled until these
>> > features work better together.
>>
>> I haven't looked deeply into whether there's any cruft packfile & MIDX
>> interaction at play here, but I suspect based on past experience that
>> this has nothing whatsoever to do with the MIDX.
>
> Taylor seemed to acknowledge a bug here a few posts back. Essentially:
>
> 1. Cruft packfile created
> 2. MIDX repack and expire treats the cruft packfile as a regular pack
> and folds its content into a new, regular packfile, deleting the
> mtimes file/metadata in the process.
>
> I don't fully grok the mechanism, but I believe the loss of the mtimes
> metadata effectively results in indefinite retention of unreferenced
> objects.

Maybe, but I don't see how indefinite retention would happen unless
you're flip-flopping back & forth.

If we're deleting the mtimes file but keeping the pack they're in we'd
notice that those packed objects are unreferenced on the next gc, so
we'd explode them into loose objects, and then the timer starts ticking
on their expiry.

It's a lot of churn, but once that timer expires we'll then expire those
objects.

>>
>> It's because git suddenly found a bunch of objects that should be
>> evicted from the packs, as you disabled the cruft pack feature.
>>
>> Here's a past test-case & report of mine where I ran into that:
>>
>>     https://lore.kernel.org/git/87fu6bmr0j.fsf@evledraar.gmail.com/
>>
>> That was way before cruft packfiles were integrated, but from my
>> understanding of how they work the mechanism is exactly the same.
>>
>> I.e. in that case I deleted a bunch of refs (tag objects), which caused
>> those to become unreferenced, so on the next "gc/repack" they were all
>> evicted from their respective packs, at which point the "gc.pruneExpire"
>> clock started ticking for them.
>>
>> Whereas in your case you disabled "cruft packs", which are basically a
>> mechanism where git keeps such "unused" objects in a pack. Once you
>> disabled it git stopped considering the new *.mtimes file, and started
>> extracting these loose objects en-masse.
>>
>> Which b.t.w. is something I think we might want to reconsider. I.e. we
>> just pass "--cruft" to pack-objects (see b757353676d
>> (builtin/pack-objects.c: --cruft without expiration, 2022-05-20)), but
>> don't have a "write cruft" v.s. "read cruft" setting (but see below).
>>
>> The one thing that gives me pause here is your mention of "summarily
>> deleted by the GC's prune". Did you run "git prune --expire=now"
>> manually, or do you have a really aggressive "gc.pruneExpire" setting?
>> It's also posible that I'm entirely misunderstanding this whole thing...
>>
>> Anyway.
>>
>> Since that 2018-era post of mine I've thought a bit about how to best
>> handle this particular edge case.
>>
>> I think the simplest implementation that would work well enough would be
>> to teach "git repack" to limit how many objects it's willing to extract
>> from a pack. I.e. we'd have the amount of "unreachable" objects we'd be
>> willing to eject from the pack limited, with check similar to how
>> "gc.auto" works (i.e. once we reach the limit we'd implicitly turn on
>> "--keep-unreachable").
>>
>> It's far from perfect, and e.g. if the N objects you're extracting
>> happen to delta-compress particularly well you could still have cases
>> where e.g. a 1GB repo becomes a 10GB one with the new loose objects.
>>
>> But it would allow us to limit the common case where we'd e.g. create 1k
>> new loose objects, not 100k or whatever.
>>
>> It does have the downside that unless you'd disable that limit the "gc"
>> -> "gc.pruneExpire" window would become even more fuzzy. I.e. now if you
>> run "gc" the clock starts ticking right away, after this the clock would
>> start ticking whenever we happened to extract those objects.
>>
>> So there's certainly less dumb ways to do this, and we'd need to make
>> sure that whatever limit we set is aggressive enough that we'd always
>> "stay ahead". I.e. if a repo just has a very high creation rate of
>> objects that become unreachable we'd need to ensure that we wouldn't
>> have an ever growing .git as our eviction rate wouldn't keep up with our
>> additions.
>
> I agree this is a hard problem! In my case, the underlying repo sees a
> few thousand pushes daily. This results in ~30k new unreferenced
> objects on a typical workday. With NFS/EFS, we've observed that the
> entire filesystem can get slow once object loosening has written as
> few as a few thousand files. Given our high volume, I worry that
> imposing a loose object creation ceiling will perpetually throttle and
> we'll accumulate unreferenced objects indefinitely.

If you're feeling adventurous you may want to try to rebase or otherwise
experiment with this old series of mine, that never made it in:
https://lore.kernel.org/git/20181028225023.26427-1-avarab@gmail.com/

I don't run git on NFS anymore, but when I did I found that the main
contributor to loose object-related slowness on pushes was the collision
checking.

Whereas if you're willing to simply write duplicate objects ....