Re: HEAD.lock and git maintenance

[Patrick Steinhardt <ps@pks.im>]

On Tue, May 27, 2025 at 09:33:50AM -0700, Emily Shaffer wrote:
> On Mon, May 26, 2025 at 6:22 AM Patrick Steinhardt <ps@pks.im> wrote:
[snip]
> > > Using GIT_TRACE_PERFORMANCE, we noticed that a Git maintenance process
> > > (/usr/libexec/git-core/git maintenance run --auto --no-quiet --detach)
> > > sometimes starts after the git fetch command, occasionally in detached
> > > mode. We suspect this operation is causing the issue because we've
> > > verified that the git maintenance command requires HEAD.lock before it
> > > starts running. We are considering setting maintenance.autoDetach to
> > > false. We are unsure if this is a bug or if it is working as intended,
> > > and would appreciate your comments on this.
> >
> > thanks for your report! A couple months ago there was a similar
> > discussion with someone else, but I cannot find that thread anymore,
> > unfortunately.
> 
> Google had a big problem with this behavior about a year ago, I'm not
> sure if we got far with a thread about it though. That may be what
> you're thinking of.

Yeah, I remembered it was Google that had problems, but I thought that
this was at max half a year ago. So I didn't care to look back further
than that :)

> > The root cause here is repository maintenance with `--auto --detach`
> > will detach before spawning git-gc(1). This command may decide to pack
> > your references and thus cause them to be locked. This then triggers a
> > race condition, where the next Git command that wants to modify refs may
> > not be able to lock "packed-refs" because we are still busy repacking
> > them.
> >
> > The actual timeout to lock the "packed-refs" file is configurable via
> > "core.packedRefsTimeout", so bumping this value may make the problem
> > less likely to happen. But it's only papering over the actual issue.
> >
> > I'll send a patch series soonish that fixes this issue. I think the
> > solution would be to make git-maintenance(1) learn about tasks that
> > should run previous and after daemonizing the process to avoid this race
> > condition. The effect would be that the caller of auto-maintenance will
> > not continue before refs have been packed, which is similar to what
> > git-gc(1) used to do in the past.
> 
> We'll look forward to this series with interest, thanks.

For reference, I've sent the series yesterday via [1]. I'll keep you
Cc'd on that series from now on.

Patrick

[1]: <20250527-b4-pks-maintenance-ref-lock-race-v1-0-e1ceb2dea66e@pks.im>