Skip to content

Commit 554d7f4

Browse files
committed
Prevent exponential backoff from overflowing.
When using `-cut-over-exponential-backoff` with `-default-retries` greater than `64`, the sleep interval will overflow and immediately begin retrying in rapid succession. This commit corrects the backoff algorithm and adds test for both "retry operation" functions.
1 parent ad5d3ea commit 554d7f4

File tree

2 files changed

+75
-7
lines changed

2 files changed

+75
-7
lines changed

go/logic/migrator.go

+8-7
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
var (
2626
ErrMigratorUnsupportedRenameAlter = errors.New("ALTER statement seems to RENAME the table. This is not supported, and you should run your RENAME outside gh-ost.")
27+
RetrySleepFn = time.Sleep
2728
)
2829

2930
type ChangelogState string
@@ -135,7 +136,7 @@ func (this *Migrator) retryOperation(operation func() error, notFatalHint ...boo
135136
for i := 0; i < maxRetries; i++ {
136137
if i != 0 {
137138
// sleep after previous iteration
138-
time.Sleep(1 * time.Second)
139+
RetrySleepFn(1 * time.Second)
139140
}
140141
err = operation()
141142
if err == nil {
@@ -155,16 +156,16 @@ func (this *Migrator) retryOperation(operation func() error, notFatalHint ...boo
155156
// attempts are reached. Wait intervals between attempts obey a maximum of
156157
// `ExponentialBackoffMaxInterval`.
157158
func (this *Migrator) retryOperationWithExponentialBackoff(operation func() error, notFatalHint ...bool) (err error) {
158-
var interval int64
159159
maxRetries := int(this.migrationContext.MaxRetries())
160160
maxInterval := this.migrationContext.ExponentialBackoffMaxInterval
161161
for i := 0; i < maxRetries; i++ {
162-
newInterval := int64(math.Exp2(float64(i - 1)))
163-
if newInterval <= maxInterval {
164-
interval = newInterval
165-
}
162+
interval := math.Min(
163+
float64(maxInterval),
164+
math.Max(1, math.Exp2(float64(i-1))),
165+
)
166+
166167
if i != 0 {
167-
time.Sleep(time.Duration(interval) * time.Second)
168+
RetrySleepFn(time.Duration(interval) * time.Second)
168169
}
169170
err = operation()
170171
if err == nil {

go/logic/migrator_test.go

+67
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"testing"
1919
"time"
2020

21+
"github.com/stretchr/testify/assert"
2122
"github.com/stretchr/testify/require"
2223
"github.com/stretchr/testify/suite"
2324
"github.com/testcontainers/testcontainers-go"
@@ -378,6 +379,72 @@ func (suite *MigratorTestSuite) TestFoo() {
378379
suite.Require().Equal("_testing_del", tableName)
379380
}
380381

382+
func TestMigratorRetry(t *testing.T) {
383+
oldRetrySleepFn := RetrySleepFn
384+
defer func() { RetrySleepFn = oldRetrySleepFn }()
385+
386+
migrationContext := base.NewMigrationContext()
387+
migrationContext.SetDefaultNumRetries(100)
388+
migrator := NewMigrator(migrationContext, "1.2.3")
389+
390+
var sleeps = 0
391+
RetrySleepFn = func(duration time.Duration) {
392+
assert.Equal(t, 1*time.Second, duration)
393+
sleeps++
394+
}
395+
396+
var tries = 0
397+
retryable := func() error {
398+
tries++
399+
if tries < int(migrationContext.MaxRetries()) {
400+
return errors.New("Backoff")
401+
}
402+
return nil
403+
}
404+
405+
result := migrator.retryOperation(retryable, false)
406+
assert.NoError(t, result)
407+
assert.Equal(t, sleeps, 99)
408+
assert.Equal(t, tries, 100)
409+
}
410+
411+
func TestMigratorRetryWithExponentialBackoff(t *testing.T) {
412+
oldRetrySleepFn := RetrySleepFn
413+
defer func() { RetrySleepFn = oldRetrySleepFn }()
414+
415+
migrationContext := base.NewMigrationContext()
416+
migrationContext.SetDefaultNumRetries(100)
417+
migrationContext.SetExponentialBackoffMaxInterval(42)
418+
migrator := NewMigrator(migrationContext, "1.2.3")
419+
420+
var sleeps = 0
421+
expected := []int{
422+
1, 2, 4, 8, 16, 32, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42,
423+
42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42,
424+
42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42,
425+
42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42,
426+
42, 42, 42, 42, 42, 42,
427+
}
428+
RetrySleepFn = func(duration time.Duration) {
429+
assert.Equal(t, time.Duration(expected[sleeps])*time.Second, duration)
430+
sleeps++
431+
}
432+
433+
var tries = 0
434+
retryable := func() error {
435+
tries++
436+
if tries < int(migrationContext.MaxRetries()) {
437+
return errors.New("Backoff")
438+
}
439+
return nil
440+
}
441+
442+
result := migrator.retryOperationWithExponentialBackoff(retryable, false)
443+
assert.NoError(t, result)
444+
assert.Equal(t, sleeps, 99)
445+
assert.Equal(t, tries, 100)
446+
}
447+
381448
func TestMigrator(t *testing.T) {
382449
suite.Run(t, new(MigratorTestSuite))
383450
}

0 commit comments

Comments
 (0)