Re: [PATCH] rerere: keep a background gc from killing a rebase

Hi Patrick,

On Thu, Sep 03, 2026 at 10:32:36AM +0200, Patrick Steinhardt wrote:

Yes. Ideally, I'd think that we should both introduce the grace period
for locking the file and adapting the heuristic used by the maintenance
strategy. Whether we should completely disable auto-maintenance when in
the sequencer... I dunno. In any case, that feels like another separate
topic that should probably be discussed in its own series.

Phillip, this is the part I said I'd do in this series, so I'd
rather answer it here than just drop it. I think Patrick is right
that it's a topic of its own. My reason for wanting it in the same
series was the recording lost at a stop while the gc holds the lock,
and that was for the variant without the wait. With the wait kept,
the next pick waits the gc out and records as before, so the
sequencer patch no longer buys the rebase anything the rerere patch
doesn't, short of a prune that outlasts the timeout.

What it would still decide is whether a rebase with the merge backend
runs maintenance at all, the question from my last mail, and that is
a discussion of its own. So I'd make v2 the rerere patch alone and
send the sequencer change separately if you still want it. Say if
you would rather keep them together.

I think that having the wait is a sensible thing to do, as the race was
a preexisting one that was only uncovered by the change to the default
maintenance strategy. It can also happen with two concurrent processes
that both happen to write rerere entries. You wouldn't normally see the
wait anyway, so in the happy path nobody will really care. And in the
cases where you would see it the user is probably more happy to wait a
bit than having Git die (or just not write a rerere entry at all).

Agreed, and that is the order v2 keeps: wait first, skip only once
the wait has run out. Since your series means the gc now only runs
when there is something to prune, I measured how long that wait can
get: pruning 20000 stale entries holds the lock for 2.7 s here,
walking 20000 fresh ones takes 0.4 s, so the one second default
covers a prune of roughly 7000 entries if it scales. I'd keep the
default. A backlog that size is a one-off, and where it does hit,
the timeout now skips one recording where it used to kill the
rebase.

My patch is based on maint since the bug is there, and I'd keep it
that way unless Junio would rather have it on master. Merged up it
conflicts with d43f701d32 (lockfile: add
repo_hold_lock_file_for_update{,_timeout}{,_mode}(), 2026-07-14) in
setup_rerere(). The resolution is to take the repo-scoped helper, and
with that t4200 and t7900 pass on top of your series. I'll wait a
day or two for Phillip before rerolling.

Thanks,
Tom