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

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

On Tue, Jul 05 2022, Gregory Szorc wrote:

> Thinking about this some more, it is clear that running `git gc` /
> `git repack` *and* `git maintenance` simultaneously is prone to
> badness since their locking mechanisms aren't aware of the other.

Yes, there are outstanding issues with the "maintenance" and "gc" locks,
they should be unified, and the gc.lock has bugs & race conditions that
need to be fixed. See these past threads:

    https://lore.kernel.org/git/87d02fi75p.fsf@evledraar.gmail.com/
    https://lore.kernel.org/git/87ef3o7ws1.fsf@evledraar.gmail.com/

I also have some old WIP patches to add a "gc testing" mode to our test
suite, similar to the "commit graph" instrumentation if you're
interested in working on this.

I.e. almost every command will fork off a "gi gc --auto", and we'll see
based on the errors whether our locking is still buggy/racy.

> I think it makes sense to consolidate onto `git maintenace` going
> forward as this seems to be where the inertia is. However, a barrier
> to that for me is the objects/maintenance.lock file has no acquisition
> timeout and will wait indefinitely. So in scenarios like mine where
> you have multiple processes looping over repos invoking `git
> maintenance run`, you can have extended execution stalls during
> long-running operations like `gc`.
> [...]
> There should probably be a configurable timeout for the maintenance
> lock like there is for other locks. This shouldn't be too much work
> and I may throw up a patch if others feel this is a good idea.

...but while I think this is all worthwhile I think you're on the wrong
path if you think this will help much or at all with the issue being
reported here.

Fixing the gc.lock (and maintenance lock...) would be nice because we'd
have "gc" be less dumb, and it wouldn't get itself into lock races etc.

But you cannot hope to fix the underlying "object expiry as things
become reachable" race condition that way, because that's those tasks
racing with *other* object/ref creation.

Such a fix would either need some repository-global lock (which would
entail all sorts of nastyness), or other workarounds around the inherent
race between different object store & ref operations, see Taylor's
recent "cruft pack write-out" series & my links to some past discussions
of the race here:

    https://lore.kernel.org/git/220630.86y1xeeeik.gmgdl@evledraar.gmail.com/