Skip to content

Commit fe40ebd

Browse files
committed
Skip retries for warning errors
Warning errors indicate data consistency issues that won't resolve on retry, so attempting to retry them is futile and causes unnecessary delays. This change detects warning errors early and aborts immediately instead of retrying.
1 parent cb9054a commit fe40ebd

File tree

2 files changed

+65
-0
lines changed

2 files changed

+65
-0
lines changed

go/logic/migrator.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,13 @@ func (this *Migrator) retryOperation(operation func() error, notFatalHint ...boo
168168
if err == nil {
169169
return nil
170170
}
171+
// Check if this is an unrecoverable error (data consistency issues won't resolve on retry)
172+
if strings.Contains(err.Error(), "warnings detected") {
173+
if len(notFatalHint) == 0 {
174+
_ = base.SendWithContext(this.migrationContext.GetContext(), this.migrationContext.PanicAbort, err)
175+
}
176+
return err
177+
}
171178
// there's an error. Let's try again.
172179
}
173180
if len(notFatalHint) == 0 {
@@ -202,6 +209,13 @@ func (this *Migrator) retryOperationWithExponentialBackoff(operation func() erro
202209
if err == nil {
203210
return nil
204211
}
212+
// Check if this is an unrecoverable error (data consistency issues won't resolve on retry)
213+
if strings.Contains(err.Error(), "warnings detected") {
214+
if len(notFatalHint) == 0 {
215+
_ = base.SendWithContext(this.migrationContext.GetContext(), this.migrationContext.PanicAbort, err)
216+
}
217+
return err
218+
}
205219
}
206220
if len(notFatalHint) == 0 {
207221
// Use helper to prevent deadlock if listenOnPanicAbort already exited

go/logic/migrator_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,57 @@ func TestMigratorRetryWithExponentialBackoffAbortsOnContextCancellation(t *testi
775775
assert.Contains(t, result.Error(), "context canceled")
776776
}
777777

778+
func TestMigratorRetrySkipsRetriesForWarnings(t *testing.T) {
779+
oldRetrySleepFn := RetrySleepFn
780+
defer func() { RetrySleepFn = oldRetrySleepFn }()
781+
782+
migrationContext := base.NewMigrationContext()
783+
migrationContext.SetDefaultNumRetries(100)
784+
migrator := NewMigrator(migrationContext, "1.2.3")
785+
786+
RetrySleepFn = func(duration time.Duration) {
787+
t.Fatal("Should not sleep/retry for warning errors")
788+
}
789+
790+
var tries = 0
791+
retryable := func() error {
792+
tries++
793+
return errors.New("warnings detected in statement 1 of 1: [Warning: Duplicate entry 'test' for key 'idx' (1062)]")
794+
}
795+
796+
result := migrator.retryOperation(retryable, false)
797+
assert.Error(t, result)
798+
// Should only try once - no retries for warnings
799+
assert.Equal(t, 1, tries, "Expected exactly 1 try (no retries) for warning error")
800+
assert.Contains(t, result.Error(), "warnings detected")
801+
}
802+
803+
func TestMigratorRetryWithExponentialBackoffSkipsRetriesForWarnings(t *testing.T) {
804+
oldRetrySleepFn := RetrySleepFn
805+
defer func() { RetrySleepFn = oldRetrySleepFn }()
806+
807+
migrationContext := base.NewMigrationContext()
808+
migrationContext.SetDefaultNumRetries(100)
809+
migrationContext.SetExponentialBackoffMaxInterval(42)
810+
migrator := NewMigrator(migrationContext, "1.2.3")
811+
812+
RetrySleepFn = func(duration time.Duration) {
813+
t.Fatal("Should not sleep/retry for warning errors")
814+
}
815+
816+
var tries = 0
817+
retryable := func() error {
818+
tries++
819+
return errors.New("warnings detected in statement 1 of 1: [Warning: Duplicate entry 'test' for key 'idx' (1062)]")
820+
}
821+
822+
result := migrator.retryOperationWithExponentialBackoff(retryable, false)
823+
assert.Error(t, result)
824+
// Should only try once - no retries for warnings
825+
assert.Equal(t, 1, tries, "Expected exactly 1 try (no retries) for warning error")
826+
assert.Contains(t, result.Error(), "warnings detected")
827+
}
828+
778829
func (suite *MigratorTestSuite) TestCutOverLossDataCaseLockGhostBeforeRename() {
779830
ctx := context.Background()
780831

0 commit comments

Comments
 (0)