From 93d059d33e5774a5f02af9e026d6c85b495a4ccf Mon Sep 17 00:00:00 2001 From: Ben Moskovitz Date: Thu, 9 Jan 2025 10:59:09 +1100 Subject: [PATCH] Allow configuring a range for jitter to be within Previously, jitter would only be available within a single second of the desired interval - eg, if we had jitter enabled and our next interval was 4.5 seconds, the actual calculated interval would be uniformly distributed between 4.5s and 5.5s. The issue with this approach is that it doesn't let callers spread load out efficiently over long timescales - eg, if i have a fleet of millions of agents set to ping every ten seconds, spreading those pings out over one second is only a little better than having them hit all at once - we'd ideally like to be able to spread those pings out over a larger area of the ping interval. This commit enables us to do just that - we can configure a custom range for jitter. To use our previous example, if we set a jitter range of (-1s, 3s], the actual intervals would be uniformly distributed between 3.5s -and 7.5s. --- retrier.go | 29 +++++++++++++++++++++++++++-- retrier_test.go | 41 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/retrier.go b/retrier.go index 2c6f933..c093a9e 100644 --- a/retrier.go +++ b/retrier.go @@ -10,12 +10,13 @@ import ( var defaultRandom = rand.New(rand.NewSource(time.Now().UnixNano())) -const jitterInterval = 1000 * time.Millisecond +const defaultJitterInterval = 1000 * time.Millisecond type Retrier struct { maxAttempts int attemptCount int jitter bool + jitterRange jitterRange forever bool rand *rand.Rand @@ -27,6 +28,8 @@ type Retrier struct { manualInterval *time.Duration } +type jitterRange struct{ min, max time.Duration } + type Strategy func(*Retrier) time.Duration const ( @@ -119,6 +122,26 @@ func WithStrategy(strategy Strategy, strategyType string) retrierOpt { func WithJitter() retrierOpt { return func(r *Retrier) { r.jitter = true + r.jitterRange = jitterRange{min: 0, max: defaultJitterInterval} + } +} + +// WithJitterRange enables jitter as [WithJitter] does, but allows the user to specify the range of the jitter as a +// half-open range [min, max) of time.Duration values. The jitter will be a random value in the range [min, max) added +// to the interval calculated by the retry strategy. The jitter will be recalculated for each retry. Both min and max may +// be negative, but min must be less than max. min and max may both be zero, which is equivalent to disabling jitter. +// If a negative jitter causes a negative interval, the interval will be clamped to zero. +func WithJitterRange(min, max time.Duration) retrierOpt { + if min >= max { + panic("min must be less than max") + } + + return func(r *Retrier) { + r.jitter = true + r.jitterRange = jitterRange{ + min: min, + max: max, + } } } @@ -174,7 +197,9 @@ func (r *Retrier) Jitter() time.Duration { if !r.jitter { return 0 } - return time.Duration((1.0 - r.rand.Float64()) * float64(jitterInterval)) + + min, max := float64(r.jitterRange.min), float64(r.jitterRange.max) + return time.Duration(min + (max-min)*rand.Float64()) } // MarkAttempt increments the attempt count for the retrier. This affects ShouldGiveUp, and also affects the retry interval diff --git a/retrier_test.go b/retrier_test.go index 3f5815b..1577f5e 100644 --- a/retrier_test.go +++ b/retrier_test.go @@ -186,10 +186,45 @@ func TestNextInterval_ConstantStrategy_WithJitter(t *testing.T) { for _, interval := range insomniac.sleepIntervals { assert.Check(t, interval > expected, "interval: %s, expected: %s", interval, expected) - assert.Check(t, cmp.DeepEqual(interval, expected, opt.DurationWithThreshold(jitterInterval))) + assert.Check(t, cmp.DeepEqual(interval, expected, opt.DurationWithThreshold(defaultJitterInterval))) } } +func TestNextInterval_ConstantStrategy_WithJitterRange(t *testing.T) { + t.Parallel() + + expected := 5 * time.Second + insomniac := newInsomniac() + + err := NewRetrier( + WithStrategy(Constant(expected)), + WithJitterRange(-3*time.Second, 3*time.Second), + WithMaxAttempts(1000), + WithSleepFunc(insomniac.sleep), + ).Do(func(_ *Retrier) error { return errDummy }) + assert.ErrorIs(t, err, errDummy) + + for _, interval := range insomniac.sleepIntervals { + assert.Check(t, cmp.DeepEqual(interval, expected, opt.DurationWithThreshold(6*time.Second))) + } +} + +func Test_WhenJitterCausesNegativeInterval_ItDoesntWait(t *testing.T) { + t.Parallel() + + before := time.Now() + err := NewRetrier( + WithStrategy(Constant(1*time.Second)), + WithJitterRange(-2*time.Second, -1*time.Second), + WithMaxAttempts(500), + // WithSleepFunc(nothing), // This should "really" sleep (for 0 seconds) + ).Do(func(_ *Retrier) error { return errDummy }) + after := time.Now() + assert.ErrorIs(t, err, errDummy) + + assert.Check(t, after.Sub(before) < 1*time.Millisecond) +} + func TestNextInterval_ExponentialStrategy(t *testing.T) { t.Parallel() @@ -263,7 +298,7 @@ func TestNextInterval_ExponentialStrategy_WithJitter(t *testing.T) { 16 * time.Second, }, insomniac.sleepIntervals, - opt.DurationWithThreshold(jitterInterval), + opt.DurationWithThreshold(defaultJitterInterval), ) } @@ -390,7 +425,7 @@ func TestNextInterval_ExponentialSubsecondStrategy_WithJitter(t *testing.T) { 13335 * time.Millisecond, 20535 * time.Millisecond, 31622 * time.Millisecond, - }, insomniac.sleepIntervals, opt.DurationWithThreshold(jitterInterval)) + }, insomniac.sleepIntervals, opt.DurationWithThreshold(defaultJitterInterval)) } func TestString_WithFiniteAttemptCount(t *testing.T) {