Merge pull request #9988 from hashicorp/dnephin/lib-retry-fix-max-wait

lib/retry: allow jitter to exceed max wait

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
 	}

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) {