From c9f981f38bfb9e345aae070151311f40a4a65118 Mon Sep 17 00:00:00 2001 From: joccau Date: Tue, 7 Mar 2023 16:37:29 +0800 Subject: [PATCH 1/3] continuing retry when pitr met the error about memory is limited Signed-off-by: joccau --- br/pkg/restore/import_retry.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/br/pkg/restore/import_retry.go b/br/pkg/restore/import_retry.go index 6f3b9fc1cca53..2293b5000fdc0 100644 --- a/br/pkg/restore/import_retry.go +++ b/br/pkg/restore/import_retry.go @@ -4,6 +4,7 @@ package restore import ( "context" + "strings" "time" "github.com/pingcap/errors" @@ -85,6 +86,13 @@ func (o *OverRegionsInRangeController) tryFindLeader(ctx context.Context, region // handleInRegionError handles the error happens internal in the region. Update the region info, and perform a suitable backoff. func (o *OverRegionsInRangeController) handleInRegionError(ctx context.Context, result RPCResult, region *split.RegionInfo) (cont bool) { + if result.StoreError.GetServerIsBusy() != nil { + if strings.Contains(result.StoreError.GetMessage(), "memory is limited") { + time.Sleep(15 * time.Second) + return true + } + } + if nl := result.StoreError.GetNotLeader(); nl != nil { if nl.Leader != nil { region.Leader = nl.Leader From 83a2951ba944e46d838dbb066a2cec74f771efda Mon Sep 17 00:00:00 2001 From: joccau Date: Tue, 7 Mar 2023 17:11:21 +0800 Subject: [PATCH 2/3] add unit test Signed-off-by: joccau --- br/pkg/restore/import_retry.go | 11 +++++++- br/pkg/restore/import_retry_test.go | 43 +++++++++++++++++++++++++++++ br/pkg/utils/backoff.go | 6 ++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/br/pkg/restore/import_retry.go b/br/pkg/restore/import_retry.go index 2293b5000fdc0..c707ea9377dee 100644 --- a/br/pkg/restore/import_retry.go +++ b/br/pkg/restore/import_retry.go @@ -8,6 +8,7 @@ import ( "time" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/errorpb" "github.com/pingcap/kvproto/pkg/import_sstpb" "github.com/pingcap/kvproto/pkg/metapb" @@ -88,7 +89,15 @@ func (o *OverRegionsInRangeController) tryFindLeader(ctx context.Context, region func (o *OverRegionsInRangeController) handleInRegionError(ctx context.Context, result RPCResult, region *split.RegionInfo) (cont bool) { if result.StoreError.GetServerIsBusy() != nil { if strings.Contains(result.StoreError.GetMessage(), "memory is limited") { - time.Sleep(15 * time.Second) + sleepDuration := 15 * time.Second + + failpoint.Inject("hint-memoryIsLimited-sleep", func(val failpoint.Value) { + if val.(bool) { + logutil.CL(ctx).Debug("failpoint hint-memoryIsLimited-sleep injected.") + sleepDuration = 100 * time.Microsecond + } + }) + time.Sleep(sleepDuration) return true } } diff --git a/br/pkg/restore/import_retry_test.go b/br/pkg/restore/import_retry_test.go index 6f3d8f490ef13..98be3f7b08ea4 100644 --- a/br/pkg/restore/import_retry_test.go +++ b/br/pkg/restore/import_retry_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" backuppb "github.com/pingcap/kvproto/pkg/brpb" "github.com/pingcap/kvproto/pkg/errorpb" "github.com/pingcap/kvproto/pkg/import_sstpb" @@ -163,6 +164,48 @@ func TestServerIsBusy(t *testing.T) { require.NoError(t, err) assertRegions(t, idEqualsTo2Regions, "aay", "bba") assertRegions(t, meetRegions, "", "aay", "bba", "bbh", "cca", "") + require.Equal(t, rs.RetryTimes(), 1) +} + +func TestServerIsBusyWithMemoryIsLimited(t *testing.T) { + _ = failpoint.Enable("github.com/pingcap/tidb/br/pkg/restore/hint-memoryIsLimited-sleep", "return(true)") + defer func() { + _ = failpoint.Disable("github.com/pingcap/tidb/br/pkg/restore/hint-memoryIsLimited-sleep") + }() + + // region: [, aay), [aay, bba), [bba, bbh), [bbh, cca), [cca, ) + cli := initTestClient(false) + rs := utils.InitialRetryState(2, 0, 0) + ctl := restore.OverRegionsInRange([]byte(""), []byte(""), cli, &rs) + ctx := context.Background() + + serverIsBusy := errorpb.Error{ + Message: "memory is limited", + ServerIsBusy: &errorpb.ServerIsBusy{ + Reason: "", + }, + } + // record the regions we didn't touch. + meetRegions := []*split.RegionInfo{} + // record all regions we meet with id == 2. + idEqualsTo2Regions := []*split.RegionInfo{} + theFirstRun := true + err := ctl.Run(ctx, func(ctx context.Context, r *split.RegionInfo) restore.RPCResult { + if theFirstRun && r.Region.Id == 2 { + idEqualsTo2Regions = append(idEqualsTo2Regions, r) + theFirstRun = false + return restore.RPCResult{ + StoreError: &serverIsBusy, + } + } + meetRegions = append(meetRegions, r) + return restore.RPCResultOK() + }) + + require.NoError(t, err) + assertRegions(t, idEqualsTo2Regions, "aay", "bba") + assertRegions(t, meetRegions, "", "aay", "bba", "bbh", "cca", "") + require.Equal(t, rs.RetryTimes(), 0) } func printRegion(name string, infos []*split.RegionInfo) { diff --git a/br/pkg/utils/backoff.go b/br/pkg/utils/backoff.go index 8db70b2a695db..5f19bc6a29078 100644 --- a/br/pkg/utils/backoff.go +++ b/br/pkg/utils/backoff.go @@ -86,6 +86,12 @@ func (rs *RetryState) RecordRetry() { rs.retryTimes++ } +// RetryTimes returns the retry times. +// usage: unit test. +func (rs *RetryState) RetryTimes() int { + return rs.retryTimes +} + // Attempt implements the `Backoffer`. // TODO: Maybe use this to replace the `exponentialBackoffer` (which is nearly homomorphic to this)? func (rs *RetryState) Attempt() int { From 893f76ffcb7e40daad3ffb146292b5d24ef90ec7 Mon Sep 17 00:00:00 2001 From: joccau Date: Tue, 7 Mar 2023 18:25:08 +0800 Subject: [PATCH 3/3] rename failpoint string Signed-off-by: joccau --- br/pkg/restore/import_retry.go | 4 ++-- br/pkg/restore/import_retry_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/br/pkg/restore/import_retry.go b/br/pkg/restore/import_retry.go index c707ea9377dee..4a6bdf1b8afcf 100644 --- a/br/pkg/restore/import_retry.go +++ b/br/pkg/restore/import_retry.go @@ -91,9 +91,9 @@ func (o *OverRegionsInRangeController) handleInRegionError(ctx context.Context, if strings.Contains(result.StoreError.GetMessage(), "memory is limited") { sleepDuration := 15 * time.Second - failpoint.Inject("hint-memoryIsLimited-sleep", func(val failpoint.Value) { + failpoint.Inject("hint-memory-is-limited", func(val failpoint.Value) { if val.(bool) { - logutil.CL(ctx).Debug("failpoint hint-memoryIsLimited-sleep injected.") + logutil.CL(ctx).Debug("failpoint hint-memory-is-limited injected.") sleepDuration = 100 * time.Microsecond } }) diff --git a/br/pkg/restore/import_retry_test.go b/br/pkg/restore/import_retry_test.go index 98be3f7b08ea4..4e885657f998f 100644 --- a/br/pkg/restore/import_retry_test.go +++ b/br/pkg/restore/import_retry_test.go @@ -168,9 +168,9 @@ func TestServerIsBusy(t *testing.T) { } func TestServerIsBusyWithMemoryIsLimited(t *testing.T) { - _ = failpoint.Enable("github.com/pingcap/tidb/br/pkg/restore/hint-memoryIsLimited-sleep", "return(true)") + _ = failpoint.Enable("github.com/pingcap/tidb/br/pkg/restore/hint-memory-is-limited", "return(true)") defer func() { - _ = failpoint.Disable("github.com/pingcap/tidb/br/pkg/restore/hint-memoryIsLimited-sleep") + _ = failpoint.Disable("github.com/pingcap/tidb/br/pkg/restore/hint-memory-is-limited") }() // region: [, aay), [aay, bba), [bba, bbh), [bbh, cca), [cca, )