Skip to content

Commit

Permalink
lightning: always retry from rescan when NotLeader (#43351) (#45135)
Browse files Browse the repository at this point in the history
close #43055
  • Loading branch information
ti-chi-bot authored Jul 4, 2023
1 parent 3034205 commit 887d342
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 21 deletions.
19 changes: 2 additions & 17 deletions br/pkg/lightning/backend/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,7 @@ WriteAndIngest:
local.ingestConcurrency.Recycle(w)
if err != nil {
if !local.isRetryableImportTiKVError(err) {
return err
return errors.Trace(err)
}
_, regionStart, _ := codec.DecodeBytes(region.Region.StartKey, []byte{})
// if we have at least succeeded one region, retry without increasing the retry count
Expand Down Expand Up @@ -1792,22 +1792,7 @@ func (local *local) isIngestRetryable(
var err error
switch errPb := resp.GetError(); {
case errPb.NotLeader != nil:
if newLeader := errPb.GetNotLeader().GetLeader(); newLeader != nil {
newRegion = &split.RegionInfo{
Leader: newLeader,
Region: region.Region,
}
} else {
newRegion, err = getRegion()
if err != nil {
return retryNone, nil, errors.Trace(err)
}
}
// TODO: because in some case, TiKV may return retryable error while the ingest is succeeded.
// Thus directly retry ingest may cause TiKV panic. So always return retryWrite here to avoid
// this issue.
// See: https://github.com/tikv/tikv/issues/9496
return retryWrite, newRegion, common.ErrKVNotLeader.GenWithStack(errPb.GetMessage())
return retryNone, nil, common.ErrKVNotLeader.GenWithStack(errPb.GetMessage())
case errPb.EpochNotMatch != nil:
if currentRegions := errPb.GetEpochNotMatch().GetCurrentRegions(); currentRegions != nil {
var currentRegion *metapb.Region
Expand Down
7 changes: 3 additions & 4 deletions br/pkg/lightning/backend/local/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,8 @@ func TestIsIngestRetryable(t *testing.T) {
},
},
}
retryType, newRegion, err := local.isIngestRetryable(ctx, resp, region, metas)
require.Equal(t, retryWrite, retryType)
require.Equal(t, uint64(2), newRegion.Leader.Id)
retryType, _, err := local.isIngestRetryable(ctx, resp, region, metas)
require.Equal(t, retryNone, retryType)
require.Error(t, err)

resp.Error = &errorpb.Error{
Expand All @@ -509,7 +508,7 @@ func TestIsIngestRetryable(t *testing.T) {
},
},
}
retryType, newRegion, err = local.isIngestRetryable(ctx, resp, region, metas)
retryType, newRegion, err := local.isIngestRetryable(ctx, resp, region, metas)
require.Equal(t, retryWrite, retryType)
require.Equal(t, uint64(2), newRegion.Region.RegionEpoch.Version)
require.Error(t, err)
Expand Down

0 comments on commit 887d342

Please sign in to comment.