Revert "br: Add retry limit of connection err in fine grained backup … (pingcap#58503)

closes pingcap#57449

Author: YuJuncen

br/pkg/backup/BUILD.bazel

@@ -72,10 +72,9 @@ go_test(
     embed = [":backup"],
     flaky = True,
     race = "on",
-    shard_count = 14,
+    shard_count = 13,
     deps = [
         "//br/pkg/conn",
-        "//br/pkg/errors",
         "//br/pkg/gluetidb",
         "//br/pkg/metautil",
         "//br/pkg/mock",
@@ -92,7 +91,6 @@ go_test(
         "//pkg/util/codec",
         "//pkg/util/table-filter",
         "@com_github_golang_protobuf//proto",
-        "@com_github_pingcap_errors//:errors",
         "@com_github_pingcap_kvproto//pkg/brpb",
         "@com_github_pingcap_kvproto//pkg/encryptionpb",
         "@com_github_pingcap_kvproto//pkg/errorpb",
@@ -105,6 +103,5 @@ go_test(
         "@io_opencensus_go//stats/view",
         "@org_golang_google_grpc//:grpc",
         "@org_uber_go_goleak//:goleak",
-        "@org_uber_go_multierr//:multierr",
     ],
 )

br/pkg/backup/client.go

@@ -77,50 +77,12 @@ type Checksum struct {
 // ProgressUnit represents the unit of progress.
 type ProgressUnit string
 
-func MakeStoreBasedErr(storeID uint64, err error) *StoreBasedErr {
-	return &StoreBasedErr{storeID: storeID, err: err}
-}
-
-type StoreBasedErr struct {
-	storeID uint64
-	err     error
-}
-
-func (e *StoreBasedErr) Error() string {
-	return fmt.Sprintf("Store ID '%d': %v", e.storeID, e.err.Error())
-}
-
-func (e *StoreBasedErr) Unwrap() error {
-	return e.err
-}
-
-func (e *StoreBasedErr) Cause() error {
-	return e.Unwrap()
-}
-
-// Errors implements errors.ErrorGroup.
-// For now `WalkDeep` cannot walk "subtree"s like:
-/* 1 - 2 - 5
- *   |
- *   + 3 - 4
- */
-// It stops after walking `1` and then gave up.
-// This is a bug: see https://github.com/pingcap/errors/issues/72
-// We manually make this a multierr to workaround this...
-func (e *StoreBasedErr) Errors() []error {
-	if errs, ok := e.err.(errors.ErrorGroup); ok {
-		return errs.Errors()
-	}
-	return nil
-}
-
 const (
 	// backupFineGrainedMaxBackoff is 1 hour.
 	// given it begins the fine-grained backup, there must be some problems in the cluster.
 	// We need to be more patient.
 	backupFineGrainedMaxBackoff = 3600000
 	backupRetryTimes            = 5
-	disconnectRetryTimeout      = 20000
 	// RangeUnit represents the progress updated counter when a range finished.
 	RangeUnit ProgressUnit = "range"
 	// RegionUnit represents the progress updated counter when a region finished.
@@ -1160,7 +1122,6 @@ func (bc *Client) fineGrainedBackup(
 	})
 
 	bo := utils.AdaptTiKVBackoffer(ctx, backupFineGrainedMaxBackoff, berrors.ErrUnknown)
-	maxDisconnect := make(map[uint64]uint)
 	for {
 		// Step1, check whether there is any incomplete range
 		incomplete := pr.Res.GetIncompleteRange(req.StartKey, req.EndKey)
@@ -1208,19 +1169,8 @@ func (bc *Client) fineGrainedBackup(
 		for {
 			select {
 			case err := <-errCh:
-				if !berrors.Is(err, berrors.ErrFailedToConnect) {
-					return errors.Trace(err)
-				}
-				storeErr, ok := err.(*StoreBasedErr)
-				if !ok {
-					return errors.Trace(err)
-				}
-
-				storeID := storeErr.storeID
-				maxDisconnect[storeID]++
-				if maxDisconnect[storeID] > backupRetryTimes {
-					return errors.Annotatef(err, "Failed to connect to store %d more than %d times", storeID, backupRetryTimes)
-				}
+				// TODO: should we handle err here?
+				return errors.Trace(err)
 			case resp, ok := <-respCh:
 				if !ok {
 					// Finished.
@@ -1321,22 +1271,12 @@ func (bc *Client) handleFineGrained(
 	storeID := targetPeer.GetStoreId()
 	lockResolver := bc.mgr.GetLockResolver()
 	client, err := bc.mgr.GetBackupClient(ctx, storeID)
-
-	// inject a disconnect failpoint
-	failpoint.Inject("disconnect", func(_ failpoint.Value) {
-		logutil.CL(ctx).Warn("This is a injected disconnection error")
-		err = berrors.ErrFailedToConnect
-	})
-
 	if err != nil {
 		if berrors.Is(err, berrors.ErrFailedToConnect) {
 			// When the leader store is died,
 			// 20s for the default max duration before the raft election timer fires.
 			logutil.CL(ctx).Warn("failed to connect to store, skipping", logutil.ShortError(err), zap.Uint64("storeID", storeID))
-			return disconnectRetryTimeout, &StoreBasedErr{
-				storeID: storeID,
-				err:     err,
-			}
+			return 20000, nil
 		}
 
 		logutil.CL(ctx).Error("fail to connect store", zap.Uint64("StoreID", storeID))
@@ -1375,10 +1315,7 @@ func (bc *Client) handleFineGrained(
 			// When the leader store is died,
 			// 20s for the default max duration before the raft election timer fires.
 			logutil.CL(ctx).Warn("failed to connect to store, skipping", logutil.ShortError(err), zap.Uint64("storeID", storeID))
-			return disconnectRetryTimeout, &StoreBasedErr{
-				storeID: storeID,
-				err:     err,
-			}
+			return 20000, nil
 		}
 		logutil.CL(ctx).Error("failed to send fine-grained backup", zap.Uint64("storeID", storeID), logutil.ShortError(err))
 		return 0, errors.Annotatef(err, "failed to send fine-grained backup [%s, %s)",

br/pkg/backup/client_test.go

@@ -10,13 +10,11 @@ import (
 	"time"
 
 	"github.com/golang/protobuf/proto"
-	"github.com/pingcap/errors"
 	backuppb "github.com/pingcap/kvproto/pkg/brpb"
 	"github.com/pingcap/kvproto/pkg/encryptionpb"
 	"github.com/pingcap/kvproto/pkg/errorpb"
 	"github.com/pingcap/tidb/br/pkg/backup"
 	"github.com/pingcap/tidb/br/pkg/conn"
-	berrors "github.com/pingcap/tidb/br/pkg/errors"
 	"github.com/pingcap/tidb/br/pkg/gluetidb"
 	"github.com/pingcap/tidb/br/pkg/metautil"
 	"github.com/pingcap/tidb/br/pkg/mock"
@@ -36,7 +34,6 @@ import (
 	"github.com/tikv/client-go/v2/txnkv/txnlock"
 	pd "github.com/tikv/pd/client"
 	"go.opencensus.io/stats/view"
-	"go.uber.org/multierr"
 )
 
 type testBackup struct {
@@ -395,11 +392,3 @@ func TestCheckBackupIsLocked(t *testing.T) {
 	require.Error(t, err)
 	require.Regexp(t, "backup lock file and sst file exist in(.+)", err.Error())
 }
-
-func TestErr(t *testing.T) {
-	serr := backup.MakeStoreBasedErr(42, multierr.Combine(
-		errors.Annotate(berrors.ErrFailedToConnect, "oops"),
-		berrors.ErrFailedToConnect.GenWithStack("whoa")))
-	require.True(t, berrors.Is(serr, berrors.ErrFailedToConnect))
-	require.False(t, berrors.Is(serr, berrors.ErrUnknown))
-}