Re: [PATCH v2] rerere: keep a background gc from killing a rebase
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Sep 7, 2026, 10:07 UTC
- Message-ID
- <595d0d45-7000-4c52-8430-f18ce8f99c71@gmail.com>
- In-Reply-To
- <CAA0xjtrkjaOC_+jhN=Vjm9e0T+iqAZeeMKx-ymVaQcLA37bm-w@mail.gmail.com>
Hi Thomas
On 04/09/2026 16:55, Thomas Bachem wrote:
Show 13 quoted lines
> On 04/09/2026 16:21, Phillip Wood wrote: >> With Patricks patches that's no-longer true I think. I think a better >> motivation, as the cache is per-repository, rather than per-worktree, is >> concurrent writers running in different worktrees. > > MERGE_RR is per worktree, though, and so is its lock: > > $ git -C linked rev-parse --git-path MERGE_RR > /path/to/main/.git/worktrees/linked/MERGE_RR > > so writers in different worktrees never meet on it. What they share is > rr-cache, which a gc in one worktree prunes under its own worktree's > lock only. That is a gap of its own, and not one this patch closes.
Oh, I didn't realize the lock was per-worktree. So the lock "rerere gc" takes does not actually stop another process running in a different worktree from altering the rerere cache.
Show 16 quoted lines
> What remains after Patrick's series is any "git rerere gc" that runs > while a command records a conflict, from "git gc", from a maintenance > run, or from auto maintenance once enough entries are stale. The v3 > message says it that way. > >> Overall, this commit message is rather long and it would be helpful if >> you could distill it to remove unnecessary and unrelated details. > > Done, it is a quarter of the size now. > >> Why do those commands fail rather than wait? > > They wait like everything else, and once the time is up they fail > instead of going on without rerere, which is all they are for. That > way a stale lock gets the usual advice to remove it. The config text > said otherwise, fixed.
That's good, I think I'd maybe misunderstood what the original patch was trying to say.
Show 16 quoted lines
>> It might be worth adding a check above here that BUG()s out if the >> caller passes an incompatible set of flags. > > Added, for RERERE_NOWAIT with RERERE_LOCK_OR_DIE and for > RERERE_READONLY with either. > >> A background job that the user did not explicitly start printing to the >> terminal is rather confusing as it is likely to get mixed in with the >> output of whatever is running in the foreground. > > The detached maintenance run has no terminal: daemonize() closes the > standard descriptors and reopens them on /dev/null, so the gc's > warning goes nowhere when it loses the lock. Where it cannot detach, > on Windows, it runs in the foreground of the commit that started it > and there is no race to lose. The warning the user does see is the > foreground command's own, when it gives up waiting.
Thanks for clarifying that
Phillip
> > Thanks, > Thomas