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

From: Thomas Bachem <mail@thomasbachem.com>
Date: Sep 3, 2026, 08:11 UTC
Message-ID: <CAA0xjtp+Og_k7BYZfwX-LRW_8TAiCyp846+Mhk+hERM_GmRYkA@mail.gmail.com>
In-Reply-To: <apkkVAYOqjfAsp9-@pks.im>

Hi Patrick,

On Thu, Sep 03, 2026 at 09:40:04AM +0200, Patrick Steinhardt wrote:

Show 8 quoted lines

> I think this hints that we should tweak the default value of
> "maintenance.rerere-gc.auto". The way it's currently written we indeed
> are quite aggressive with spawning `git rerere gc`, and I agree that we
> should tweak it. And in the best case we'd not only respect whether we
> have a specific number of entries, but we should also respect whether
> those would be garbage collected in the first place.
>
> I'll send a patch series later today to do this.

Thanks. Checking whether anything would actually be pruned sounds right to me. It takes the frequency away, not the race, so I'd still do the sequencer part Phillip asked for.

> Having a locking timeout is sensible anyway, I think. It does not only
> solve races with a concurrent maintenance run, but also with concurrent
> writers.

Phillip found the wait unfortunate and I offered to drop it. You would keep it. I think the two fit together: wait up to rerere.lockTimeout, then warn and return -1 instead of dying, so the caller goes on without rerere this once. The gc passes 0 and does not wait. That takes the die out, which is what broke the rebase. The wait stays, bounded to a second, but skipping rerere is not free either: it can mean resolving a conflict again that rerere had already recorded, and a second is cheap next to that. With the sequencer no longer spawning the gc and your heuristic change, it should rarely come to either. Phillip, would that work for you?

> We should instead pass `LOCK_REPORT_ON_ERROR`, as the lockfile machinery
> knows better why exactly locking has failed.

Agreed on the text, which also names a stale lock. But the callers that go on without rerere then exit as if it were disabled, "git commit" with 0, so for them I'd print it as a warning through unable_to_lock_message() rather than let LOCK_REPORT_ON_ERROR call it an error. An explicit "git rerere forget" or "clear" fails as before.

> I think we can easily combine those two branches and simply set the
> timeout value to 0 in case we see the flag.

Yes, that folds into one call.

So v2: setup_rerere() waits up to rerere.lockTimeout, 0 for the gc, then warns and returns -1 where the caller can go on, with the sequencer patch on top. I'll reroll once Phillip has had a look.

Thanks, Tom