Back off exponentially between lock-error retries

The retry budget was five fixed 50ms waits (250ms total). A foreground
`git status` refresh can hold index.lock for longer than that on a large
repo, so the retries could be exhausted before the lock clears. Wait 20ms
before the first retry and double each time, giving seven attempts over a
bit more than a second — enough to outlast a slow refresh while keeping
the common case (a lock that clears almost immediately) fast. The initial
delay is now a runner field so tests can zero it out instead of sleeping.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-07-10 08:58:22 +02:00
parent e3ecb77939
commit 4052057eee
3 changed files with 39 additions and 9 deletions

View file

@ -25,8 +25,9 @@ func NewGitCmdObjBuilder(log *logrus.Entry, innerBuilder *oscommands.CmdObjBuild
// the price of having a convenient interface where we can say .New(...).Run() is that our builder now depends on our runner, so when we want to wrap the default builder/runner in new functionality we need to jump through some hoops. We could avoid the use of a decorator function here by just exporting the runner field on the default builder but that would be misleading because we don't want anybody using that to run commands (i.e. we want there to be a single API used across the codebase)
updatedBuilder := innerBuilder.CloneWithNewRunner(func(runner oscommands.ICmdObjRunner) oscommands.ICmdObjRunner {
return &gitCmdObjRunner{
log: log,
innerRunner: runner,
log: log,
innerRunner: runner,
initialRetryDelay: defaultInitialRetryDelay,
}
})

View file

@ -11,13 +11,24 @@ import (
// here we're wrapping the default command runner in some git-specific stuff e.g. retry logic if we get an error due to the presence of .git/index.lock
const (
WaitTime = 50 * time.Millisecond
RetryCount = 5
// defaultInitialRetryDelay is how long we wait before the first retry of a
// command that failed with a transient lock error. We double it before each
// subsequent retry (see retryOnLockError), so across maxRetries attempts we
// wait for a bit over a second in total. That's long enough to outlast the
// brief window during which another git process holds a lock we need —
// typically our own foreground `git status` refresh, which takes index.lock
// to persist its refreshed stat-cache.
defaultInitialRetryDelay = 20 * time.Millisecond
maxRetries = 7
)
type gitCmdObjRunner struct {
log *logrus.Entry
innerRunner oscommands.ICmdObjRunner
// initialRetryDelay is the wait before the first lock-error retry. It's a
// field rather than the constant directly so tests can set it to zero and
// not actually sleep.
initialRetryDelay time.Duration
}
// isRetryableError returns true if a failed command hit a transient
@ -64,18 +75,21 @@ func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string,
// the command output we inspect to classify the failure. We clone the command
// for each attempt (inside run) because an *exec.Cmd can only be run once.
func (self *gitCmdObjRunner) retryOnLockError(run func() (string, error)) (string, error) {
delay := self.initialRetryDelay
var output string
var err error
for range RetryCount {
for attempt := range maxRetries {
output, err = run()
if err == nil || !isRetryableError(output, err) {
return output, err
break
}
// if we have an error based on a lock, we should wait a bit and then retry
self.log.Warn("lock error prevented command from running. Retrying command after a small wait")
time.Sleep(WaitTime)
if attempt < maxRetries-1 {
self.log.Warnf("lock error prevented command from running; retrying in %s", delay)
time.Sleep(delay)
delay *= 2
}
}
return output, err

View file

@ -50,6 +50,8 @@ func newTestRunner(inner *scriptedRunner) *gitCmdObjRunner {
return &gitCmdObjRunner{
log: utils.NewDummyLog(),
innerRunner: inner,
// don't actually sleep between retries
initialRetryDelay: 0,
}
}
@ -106,6 +108,19 @@ func TestRunWithOutputRetriesWhenLockErrorIsOnlyInError(t *testing.T) {
assert.Equal(t, 2, inner.calls)
}
func TestRunWithOutputGivesUpAfterMaxRetries(t *testing.T) {
results := make([]runnerResult, maxRetries)
for i := range results {
results[i] = runnerResult{err: errors.New("fatal: Unable to create '/repo/.git/index.lock': File exists.")}
}
inner := &scriptedRunner{results: results}
_, err := newTestRunner(inner).RunWithOutput(dummyCmdObj())
assert.Error(t, err)
assert.Equal(t, maxRetries, inner.calls)
}
func TestRunWithOutputRetriesLockErrorInLinkedWorktree(t *testing.T) {
// In a linked worktree the lock lives at .git/worktrees/<name>/index.lock
// rather than .git/index.lock, so only matching the bare "index.lock"