Skip to content

Commit

Permalink
Dumpling: GCS client always retry and also retry for 401 error. (#58198)
Browse files Browse the repository at this point in the history
close #56127
  • Loading branch information
OliverS929 authored Dec 12, 2024
1 parent 1a3cb4c commit 0b9e7f6
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
1 change: 1 addition & 0 deletions br/pkg/storage/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ go_library(
"@com_github_pingcap_log//:log",
"@com_github_spf13_pflag//:pflag",
"@com_google_cloud_go_storage//:storage",
"@org_golang_google_api//googleapi",
"@org_golang_google_api//iterator",
"@org_golang_google_api//option",
"@org_golang_x_oauth2//google",
Expand Down
21 changes: 21 additions & 0 deletions br/pkg/storage/gcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package storage
import (
"bytes"
"context"
goerrors "errors"
"io"
"os"
"path"
Expand All @@ -13,9 +14,12 @@ import (
"cloud.google.com/go/storage"
"github.com/pingcap/errors"
backuppb "github.com/pingcap/kvproto/pkg/brpb"
"github.com/pingcap/log"
berrors "github.com/pingcap/tidb/br/pkg/errors"
"github.com/spf13/pflag"
"go.uber.org/zap"
"golang.org/x/oauth2/google"
"google.golang.org/api/googleapi"
"google.golang.org/api/iterator"
"google.golang.org/api/option"
)
Expand Down Expand Up @@ -327,6 +331,7 @@ func NewGCSStorage(ctx context.Context, gcs *backuppb.GCS, opts *ExternalStorage
if err != nil {
return nil, errors.Trace(err)
}
client.SetRetry(storage.WithErrorFunc(shouldRetry), storage.WithPolicy(storage.RetryAlways))

if !opts.SendCredentials {
// Clear the credentials if exists so that they will not be sent to TiKV
Expand All @@ -337,6 +342,22 @@ func NewGCSStorage(ctx context.Context, gcs *backuppb.GCS, opts *ExternalStorage
return &GCSStorage{gcs: gcs, bucket: bucket}, nil
}

func shouldRetry(err error) bool {
if storage.ShouldRetry(err) {
return true
}

// workaround for https://github.com/googleapis/google-cloud-go/issues/9262
if e := (&googleapi.Error{}); goerrors.As(err, &e) {
if e.Code == 401 && strings.Contains(e.Message, "Authentication required.") {
log.Warn("retrying gcs request due to internal authentication error", zap.Error(err))
return true
}
}

return false
}

// gcsObjectReader wrap storage.Reader and add the `Seek` method.
type gcsObjectReader struct {
storage *GCSStorage
Expand Down

0 comments on commit 0b9e7f6

Please sign in to comment.