retry, client: append errors for backoffer (#7896)

ref #7894 Signed-off-by: Cabinfever_B <[email protected]> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
tikv · Mar 28, 2024 · 2c20897 · 2c20897
1 parent bc92c13
commit 2c20897
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 6 deletions.
diff --git a/client/go.mod b/client/go.mod
@@ -16,6 +16,7 @@ require (
 	github.com/stretchr/testify v1.8.2
 	go.uber.org/atomic v1.10.0
 	go.uber.org/goleak v1.1.11
+	go.uber.org/multierr v1.11.0
 	go.uber.org/zap v1.24.0
 	golang.org/x/exp v0.0.0-20230711005742-c3f37128e5a4
 	google.golang.org/grpc v1.59.0
@@ -33,7 +34,6 @@ require (
 	github.com/prometheus/client_model v0.5.0 // indirect
 	github.com/prometheus/common v0.46.0 // indirect
 	github.com/prometheus/procfs v0.12.0 // indirect
-	go.uber.org/multierr v1.11.0 // indirect
 	golang.org/x/net v0.20.0 // indirect
 	golang.org/x/sys v0.16.0 // indirect
 	golang.org/x/text v0.14.0 // indirect

diff --git a/client/retry/backoff.go b/client/retry/backoff.go
@@ -20,8 +20,11 @@ import (
 
 	"github.com/pingcap/errors"
 	"github.com/pingcap/failpoint"
+	"go.uber.org/multierr"
 )
 
+const maxRecordErrorCount = 20
+
 // Backoffer is a backoff policy for retrying operations.
 type Backoffer struct {
 	// base defines the initial time interval to wait before each retry.
@@ -34,6 +37,7 @@ type Backoffer struct {
 	// By default, all errors are retryable.
 	retryableChecker func(err error) bool
 
+	attempt      int
 	next         time.Duration
 	currentTotal time.Duration
 }
@@ -45,11 +49,16 @@ func (bo *Backoffer) Exec(
 ) error {
 	defer bo.resetBackoff()
 	var (
-		err   error
-		after *time.Timer
+		allErrors error
+		after     *time.Timer
 	)
 	for {
-		err = fn()
+		err := fn()
+		bo.attempt++
+		if bo.attempt < maxRecordErrorCount {
+			// multierr.Append will ignore nil error.
+			allErrors = multierr.Append(allErrors, err)
+		}
 		if !bo.isRetryable(err) {
 			break
 		}
@@ -62,7 +71,7 @@ func (bo *Backoffer) Exec(
 		select {
 		case <-ctx.Done():
 			after.Stop()
-			return errors.Trace(ctx.Err())
+			return multierr.Append(allErrors, errors.Trace(ctx.Err()))
 		case <-after.C:
 			failpoint.Inject("backOffExecute", func() {
 				testBackOffExecuteFlag = true
@@ -77,7 +86,7 @@ func (bo *Backoffer) Exec(
 			}
 		}
 	}
-	return err
+	return allErrors
 }
 
 // InitialBackoffer make the initial state for retrying.
@@ -102,6 +111,7 @@ func InitialBackoffer(base, max, total time.Duration) *Backoffer {
 		},
 		next:         base,
 		currentTotal: 0,
+		attempt:      0,
 	}
 }
 
@@ -141,6 +151,7 @@ func (bo *Backoffer) exponentialInterval() time.Duration {
 func (bo *Backoffer) resetBackoff() {
 	bo.next = bo.base
 	bo.currentTotal = 0
+	bo.attempt = 0
 }
 
 // Only used for test.

diff --git a/client/retry/backoff_test.go b/client/retry/backoff_test.go
@@ -84,6 +84,7 @@ func TestBackoffer(t *testing.T) {
 		return expectedErr
 	})
 	re.InDelta(total, time.Since(start), float64(250*time.Millisecond))
+	re.ErrorContains(err, "test; test; test; test")
 	re.ErrorIs(err, expectedErr)
 	re.Equal(4, execCount)
 	re.True(isBackofferReset(bo))