From bfdf24b84aa4b5f14a33f9dc59b6cea7d5804b83 Mon Sep 17 00:00:00 2001 From: Cabinfever_B Date: Thu, 7 Mar 2024 18:38:07 +0800 Subject: [PATCH 1/4] append errors Signed-off-by: Cabinfever_B --- client/retry/backoff.go | 12 ++++++++---- client/retry/backoff_test.go | 1 + 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/client/retry/backoff.go b/client/retry/backoff.go index e79d0e3e4eb..ed6c088b18d 100644 --- a/client/retry/backoff.go +++ b/client/retry/backoff.go @@ -20,6 +20,7 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/failpoint" + "go.uber.org/multierr" ) // Backoffer is a backoff policy for retrying operations. @@ -45,11 +46,14 @@ func (bo *Backoffer) Exec( ) error { defer bo.resetBackoff() var ( - err error - after *time.Timer + allErrors error + after *time.Timer ) for { - err = fn() + err := fn() + if err != nil { + allErrors = multierr.Append(allErrors, err) + } if !bo.isRetryable(err) { break } @@ -77,7 +81,7 @@ func (bo *Backoffer) Exec( } } } - return err + return allErrors } // InitialBackoffer make the initial state for retrying. diff --git a/client/retry/backoff_test.go b/client/retry/backoff_test.go index 3dd983f2afa..32a42d083bd 100644 --- 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)) From 26ae0b56987cb98dfa5927e9222e9d6423196e95 Mon Sep 17 00:00:00 2001 From: Cabinfever_B Date: Thu, 7 Mar 2024 20:02:35 +0800 Subject: [PATCH 2/4] support log Signed-off-by: Cabinfever_B --- client/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/go.mod b/client/go.mod index 6a9d29a3184..9b2cb87f75e 100644 --- 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 From 2181fc7f02886e5017c3d21af3e65acd9ef8a079 Mon Sep 17 00:00:00 2001 From: Cabinfever_B Date: Mon, 11 Mar 2024 23:51:54 +0800 Subject: [PATCH 3/4] address comment Signed-off-by: Cabinfever_B --- client/retry/backoff.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/client/retry/backoff.go b/client/retry/backoff.go index ed6c088b18d..3cfc957c73c 100644 --- a/client/retry/backoff.go +++ b/client/retry/backoff.go @@ -23,6 +23,8 @@ import ( "go.uber.org/multierr" ) +const maxRecordErrorCount = 20 - 1 + // Backoffer is a backoff policy for retrying operations. type Backoffer struct { // base defines the initial time interval to wait before each retry. @@ -35,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 } @@ -51,7 +54,9 @@ func (bo *Backoffer) Exec( ) for { err := fn() - if err != nil { + bo.attempt++ + if bo.attempt < maxRecordErrorCount { + // multierr.Append will ignore nil error. allErrors = multierr.Append(allErrors, err) } if !bo.isRetryable(err) { @@ -66,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 @@ -106,6 +111,7 @@ func InitialBackoffer(base, max, total time.Duration) *Backoffer { }, next: base, currentTotal: 0, + attempt: 0, } } @@ -145,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. From afb5af25a3acea1b78df73d47c75fd413d7d4ac7 Mon Sep 17 00:00:00 2001 From: Cabinfever_B Date: Tue, 12 Mar 2024 10:33:08 +0800 Subject: [PATCH 4/4] address comment Signed-off-by: Cabinfever_B --- client/retry/backoff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/retry/backoff.go b/client/retry/backoff.go index 3cfc957c73c..6c293098971 100644 --- a/client/retry/backoff.go +++ b/client/retry/backoff.go @@ -23,7 +23,7 @@ import ( "go.uber.org/multierr" ) -const maxRecordErrorCount = 20 - 1 +const maxRecordErrorCount = 20 // Backoffer is a backoff policy for retrying operations. type Backoffer struct {