Skip to content

Commit 50177cd

Browse files
Bigtable: Update retryable logic (#6612) (#12707)
Signed-off-by: Modular Magician <[email protected]> Signed-off-by: Modular Magician <[email protected]>
1 parent ed0ecf7 commit 50177cd

File tree

3 files changed

+49
-12
lines changed

3 files changed

+49
-12
lines changed

.changelog/6612.txt

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:enhancement
2+
bigtable: updated the retryable logic
3+
```

google/error_retry_predicates.go

+21-4
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ import (
88
"net/url"
99
"regexp"
1010
"strings"
11+
"time"
1112

1213
"google.golang.org/api/googleapi"
1314
sqladmin "google.golang.org/api/sqladmin/v1beta4"
15+
"google.golang.org/genproto/googleapis/rpc/errdetails"
16+
"google.golang.org/grpc/codes"
1417
"google.golang.org/grpc/status"
1518
)
1619

@@ -408,13 +411,27 @@ func iamServiceAccountNotFound(err error) (bool, string) {
408411
return false, ""
409412
}
410413

411-
// Big Table uses gRPC and thus does not return errors of type *googleapi.Error.
414+
// Bigtable uses gRPC and thus does not return errors of type *googleapi.Error.
412415
// Instead the errors returned are *status.Error. See the types of codes returned
413416
// here (https://pkg.go.dev/google.golang.org/grpc/codes#Code).
414417
func isBigTableRetryableError(err error) (bool, string) {
415-
statusCode := status.Code(err)
416-
if statusCode.String() == "FailedPrecondition" {
417-
return true, "Waiting for table to be in a valid state"
418+
// The error is retryable if the error code is not OK and has a retry delay.
419+
// The retry delay is currently not used.
420+
if errorStatus, ok := status.FromError(err); ok && errorStatus.Code() != codes.OK {
421+
var retryDelayDuration time.Duration
422+
for _, detail := range errorStatus.Details() {
423+
retryInfo, ok := detail.(*errdetails.RetryInfo)
424+
if !ok {
425+
continue
426+
}
427+
retryDelay := retryInfo.GetRetryDelay()
428+
retryDelayDuration = time.Duration(retryDelay.Seconds)*time.Second + time.Duration(retryDelay.Nanos)*time.Nanosecond
429+
break
430+
}
431+
if retryDelayDuration != 0 {
432+
// TODO: Consider sleep for `retryDelayDuration` before retrying.
433+
return true, "Bigtable operation failed with a retryable error, will retry"
434+
}
418435
}
419436

420437
return false, ""

google/error_retry_predicates_test.go

+25-8
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ import (
55
"testing"
66

77
"google.golang.org/api/googleapi"
8+
"google.golang.org/genproto/googleapis/rpc/errdetails"
89
"google.golang.org/grpc/codes"
910
"google.golang.org/grpc/status"
11+
"google.golang.org/protobuf/types/known/durationpb"
1012
)
1113

1214
func TestIsAppEngineRetryableError_operationInProgress(t *testing.T) {
@@ -114,19 +116,34 @@ func TestIs403QuotaExceededPerMinuteError_perDayQuotaExceededNotRetryable(t *tes
114116
}
115117
}
116118

117-
func TestGRPCRetryable(t *testing.T) {
118-
code := codes.FailedPrecondition
119-
err := status.Error(code, "is retryable")
120-
isRetryable, _ := isBigTableRetryableError(err)
119+
// An error with retry info is retryable.
120+
func TestBigtableError_retryable(t *testing.T) {
121+
retryInfo := &errdetails.RetryInfo{
122+
RetryDelay: &durationpb.Duration{Seconds: 10, Nanos: 10},
123+
}
124+
status, _ := status.New(codes.FailedPrecondition, "is retryable").WithDetails(retryInfo)
125+
isRetryable, _ := isBigTableRetryableError(status.Err())
121126
if !isRetryable {
122127
t.Errorf("Error not detected as retryable")
123128
}
124129
}
125130

126-
func TestGRPCNotRetryable(t *testing.T) {
127-
code := codes.InvalidArgument
128-
err := status.Error(code, "is noto retryable")
129-
isRetryable, _ := isBigTableRetryableError(err)
131+
// An error without retry info is not retryable.
132+
func TestBigtableError_withoutRetryInfoNotRetryable(t *testing.T) {
133+
status := status.New(codes.FailedPrecondition, "is not retryable")
134+
isRetryable, _ := isBigTableRetryableError(status.Err())
135+
if isRetryable {
136+
t.Errorf("Error incorrectly detected as retryable")
137+
}
138+
}
139+
140+
// An OK status with retry info is not retryable.
141+
func TestBigtableError_okIsNotRetryable(t *testing.T) {
142+
retryInfo := &errdetails.RetryInfo{
143+
RetryDelay: &durationpb.Duration{Seconds: 10, Nanos: 10},
144+
}
145+
status, _ := status.New(codes.OK, "is not retryable").WithDetails(retryInfo)
146+
isRetryable, _ := isBigTableRetryableError(status.Err())
130147
if isRetryable {
131148
t.Errorf("Error incorrectly detected as retryable")
132149
}

0 commit comments

Comments
 (0)