diff --git a/br/pkg/backup/client.go b/br/pkg/backup/client.go index 8125d21f8848a..602838ba61f21 100644 --- a/br/pkg/backup/client.go +++ b/br/pkg/backup/client.go @@ -1237,7 +1237,7 @@ func (bc *Client) OnBackupResponse( } res := utils.HandleBackupError(errPb, storeID, errContext) switch res.Strategy { - case utils.GiveUp: + case utils.StrategyGiveUp: errMsg := res.Reason if len(errMsg) <= 0 { errMsg = errPb.Msg diff --git a/br/pkg/utils/backoff.go b/br/pkg/utils/backoff.go index ffb58646e1752..89c6196ad953d 100644 --- a/br/pkg/utils/backoff.go +++ b/br/pkg/utils/backoff.go @@ -173,7 +173,7 @@ func (bo *importerBackoffer) NextBackoff(err error) time.Duration { errs := multierr.Errors(err) lastErr := errs[len(errs)-1] res := HandleUnknownBackupError(lastErr.Error(), 0, bo.errContext) - if res.Strategy == Retry { + if res.Strategy == StrategyRetry { bo.delayTime = 2 * bo.delayTime bo.attempt-- } else { diff --git a/br/pkg/utils/error_handling.go b/br/pkg/utils/error_handling.go index f6d3c0abb13d6..6c6c84a2a1884 100644 --- a/br/pkg/utils/error_handling.go +++ b/br/pkg/utils/error_handling.go @@ -72,16 +72,16 @@ type ErrorHandlingResult struct { type ErrorHandlingStrategy int const ( - // Retry error can be retried but will consume the backoff attempt quota. - Retry ErrorHandlingStrategy = iota - // GiveUp means unrecoverable error happened and the BR should exit + // StrategyRetry error can be retried but will consume the backoff attempt quota. + StrategyRetry ErrorHandlingStrategy = iota + // StrategyGiveUp means unrecoverable error happened and the BR should exit // for example: // 1. permission not valid. // 2. data not found. // 3. retry too many times - GiveUp - // Unknown for Unknown error - Unknown + StrategyGiveUp + // StrategyUnknown for StrategyUnknown error + StrategyUnknown ) type ErrorContext struct { @@ -113,11 +113,11 @@ func NewDefaultContext() *ErrorContext { func HandleBackupError(err *backuppb.Error, storeId uint64, ec *ErrorContext) ErrorHandlingResult { if err == nil { - return ErrorHandlingResult{Retry, unreachableRetryMsg} + return ErrorHandlingResult{StrategyRetry, unreachableRetryMsg} } res := handleBackupProtoError(err) // try the best effort handle unknown error based on their error message - if res.Strategy == Unknown && len(err.Msg) != 0 { + if res.Strategy == StrategyUnknown && len(err.Msg) != 0 { return HandleUnknownBackupError(err.Msg, storeId, ec) } return res @@ -126,13 +126,13 @@ func HandleBackupError(err *backuppb.Error, storeId uint64, ec *ErrorContext) Er func handleBackupProtoError(e *backuppb.Error) ErrorHandlingResult { switch e.Detail.(type) { case *backuppb.Error_KvError: - return ErrorHandlingResult{Retry, retryOnKvErrorMsg} + return ErrorHandlingResult{StrategyRetry, retryOnKvErrorMsg} case *backuppb.Error_RegionError: - return ErrorHandlingResult{Retry, retryOnRegionErrorMsg} + return ErrorHandlingResult{StrategyRetry, retryOnRegionErrorMsg} case *backuppb.Error_ClusterIdError: - return ErrorHandlingResult{GiveUp, clusterIdMismatchMsg} + return ErrorHandlingResult{StrategyGiveUp, clusterIdMismatchMsg} } - return ErrorHandlingResult{Unknown, unknownErrorMsg} + return ErrorHandlingResult{StrategyUnknown, unknownErrorMsg} } // HandleUnknownBackupError UNSAFE! TODO: remove this method and map all the current unknown errors to error types @@ -143,22 +143,22 @@ func HandleUnknownBackupError(msg string, uuid uint64, ec *ErrorContext) ErrorHa reason := fmt.Sprintf("File or directory not found on TiKV Node (store id: %v). "+ "workaround: please ensure br and tikv nodes share a same storage and the user of br and tikv has same uid.", uuid) - return ErrorHandlingResult{GiveUp, reason} + return ErrorHandlingResult{StrategyGiveUp, reason} } if messageIsPermissionDeniedStorageError(msg) { reason := fmt.Sprintf("I/O permission denied error occurs on TiKV Node(store id: %v). "+ "workaround: please ensure tikv has permission to read from & write to the storage.", uuid) - return ErrorHandlingResult{GiveUp, reason} + return ErrorHandlingResult{StrategyGiveUp, reason} } msgLower := strings.ToLower(msg) if strings.Contains(msgLower, contextCancelledMsg) { - return ErrorHandlingResult{GiveUp, contextCancelledMsg} + return ErrorHandlingResult{StrategyGiveUp, contextCancelledMsg} } if MessageIsRetryableStorageError(msg) { logger.Warn(retryableStorageErrorMsg, zap.String("error", msg)) - return ErrorHandlingResult{Retry, retryableStorageErrorMsg} + return ErrorHandlingResult{StrategyRetry, retryableStorageErrorMsg} } // retry enough on same store @@ -166,9 +166,9 @@ func HandleUnknownBackupError(msg string, uuid uint64, ec *ErrorContext) ErrorHa defer ec.mu.Unlock() ec.encounterTimes[uuid]++ if ec.encounterTimes[uuid] <= ec.encounterTimesLimitation { - return ErrorHandlingResult{Retry, retryOnUnknownErrorMsg} + return ErrorHandlingResult{StrategyRetry, retryOnUnknownErrorMsg} } - return ErrorHandlingResult{GiveUp, noRetryOnUnknownErrorMsg} + return ErrorHandlingResult{StrategyGiveUp, noRetryOnUnknownErrorMsg} } // messageIsNotFoundStorageError checks whether the message returning from TiKV is "NotFound" storage I/O error diff --git a/br/pkg/utils/error_handling_test.go b/br/pkg/utils/error_handling_test.go index a4c098f528ada..6b82983843560 100644 --- a/br/pkg/utils/error_handling_test.go +++ b/br/pkg/utils/error_handling_test.go @@ -12,23 +12,23 @@ func TestHandleError(t *testing.T) { ec := NewErrorContext("test", 3) // Test case 1: Error is nil result := HandleBackupError(nil, 123, ec) - require.Equal(t, ErrorHandlingResult{Strategy: Retry, Reason: unreachableRetryMsg}, result) + require.Equal(t, ErrorHandlingResult{Strategy: StrategyRetry, Reason: unreachableRetryMsg}, result) // Test case 2: Error is KvError and can be ignored kvError := &backuppb.Error_KvError{} result = HandleBackupError(&backuppb.Error{Detail: kvError}, 123, ec) - require.Equal(t, ErrorHandlingResult{Strategy: Retry, Reason: retryOnKvErrorMsg}, result) + require.Equal(t, ErrorHandlingResult{Strategy: StrategyRetry, Reason: retryOnKvErrorMsg}, result) // Test case 3: Error is RegionError and can be ignored regionError := &backuppb.Error_RegionError{ RegionError: &errorpb.Error{NotLeader: &errorpb.NotLeader{RegionId: 1}}} result = HandleBackupError(&backuppb.Error{Detail: regionError}, 123, ec) - require.Equal(t, ErrorHandlingResult{Strategy: Retry, Reason: retryOnRegionErrorMsg}, result) + require.Equal(t, ErrorHandlingResult{Strategy: StrategyRetry, Reason: retryOnRegionErrorMsg}, result) // Test case 4: Error is ClusterIdError clusterIdError := &backuppb.Error_ClusterIdError{} result = HandleBackupError(&backuppb.Error{Detail: clusterIdError}, 123, ec) - require.Equal(t, ErrorHandlingResult{Strategy: GiveUp, Reason: clusterIdMismatchMsg}, result) + require.Equal(t, ErrorHandlingResult{Strategy: StrategyGiveUp, Reason: clusterIdMismatchMsg}, result) } func TestHandleErrorMsg(t *testing.T) { @@ -38,33 +38,33 @@ func TestHandleErrorMsg(t *testing.T) { msg := "IO: files Notfound error" uuid := uint64(456) expectedReason := "File or directory not found on TiKV Node (store id: 456). workaround: please ensure br and tikv nodes share a same storage and the user of br and tikv has same uid." - expectedResult := ErrorHandlingResult{Strategy: GiveUp, Reason: expectedReason} + expectedResult := ErrorHandlingResult{Strategy: StrategyGiveUp, Reason: expectedReason} actualResult := HandleUnknownBackupError(msg, uuid, ec) require.Equal(t, expectedResult, actualResult) // Test messageIsPermissionDeniedStorageError msg = "I/O permissiondenied error occurs on TiKV Node(store id: 456)." expectedReason = "I/O permission denied error occurs on TiKV Node(store id: 456). workaround: please ensure tikv has permission to read from & write to the storage." - expectedResult = ErrorHandlingResult{Strategy: GiveUp, Reason: expectedReason} + expectedResult = ErrorHandlingResult{Strategy: StrategyGiveUp, Reason: expectedReason} actualResult = HandleUnknownBackupError(msg, uuid, ec) require.Equal(t, expectedResult, actualResult) // Test MessageIsRetryableStorageError msg = "server closed" - expectedResult = ErrorHandlingResult{Strategy: Retry, Reason: retryableStorageErrorMsg} + expectedResult = ErrorHandlingResult{Strategy: StrategyRetry, Reason: retryableStorageErrorMsg} actualResult = HandleUnknownBackupError(msg, uuid, ec) require.Equal(t, expectedResult, actualResult) // Test unknown error msg = "unknown error" - expectedResult = ErrorHandlingResult{Strategy: Retry, Reason: retryOnUnknownErrorMsg} + expectedResult = ErrorHandlingResult{Strategy: StrategyRetry, Reason: retryOnUnknownErrorMsg} actualResult = HandleUnknownBackupError(msg, uuid, ec) require.Equal(t, expectedResult, actualResult) // Test retry too many times _ = HandleUnknownBackupError(msg, uuid, ec) _ = HandleUnknownBackupError(msg, uuid, ec) - expectedResult = ErrorHandlingResult{Strategy: GiveUp, Reason: noRetryOnUnknownErrorMsg} + expectedResult = ErrorHandlingResult{Strategy: StrategyGiveUp, Reason: noRetryOnUnknownErrorMsg} actualResult = HandleUnknownBackupError(msg, uuid, ec) require.Equal(t, expectedResult, actualResult) }