Visitar URL original
Back off exponentially between lock-error retries · jesseduffield/lazygit@ee7e70e · GitHub
Skip to content

Commit ee7e70e

Browse files
stefanhallerclaude
andcommitted
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>
1 parent 070dedf commit ee7e70e

3 files changed

Lines changed: 39 additions & 9 deletions

File tree

‎pkg/commands/git_cmd_obj_builder.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ func NewGitCmdObjBuilder(log *logrus.Entry, innerBuilder *oscommands.CmdObjBuild
2525
// 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)
2626
updatedBuilder := innerBuilder.CloneWithNewRunner(func(runner oscommands.ICmdObjRunner) oscommands.ICmdObjRunner {
2727
return &gitCmdObjRunner{
28-
log: log,
29-
innerRunner: runner,
28+
log: log,
29+
innerRunner: runner,
30+
initialRetryDelay: defaultInitialRetryDelay,
3031
}
3132
})
3233

‎pkg/commands/git_cmd_obj_runner.go‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,24 @@ import (
1111
// 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
1212

1313
const (
14-
WaitTime = 50 * time.Millisecond
15-
RetryCount = 5
14+
// defaultInitialRetryDelay is how long we wait before the first retry of a
15+
// command that failed with a transient lock error. We double it before each
16+
// subsequent retry (see retryOnLockError), so across maxRetries attempts we
17+
// wait for a bit over a second in total. That's long enough to outlast the
18+
// brief window during which another git process holds a lock we need —
19+
// typically our own foreground `git status` refresh, which takes index.lock
20+
// to persist its refreshed stat-cache.
21+
defaultInitialRetryDelay = 20 * time.Millisecond
22+
maxRetries = 7
1623
)
1724

1825
type gitCmdObjRunner struct {
1926
log *logrus.Entry
2027
innerRunner oscommands.ICmdObjRunner
28+
// initialRetryDelay is the wait before the first lock-error retry. It's a
29+
// field rather than the constant directly so tests can set it to zero and
30+
// not actually sleep.
31+
initialRetryDelay time.Duration
2132
}
2233

2334
// isRetryableError returns true if a failed command hit a transient
@@ -64,18 +75,21 @@ func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string,
6475
// the command output we inspect to classify the failure. We clone the command
6576
// for each attempt (inside run) because an *exec.Cmd can only be run once.
6677
func (self *gitCmdObjRunner) retryOnLockError(run func() (string, error)) (string, error) {
78+
delay := self.initialRetryDelay
6779
var output string
6880
var err error
69-
for range RetryCount {
81+
for attempt := range maxRetries {
7082
output, err = run()
7183

7284
if err == nil || !isRetryableError(output, err) {
73-
return output, err
85+
break
7486
}
7587

76-
// if we have an error based on a lock, we should wait a bit and then retry
77-
self.log.Warn("lock error prevented command from running. Retrying command after a small wait")
78-
time.Sleep(WaitTime)
88+
if attempt < maxRetries-1 {
89+
self.log.Warnf("lock error prevented command from running; retrying in %s", delay)
90+
time.Sleep(delay)
91+
delay *= 2
92+
}
7993
}
8094

8195
return output, err

‎pkg/commands/git_cmd_obj_runner_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ func newTestRunner(inner *scriptedRunner) *gitCmdObjRunner {
5050
return &gitCmdObjRunner{
5151
log: utils.NewDummyLog(),
5252
innerRunner: inner,
53+
// don't actually sleep between retries
54+
initialRetryDelay: 0,
5355
}
5456
}
5557

@@ -106,6 +108,19 @@ func TestRunWithOutputRetriesWhenLockErrorIsOnlyInError(t *testing.T) {
106108
assert.Equal(t, 2, inner.calls)
107109
}
108110

111+
func TestRunWithOutputGivesUpAfterMaxRetries(t *testing.T) {
112+
results := make([]runnerResult, maxRetries)
113+
for i := range results {
114+
results[i] = runnerResult{err: errors.New("fatal: Unable to create '/repo/.git/index.lock': File exists.")}
115+
}
116+
inner := &scriptedRunner{results: results}
117+
118+
_, err := newTestRunner(inner).RunWithOutput(dummyCmdObj())
119+
120+
assert.Error(t, err)
121+
assert.Equal(t, maxRetries, inner.calls)
122+
}
123+
109124
func TestRunWithOutputRetriesLockErrorInLinkedWorktree(t *testing.T) {
110125
// In a linked worktree the lock lives at .git/worktrees/<name>/index.lock
111126
// rather than .git/index.lock, so only matching the bare "index.lock"

0 commit comments

Comments
 (0)