diff --git a/.changelog/6612.txt b/.changelog/6612.txt new file mode 100644 index 00000000000..d301a6317d6 --- /dev/null +++ b/.changelog/6612.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +bigtable: updated the retryable logic +``` diff --git a/google/error_retry_predicates.go b/google/error_retry_predicates.go index abb26b2816c..b65b312a0ca 100644 --- a/google/error_retry_predicates.go +++ b/google/error_retry_predicates.go @@ -8,9 +8,12 @@ import ( "net/url" "regexp" "strings" + "time" "google.golang.org/api/googleapi" sqladmin "google.golang.org/api/sqladmin/v1beta4" + "google.golang.org/genproto/googleapis/rpc/errdetails" + "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -408,13 +411,27 @@ func iamServiceAccountNotFound(err error) (bool, string) { return false, "" } -// Big Table uses gRPC and thus does not return errors of type *googleapi.Error. +// Bigtable uses gRPC and thus does not return errors of type *googleapi.Error. // Instead the errors returned are *status.Error. See the types of codes returned // here (https://pkg.go.dev/google.golang.org/grpc/codes#Code). func isBigTableRetryableError(err error) (bool, string) { - statusCode := status.Code(err) - if statusCode.String() == "FailedPrecondition" { - return true, "Waiting for table to be in a valid state" + // The error is retryable if the error code is not OK and has a retry delay. + // The retry delay is currently not used. + if errorStatus, ok := status.FromError(err); ok && errorStatus.Code() != codes.OK { + var retryDelayDuration time.Duration + for _, detail := range errorStatus.Details() { + retryInfo, ok := detail.(*errdetails.RetryInfo) + if !ok { + continue + } + retryDelay := retryInfo.GetRetryDelay() + retryDelayDuration = time.Duration(retryDelay.Seconds)*time.Second + time.Duration(retryDelay.Nanos)*time.Nanosecond + break + } + if retryDelayDuration != 0 { + // TODO: Consider sleep for `retryDelayDuration` before retrying. + return true, "Bigtable operation failed with a retryable error, will retry" + } } return false, "" diff --git a/google/error_retry_predicates_test.go b/google/error_retry_predicates_test.go index f34a0445659..464ec52927c 100644 --- a/google/error_retry_predicates_test.go +++ b/google/error_retry_predicates_test.go @@ -5,8 +5,10 @@ import ( "testing" "google.golang.org/api/googleapi" + "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/durationpb" ) func TestIsAppEngineRetryableError_operationInProgress(t *testing.T) { @@ -114,19 +116,34 @@ func TestIs403QuotaExceededPerMinuteError_perDayQuotaExceededNotRetryable(t *tes } } -func TestGRPCRetryable(t *testing.T) { - code := codes.FailedPrecondition - err := status.Error(code, "is retryable") - isRetryable, _ := isBigTableRetryableError(err) +// An error with retry info is retryable. +func TestBigtableError_retryable(t *testing.T) { + retryInfo := &errdetails.RetryInfo{ + RetryDelay: &durationpb.Duration{Seconds: 10, Nanos: 10}, + } + status, _ := status.New(codes.FailedPrecondition, "is retryable").WithDetails(retryInfo) + isRetryable, _ := isBigTableRetryableError(status.Err()) if !isRetryable { t.Errorf("Error not detected as retryable") } } -func TestGRPCNotRetryable(t *testing.T) { - code := codes.InvalidArgument - err := status.Error(code, "is noto retryable") - isRetryable, _ := isBigTableRetryableError(err) +// An error without retry info is not retryable. +func TestBigtableError_withoutRetryInfoNotRetryable(t *testing.T) { + status := status.New(codes.FailedPrecondition, "is not retryable") + isRetryable, _ := isBigTableRetryableError(status.Err()) + if isRetryable { + t.Errorf("Error incorrectly detected as retryable") + } +} + +// An OK status with retry info is not retryable. +func TestBigtableError_okIsNotRetryable(t *testing.T) { + retryInfo := &errdetails.RetryInfo{ + RetryDelay: &durationpb.Duration{Seconds: 10, Nanos: 10}, + } + status, _ := status.New(codes.OK, "is not retryable").WithDetails(retryInfo) + isRetryable, _ := isBigTableRetryableError(status.Err()) if isRetryable { t.Errorf("Error incorrectly detected as retryable") }