On Mon, May 06, 2019 at 11:44:06AM +0200, Ævar Arnfjörð Bjarmason wrote:
> Maybe there's some case I haven't thought of that makes this stupid, but
> I wonder if something like a "gc quarantine" might be a fix fo both of
> the the issues you noted above.
>
> I.e. it seems to me that the main issue is that we conflate "mtime 2
> weeks old because it's unreferenced for 2 weeks" v.s. "mtime 2 weeks old
> because we haven't gotten around to a 'gc'".
>
> So in such a "gc quarantine" mode when we discover an object/pack that's
> unreachable/purely made up of unreachable objects we'd move the relevant
> loose object/"loose" pack to such a quarantine, which would just be
> .git/unreferenced-objects/{??,pack}/ or whatever.
>
> AFAICT both cases you mentioned above would be mitigated by this because
> we'd no longer conflate "haven't gc'd this yet and it's 2 weeks old"
> v.s. "hasn't been referenced in 2 weeks".
Michael Haggerty and I have (off-list) discussed variations on that, but
it opens up a lot of new issues. Moving something into quarantine isn't
atomic. So you've still corrupted the repo, but now it's recoverable by
reaching into the quarantine. Who notices that the repo is corrupt, and
how? When do we expire objects from quarantine?
I think the heart of the issue is really the lack of atomicity in the
operations. You need some way to mark "I am using this now" in a way
that cannot race with "looks like nobody is using this, so I'll delete
it".
And ideally without traversing large bits of the graph on the writing
side, and without requiring any stop-the-world locks during pruning.
> I started looking at this initially because I was wondering if the
> --keep-unreachable mode you modified in e26a8c4721 ("repack: extend
> --keep-unreachable to loose objects", 2016-06-13) could be made to write
> out such "unreferenced" objects into their *own* pack, so we could
> delete them all at once as a batch, and wouldn't create the "ref
> explosions" mentioned in [1].
>
> But of course without an accompanying quarantine described above doing
> that would just make this race condition worse.
I'm not sure it really makes it worse. The pack would have the same
mtime as the loose objects would.
-Peff
