Skip to content

Commit aaa96fc

Browse files
committed
Fix flake by refactoring testutil.PollUpdate()
Decouple calculation of jitter from runtime execution of backoff. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 7a694ad commit aaa96fc

4 files changed

Lines changed: 122 additions & 35 deletions

File tree

opensearchapi/errors_internal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// this file be licensed under the Apache-2.0 license or a
55
// compatible open source license.
66

7-
package opensearchapi //nolint:testpackage // tests unexported resolveReturnQueryErrors
7+
package opensearchapi
88

99
import (
1010
"testing"

opensearchapi/testutil/helpers.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ var (
8383

8484
// PollUntil repeatedly calls checkFn until it returns true or the context times out.
8585
PollUntil = tptestutil.PollUntil
86+
87+
// BackoffDelay returns the delay for a given attempt using exponential backoff with optional jitter.
88+
BackoffDelay = tptestutil.BackoffDelay
8689
)
8790

8891
// ClientConfig returns an opensearchapi.Config for both secure and insecure opensearch

opensearchtransport/testutil/helpers.go

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,31 @@ func MustUniqueString(t *testing.T, prefix string) string {
4848
return fmt.Sprintf("%s-%d", prefix, rand.Int64()) // #nosec G404 -- Using math/rand for test resource names, not cryptographic purposes
4949
}
5050

51+
// BackoffDelay returns the delay for the given attempt using exponential
52+
// backoff with optional jitter. The formula is:
53+
//
54+
// delay = baseDelay * 2^attempt ± (jitter * delay)
55+
//
56+
// jitter is a factor in [0.0, 1.0] that randomizes the delay symmetrically
57+
// around the exponential value. A jitter of 0.0 returns the exact
58+
// exponential delay; 0.5 returns a value in [delay*0.5, delay*1.5].
59+
// The attempt is capped at 30 to prevent overflow.
60+
func BackoffDelay(baseDelay time.Duration, attempt int, jitter float64) time.Duration {
61+
cappedAttempt := min(attempt, 30)
62+
delay := time.Duration(int64(baseDelay) * (1 << cappedAttempt))
63+
64+
// #nosec G404 -- Using math/rand for test retry jitter, not cryptographic purposes
65+
if jitter > 0.0 {
66+
jitterRange := float64(delay) * jitter
67+
jitterOffset := (rand.Float64()*2 - 1) * jitterRange // -jitter to +jitter
68+
delay = time.Duration(float64(delay) + jitterOffset)
69+
}
70+
71+
return delay
72+
}
73+
5174
// PollUntil repeatedly calls checkFn until it returns true or the context times out.
52-
// It uses exponential backoff with jitter between attempts, based on the retry logic
53-
// from opensearchtransport.backoffRetry().
75+
// It uses exponential backoff with jitter between attempts via [BackoffDelay].
5476
//
5577
// This is useful for waiting for eventual consistency in integration tests, such as
5678
// waiting for ISM policies to be applied, indices to be ready, or cluster state changes.
@@ -103,18 +125,7 @@ func PollUntil(
103125

104126
// If this is not the last attempt, wait before retrying
105127
if attempt < maxAttempts-1 && baseDelay > 0 {
106-
// Exponential backoff: base delay * 2^attempt
107-
// Cap attempt to prevent overflow (2^30 is ~1 billion, more than enough)
108-
cappedAttempt := min(attempt, 30)
109-
delay := time.Duration(int64(baseDelay) * (1 << cappedAttempt))
110-
111-
// Apply jitter to avoid thundering herd
112-
// #nosec G404 -- Using math/rand for test retry jitter, not cryptographic purposes
113-
if jitter > 0.0 {
114-
jitterRange := float64(delay) * jitter
115-
jitterOffset := (rand.Float64()*2 - 1) * jitterRange // -jitter to +jitter
116-
delay = time.Duration(float64(delay) + jitterOffset)
117-
}
128+
delay := BackoffDelay(baseDelay, attempt, jitter)
118129

119130
// Wait with context cancellation support
120131
timer := time.NewTimer(delay)

opensearchtransport/testutil/helpers_test.go

Lines changed: 93 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -134,31 +134,104 @@ func TestPollUntil_ContextCancellation(t *testing.T) {
134134
}
135135
}
136136

137+
func TestBackoffDelay(t *testing.T) {
138+
t.Parallel()
139+
140+
tests := []struct {
141+
name string
142+
baseDelay time.Duration
143+
attempt int
144+
jitter float64
145+
wantExact time.Duration // -1 means check bounds instead
146+
wantMin time.Duration // only used when wantExact == -1
147+
wantMax time.Duration // only used when wantExact == -1
148+
}{
149+
{
150+
name: "attempt 0 no jitter",
151+
baseDelay: 10 * time.Millisecond, attempt: 0, jitter: 0.0,
152+
wantExact: 10 * time.Millisecond,
153+
},
154+
{
155+
name: "attempt 1 no jitter",
156+
baseDelay: 10 * time.Millisecond, attempt: 1, jitter: 0.0,
157+
wantExact: 20 * time.Millisecond,
158+
},
159+
{
160+
name: "attempt 2 no jitter",
161+
baseDelay: 10 * time.Millisecond, attempt: 2, jitter: 0.0,
162+
wantExact: 40 * time.Millisecond,
163+
},
164+
{
165+
name: "attempt 5 no jitter",
166+
baseDelay: 10 * time.Millisecond, attempt: 5, jitter: 0.0,
167+
wantExact: 320 * time.Millisecond,
168+
},
169+
{
170+
name: "attempt capped at 30",
171+
baseDelay: 1 * time.Nanosecond, attempt: 31, jitter: 0.0,
172+
wantExact: time.Duration(1 << 30), // same as attempt 30
173+
},
174+
{
175+
name: "jitter 0.5 in bounds",
176+
baseDelay: 100 * time.Millisecond, attempt: 0, jitter: 0.5,
177+
wantExact: -1,
178+
wantMin: 50 * time.Millisecond,
179+
wantMax: 150 * time.Millisecond,
180+
},
181+
{
182+
name: "jitter 1.0 in bounds",
183+
baseDelay: 100 * time.Millisecond, attempt: 0, jitter: 1.0,
184+
wantExact: -1,
185+
wantMin: 0,
186+
wantMax: 200 * time.Millisecond,
187+
},
188+
}
189+
190+
for _, tt := range tests {
191+
t.Run(tt.name, func(t *testing.T) {
192+
t.Parallel()
193+
194+
if tt.wantExact >= 0 {
195+
got := testutil.BackoffDelay(tt.baseDelay, tt.attempt, tt.jitter)
196+
require.Equal(t, tt.wantExact, got, "exact delay mismatch")
197+
return
198+
}
199+
200+
// For jittered cases, run multiple times and check bounds
201+
const iterations = 100
202+
for i := range iterations {
203+
got := testutil.BackoffDelay(tt.baseDelay, tt.attempt, tt.jitter)
204+
require.GreaterOrEqual(t, got, tt.wantMin, "iteration %d: delay below minimum", i)
205+
require.LessOrEqual(t, got, tt.wantMax, "iteration %d: delay above maximum", i)
206+
}
207+
})
208+
}
209+
}
210+
211+
func TestBackoffDelay_JitterProducesVariance(t *testing.T) {
212+
t.Parallel()
213+
214+
// Verify that jitter actually produces different values across calls.
215+
const iterations = 20
216+
seen := make(map[time.Duration]struct{}, iterations)
217+
for range iterations {
218+
d := testutil.BackoffDelay(100*time.Millisecond, 0, 0.5)
219+
seen[d] = struct{}{}
220+
}
221+
// With 50% jitter on 100ms, the chance of 20 identical values is vanishingly small.
222+
require.Greater(t, len(seen), 1, "jitter should produce different delay values")
223+
}
224+
137225
func TestPollUntil_WithJitter(t *testing.T) {
226+
// Verify PollUntil works correctly with jitter enabled (no timing assertion).
138227
ctx := context.Background()
139228
attempts := 0
140-
141-
start := time.Now()
142-
err := testutil.PollUntil(t, ctx, 10*time.Millisecond, 3, 0.5, func() (bool, error) {
229+
err := testutil.PollUntil(t, ctx, 1*time.Millisecond, 3, 0.5, func() (bool, error) {
143230
attempts++
144-
if attempts == 3 {
145-
return true, nil
146-
}
147-
return false, nil
231+
return attempts == 3, nil
148232
})
149-
elapsed := time.Since(start)
150-
151-
if err != nil {
152-
t.Fatalf("Expected success, got error: %v", err)
153-
}
154-
if attempts != 3 {
155-
t.Fatalf("Expected 3 attempts, got %d", attempts)
156-
}
157-
// With exponential backoff and jitter, timing should be reasonable
158-
// Attempt 0: ~10ms +/- 5ms, Attempt 1: ~20ms +/- 10ms = ~15-45ms baseline + overhead
159-
if elapsed > 100*time.Millisecond {
160-
t.Fatalf("Expected completion within 100ms with backoff and jitter, took %v", elapsed)
161-
}
233+
require.NoError(t, err)
234+
require.Equal(t, 3, attempts)
162235
}
163236

164237
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)