Skip to content

Commit

Permalink
rename enum to be more meaningful
Browse files Browse the repository at this point in the history
Signed-off-by: Wenqi Mou <[email protected]>
  • Loading branch information
Tristan1900 committed Jul 22, 2024
1 parent 6b994c9 commit 7e3a7e9
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 29 deletions.
2 changes: 1 addition & 1 deletion br/pkg/backup/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion br/pkg/utils/backoff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
36 changes: 18 additions & 18 deletions br/pkg/utils/error_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -143,32 +143,32 @@ 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
ec.mu.Lock()
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
Expand Down
18 changes: 9 additions & 9 deletions br/pkg/utils/error_handling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
}

0 comments on commit 7e3a7e9

Please sign in to comment.