br: wait more time to wait spitting the region (#42182) (#42314)

close #42001

br/pkg/lightning/backend/local/localhelper_test.go

@@ -480,6 +480,13 @@ func doTestBatchSplitRegionByRanges(ctx context.Context, t *testing.T, hook clie
 		start = end
 	}
 
+	if len(errPat) > 0 {
+		backup := split.ScanRegionAttemptTimes
+		split.ScanRegionAttemptTimes = 3
+		defer func() {
+			split.ScanRegionAttemptTimes = backup
+		}()
+	}
 	err = local.SplitAndScatterRegionByRanges(ctx, ranges, nil, true, 1000)
 	if len(errPat) == 0 {
 		require.NoError(t, err)

br/pkg/restore/split/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "split",
@@ -34,3 +34,17 @@ go_library(
         "@org_uber_go_zap//:zap",
     ],
 )
+
+go_test(
+    name = "split_test",
+    timeout = "short",
+    srcs = ["split_test.go"],
+    flaky = True,
+    deps = [
+        ":split",
+        "//br/pkg/errors",
+        "//br/pkg/utils",
+        "@com_github_pingcap_failpoint//:failpoint",
+        "@com_github_stretchr_testify//require",
+    ],
+)

br/pkg/restore/split/split.go

@@ -18,7 +18,7 @@ import (
 )
 
 var (
-	ScanRegionAttemptTimes = 128
+	ScanRegionAttemptTimes = 150
 )
 
 // Constants for split retry machinery.
@@ -116,40 +116,44 @@ func PaginateScanRegion(
 			return err
 		}
 		return nil
-	}, newScanRegionBackoffer())
+	}, NewScanRegionBackoffer())
 
 	return regions, err
 }
 
 type scanRegionBackoffer struct {
-	attempt int
+	stat utils.RetryState
 }
 
-func newScanRegionBackoffer() utils.Backoffer {
-	attempt := ScanRegionAttemptTimes
-	// only use for test.
-	failpoint.Inject("scanRegionBackoffer", func(val failpoint.Value) {
-		if val.(bool) {
-			attempt = 3
-		}
-	})
+// NewScanRegionBackoffer create a backoff to retry to scan regions.
+func NewScanRegionBackoffer() utils.Backoffer {
 	return &scanRegionBackoffer{
-		attempt: attempt,
+		stat: utils.InitialRetryState(
+			ScanRegionAttemptTimes,
+			time.Millisecond*10,
+			time.Second*2,
+		),
 	}
 }
 
 // NextBackoff returns a duration to wait before retrying again
 func (b *scanRegionBackoffer) NextBackoff(err error) time.Duration {
 	if berrors.ErrPDBatchScanRegion.Equal(err) {
-		// 1s * 60 could be enough for splitting remain regions in the hole.
-		b.attempt--
-		return time.Second
+		// it needs more time to wait splitting the regions that contains data in PITR.
+		// 2s * 150
+		delayTime := b.stat.ExponentialBackoff()
+		failpoint.Inject("hint-scan-region-backoff", func(val failpoint.Value) {
+			if val.(bool) {
+				delayTime = time.Microsecond
+			}
+		})
+		return delayTime
 	}
-	b.attempt = 0
+	b.stat.StopRetry()
 	return 0
 }
 
 // Attempt returns the remain attempt times
 func (b *scanRegionBackoffer) Attempt() int {
-	return b.attempt
+	return b.stat.Attempt()
 }

br/pkg/restore/split/split_test.go

@@ -0,0 +1,73 @@
+// Copyright 2022 PingCAP, Inc. Licensed under Apache-2.0.
+package split_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/pingcap/failpoint"
+	berrors "github.com/pingcap/tidb/br/pkg/errors"
+	"github.com/pingcap/tidb/br/pkg/restore/split"
+	"github.com/pingcap/tidb/br/pkg/utils"
+	"github.com/stretchr/testify/require"
+)
+
+func TestScanRegionBackOfferWithSuccess(t *testing.T) {
+	var counter int
+	bo := split.NewScanRegionBackoffer()
+
+	err := utils.WithRetry(context.Background(), func() error {
+		defer func() {
+			counter++
+		}()
+
+		if counter == 3 {
+			return nil
+		}
+		return berrors.ErrPDBatchScanRegion
+	}, bo)
+	require.NoError(t, err)
+	require.Equal(t, counter, 4)
+}
+
+func TestScanRegionBackOfferWithFail(t *testing.T) {
+	_ = failpoint.Enable("github.com/pingcap/tidb/br/pkg/restore/split/hint-scan-region-backoff", "return(true)")
+	defer func() {
+		_ = failpoint.Disable("github.com/pingcap/tidb/br/pkg/restore/split/hint-scan-region-backoff")
+	}()
+
+	var counter int
+	bo := split.NewScanRegionBackoffer()
+
+	err := utils.WithRetry(context.Background(), func() error {
+		defer func() {
+			counter++
+		}()
+		return berrors.ErrPDBatchScanRegion
+	}, bo)
+	require.Error(t, err)
+	require.Equal(t, counter, split.ScanRegionAttemptTimes)
+}
+
+func TestScanRegionBackOfferWithStopRetry(t *testing.T) {
+	_ = failpoint.Enable("github.com/pingcap/tidb/br/pkg/restore/split/hint-scan-region-backoff", "return(true)")
+	defer func() {
+		_ = failpoint.Disable("github.com/pingcap/tidb/br/pkg/restore/split/hint-scan-region-backoff")
+	}()
+
+	var counter int
+	bo := split.NewScanRegionBackoffer()
+
+	err := utils.WithRetry(context.Background(), func() error {
+		defer func() {
+			counter++
+		}()
+
+		if counter < 5 {
+			return berrors.ErrPDBatchScanRegion
+		}
+		return berrors.ErrKVUnknown
+	}, bo)
+	require.Error(t, err)
+	require.Equal(t, counter, 6)
+}

br/pkg/restore/util_test.go

@@ -7,7 +7,6 @@ import (
 	"encoding/binary"
 	"testing"
 
-	"github.com/pingcap/failpoint"
 	backuppb "github.com/pingcap/kvproto/pkg/brpb"
 	"github.com/pingcap/kvproto/pkg/import_sstpb"
 	"github.com/pingcap/kvproto/pkg/metapb"
@@ -230,7 +229,11 @@ func TestPaginateScanRegion(t *testing.T) {
 	regionMap := make(map[uint64]*split.RegionInfo)
 	var regions []*split.RegionInfo
 	var batch []*split.RegionInfo
-	require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/br/pkg/restore/split/scanRegionBackoffer", "return(true)"))
+	backup := split.ScanRegionAttemptTimes
+	split.ScanRegionAttemptTimes = 3
+	defer func() {
+		split.ScanRegionAttemptTimes = backup
+	}()
 	_, err := split.PaginateScanRegion(ctx, NewTestClient(stores, regionMap, 0), []byte{}, []byte{}, 3)
 	require.Error(t, err)
 	require.True(t, berrors.ErrPDBatchScanRegion.Equal(err))
@@ -293,8 +296,6 @@ func TestPaginateScanRegion(t *testing.T) {
 	require.Error(t, err)
 	require.True(t, berrors.ErrPDBatchScanRegion.Equal(err))
 	require.Regexp(t, ".*region endKey not equal to next region startKey.*", err.Error())
-
-	require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/br/pkg/restore/split/scanRegionBackoffer"))
 }
 
 func TestRewriteFileKeys(t *testing.T) {

br/pkg/utils/backoff.go

@@ -94,6 +94,10 @@ func (rs *RetryState) Attempt() int {
 	return rs.maxRetry - rs.retryTimes
 }
 
+func (rs *RetryState) StopRetry() {
+	rs.retryTimes = rs.maxRetry
+}
+
 // NextBackoff implements the `Backoffer`.
 func (rs *RetryState) NextBackoff(error) time.Duration {
 	return rs.ExponentialBackoff()