Skip to content

Commit cb9054a

Browse files
committed
Fix retry after abort issue
1 parent e7e34e2 commit cb9054a

File tree

2 files changed

+69
-0
lines changed

2 files changed

+69
-0
lines changed

go/logic/migrator.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ func (this *Migrator) retryOperation(operation func() error, notFatalHint ...boo
160160
// sleep after previous iteration
161161
RetrySleepFn(1 * time.Second)
162162
}
163+
// Check for abort/context cancellation before each retry
164+
if abortErr := this.checkAbort(); abortErr != nil {
165+
return abortErr
166+
}
163167
err = operation()
164168
if err == nil {
165169
return nil
@@ -190,6 +194,10 @@ func (this *Migrator) retryOperationWithExponentialBackoff(operation func() erro
190194
if i != 0 {
191195
RetrySleepFn(time.Duration(interval) * time.Second)
192196
}
197+
// Check for abort/context cancellation before each retry
198+
if abortErr := this.checkAbort(); abortErr != nil {
199+
return abortErr
200+
}
193201
err = operation()
194202
if err == nil {
195203
return nil

go/logic/migrator_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,67 @@ func TestMigratorRetryWithExponentialBackoff(t *testing.T) {
714714
assert.Equal(t, tries, 100)
715715
}
716716

717+
func TestMigratorRetryAbortsOnContextCancellation(t *testing.T) {
718+
oldRetrySleepFn := RetrySleepFn
719+
defer func() { RetrySleepFn = oldRetrySleepFn }()
720+
721+
migrationContext := base.NewMigrationContext()
722+
migrationContext.SetDefaultNumRetries(100)
723+
migrator := NewMigrator(migrationContext, "1.2.3")
724+
725+
RetrySleepFn = func(duration time.Duration) {
726+
// No sleep needed for this test
727+
}
728+
729+
var tries = 0
730+
retryable := func() error {
731+
tries++
732+
if tries == 5 {
733+
// Cancel context on 5th try
734+
migrationContext.CancelContext()
735+
}
736+
return errors.New("Simulated error")
737+
}
738+
739+
result := migrator.retryOperation(retryable, false)
740+
assert.Error(t, result)
741+
// Should abort after 6 tries: 5 failures + 1 checkAbort detection
742+
assert.True(t, tries <= 6, "Expected tries <= 6, got %d", tries)
743+
// Verify we got context cancellation error
744+
assert.Contains(t, result.Error(), "context canceled")
745+
}
746+
747+
func TestMigratorRetryWithExponentialBackoffAbortsOnContextCancellation(t *testing.T) {
748+
oldRetrySleepFn := RetrySleepFn
749+
defer func() { RetrySleepFn = oldRetrySleepFn }()
750+
751+
migrationContext := base.NewMigrationContext()
752+
migrationContext.SetDefaultNumRetries(100)
753+
migrationContext.SetExponentialBackoffMaxInterval(42)
754+
migrator := NewMigrator(migrationContext, "1.2.3")
755+
756+
RetrySleepFn = func(duration time.Duration) {
757+
// No sleep needed for this test
758+
}
759+
760+
var tries = 0
761+
retryable := func() error {
762+
tries++
763+
if tries == 5 {
764+
// Cancel context on 5th try
765+
migrationContext.CancelContext()
766+
}
767+
return errors.New("Simulated error")
768+
}
769+
770+
result := migrator.retryOperationWithExponentialBackoff(retryable, false)
771+
assert.Error(t, result)
772+
// Should abort after 6 tries: 5 failures + 1 checkAbort detection
773+
assert.True(t, tries <= 6, "Expected tries <= 6, got %d", tries)
774+
// Verify we got context cancellation error
775+
assert.Contains(t, result.Error(), "context canceled")
776+
}
777+
717778
func (suite *MigratorTestSuite) TestCutOverLossDataCaseLockGhostBeforeRename() {
718779
ctx := context.Background()
719780

0 commit comments

Comments
 (0)