From ce56cc6998ddbf2bfd12653367389aac109d0fea Mon Sep 17 00:00:00 2001 From: riccardom Date: Wed, 5 Aug 2026 15:03:52 +0200 Subject: [PATCH] [relay] randomize the relay reconnect backoff The relay client's reconnect backoff was built without a RandomizationFactor, so it stayed at the zero value and every client that lost the same relay server retried on the identical 2/4/8/16/32/60 second schedule, bunching up on the server as soon as it came back. Use backoff.DefaultRandomizationFactor, as the other backoffs in the client do. The mean interval is unchanged; only the per-client offset differs. The exponential backoff construction moves into newExponentialBackOff so the configuration can be asserted directly: backoff.NewTicker fires its first tick immediately, so the schedule is not observable through the ticker. --- shared/relay/client/guard.go | 19 +++++++---- shared/relay/client/guard_backoff_test.go | 40 +++++++++++++++++++++++ 2 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 shared/relay/client/guard_backoff_test.go diff --git a/shared/relay/client/guard.go b/shared/relay/client/guard.go index 98b1b333e..8fba49b5b 100644 --- a/shared/relay/client/guard.go +++ b/shared/relay/client/guard.go @@ -154,14 +154,19 @@ func (g *Guard) notifyReconnected() { } func (g *Guard) exponentTicker(ctx context.Context) *backoff.Ticker { - bo := backoff.WithContext(&backoff.ExponentialBackOff{ - InitialInterval: 2 * time.Second, - Multiplier: 2, - MaxInterval: g.maxBackoffInterval, - Clock: backoff.SystemClock, - }, ctx) + return backoff.NewTicker(backoff.WithContext(g.newExponentialBackOff(), ctx)) +} - return backoff.NewTicker(bo) +func (g *Guard) newExponentialBackOff() *backoff.ExponentialBackOff { + return &backoff.ExponentialBackOff{ + InitialInterval: 2 * time.Second, + // Spreads the reconnects of every client that lost the same relay server, so they do + // not hit it at the same instants. + RandomizationFactor: backoff.DefaultRandomizationFactor, + Multiplier: 2, + MaxInterval: g.maxBackoffInterval, + Clock: backoff.SystemClock, + } } func waiteBeforeRetry(ctx context.Context) bool { diff --git a/shared/relay/client/guard_backoff_test.go b/shared/relay/client/guard_backoff_test.go new file mode 100644 index 000000000..700a3f822 --- /dev/null +++ b/shared/relay/client/guard_backoff_test.go @@ -0,0 +1,40 @@ +package client + +import ( + "testing" + "time" + + "github.com/cenkalti/backoff/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestExponentialBackOff_IsRandomized fails without the randomization factor, where every +// client that lost the same relay server walked the identical 2/4/8/16... second schedule. +func TestExponentialBackOff_IsRandomized(t *testing.T) { + g := NewGuard(&ServerPicker{}, 0) + + const samples = 50 + intervals := make(map[time.Duration]struct{}, samples) + for i := 0; i < samples; i++ { + bo := g.newExponentialBackOff() + bo.Reset() + intervals[bo.NextBackOff()] = struct{}{} + } + + assert.Greater(t, len(intervals), samples/2, "backoff intervals are not randomized across clients") +} + +func TestExponentialBackOff_RespectsMaxInterval(t *testing.T) { + const maxInterval = 10 * time.Second + g := NewGuard(&ServerPicker{}, maxInterval) + + bo := g.newExponentialBackOff() + bo.Reset() + + delta := time.Duration(backoff.DefaultRandomizationFactor * float64(maxInterval)) + for i := 0; i < 20; i++ { + require.LessOrEqual(t, bo.NextBackOff(), maxInterval+delta, + "interval exceeded MaxInterval plus its randomization window") + } +}