diff --git a/lib/retry/retry.go b/lib/retry/retry.go index 17bbeaf83..e843b2f4f 100644 --- a/lib/retry/retry.go +++ b/lib/retry/retry.go @@ -42,9 +42,11 @@ type Waiter struct { MinFailures uint // MinWait time. Returned after the first failure. MinWait time.Duration - // MaxWait time. + // MaxWait time applied before Jitter. Note that the actual maximum wait time + // is MaxWait + MaxWait * Jitter. MaxWait time.Duration - // Jitter to add to each wait time. + // Jitter to add to each wait time. The Jitter is applied after MaxWait, which + // may cause the actual wait time to exceed MaxWait. Jitter Jitter // Factor is the multiplier to use when calculating the delay. Defaults to // 1 second. @@ -67,12 +69,14 @@ func (w *Waiter) delay() time.Duration { if shift < 31 { waitTime = (1 << shift) * factor } + // apply MaxWait before jitter so that multiple waiters with the same MaxWait + // do not converge when they hit their max. + if w.MaxWait != 0 && waitTime > w.MaxWait { + waitTime = w.MaxWait + } if w.Jitter != nil { waitTime = w.Jitter(waitTime) } - if w.MaxWait != 0 && waitTime > w.MaxWait { - return w.MaxWait - } if waitTime < w.MinWait { return w.MinWait } diff --git a/lib/retry/retry_test.go b/lib/retry/retry_test.go index 3b6e8f258..8c4f664f6 100644 --- a/lib/retry/retry_test.go +++ b/lib/retry/retry_test.go @@ -105,6 +105,18 @@ func TestWaiter_Delay(t *testing.T) { require.Equal(t, expected*time.Millisecond, w.delay(), "failure count: %d", i) } }) + + t.Run("jitter can exceed MaxWait", func(t *testing.T) { + w := &Waiter{ + MaxWait: 20 * time.Second, + Jitter: NewJitter(300), + + failures: 30, + } + + delay := w.delay() + require.True(t, delay > 20*time.Second, "expected delay %v to be greater than MaxWait %v", delay, w.MaxWait) + }) } func TestWaiter_Wait(t *testing.T) {