backend/local: fallback retryIngest to retryWrite (#554)

lightning/backend/local.go

@@ -1394,7 +1394,11 @@ func (local *local) isIngestRetryable(
 				return retryNone, nil, errors.Trace(err)
 			}
 		}
-		return retryIngest, newRegion, errors.Errorf("not leader: %s", errPb.GetMessage())
+		// 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, errors.Errorf("not leader: %s", errPb.GetMessage())
 	case errPb.EpochNotMatch != nil:
 		if currentRegions := errPb.GetEpochNotMatch().GetCurrentRegions(); currentRegions != nil {
 			var currentRegion *metapb.Region
@@ -1431,7 +1435,7 @@ func (local *local) isIngestRetryable(
 		if err != nil {
 			return retryNone, nil, errors.Trace(err)
 		}
-		return retryIngest, newRegion, errors.New(errPb.GetMessage())
+		return retryWrite, newRegion, errors.New(errPb.GetMessage())
 	}
 	return retryNone, nil, errors.Errorf("non-retryable error: %s", resp.GetError().GetMessage())
 }

lightning/backend/local_test.go

@@ -23,16 +23,20 @@ import (
 	"path/filepath"
 	"sort"
 
-	"github.com/pingcap/tidb-lightning/lightning/common"
-
 	"github.com/cockroachdb/pebble"
+	"github.com/pingcap/br/pkg/restore"
 	. "github.com/pingcap/check"
+	"github.com/pingcap/kvproto/pkg/errorpb"
+	sst "github.com/pingcap/kvproto/pkg/import_sstpb"
+	"github.com/pingcap/kvproto/pkg/metapb"
 	"github.com/pingcap/tidb/kv"
 	"github.com/pingcap/tidb/sessionctx/stmtctx"
 	"github.com/pingcap/tidb/tablecodec"
 	"github.com/pingcap/tidb/types"
 	"github.com/pingcap/tidb/util/codec"
 	"github.com/pingcap/tidb/util/hack"
+
+	"github.com/pingcap/tidb-lightning/lightning/common"
 )
 
 type localSuite struct{}
@@ -388,3 +392,84 @@ func (s *localSuite) TestLocalWriterWithIngest(c *C) {
 func (s *localSuite) TestLocalWriterWithIngestUnsort(c *C) {
 	testLocalWriter(c, true, true)
 }
+
+type mockSplitClient struct {
+	restore.SplitClient
+}
+
+func (c *mockSplitClient) GetRegion(ctx context.Context, key []byte) (*restore.RegionInfo, error) {
+	return &restore.RegionInfo{
+		Leader: &metapb.Peer{Id: 1},
+		Region: &metapb.Region{
+			Id:       1,
+			StartKey: key,
+		},
+	}, nil
+}
+
+func (s *localSuite) TestIsIngestRetryable(c *C) {
+	local := &local{
+		splitCli: &mockSplitClient{},
+	}
+
+	resp := &sst.IngestResponse{
+		Error: &errorpb.Error{
+			NotLeader: &errorpb.NotLeader{
+				Leader: &metapb.Peer{Id: 2},
+			},
+		},
+	}
+	ctx := context.Background()
+	region := &restore.RegionInfo{
+		Leader: &metapb.Peer{Id: 1},
+		Region: &metapb.Region{
+			Id:       1,
+			StartKey: []byte{1},
+			EndKey:   []byte{3},
+			RegionEpoch: &metapb.RegionEpoch{
+				ConfVer: 1,
+				Version: 1,
+			},
+		},
+	}
+	meta := &sst.SSTMeta{
+		Range: &sst.Range{
+			Start: []byte{1},
+			End:   []byte{2},
+		},
+	}
+	retryType, newRegion, err := local.isIngestRetryable(ctx, resp, region, meta)
+	c.Assert(retryType, Equals, retryWrite)
+	c.Assert(newRegion.Leader.Id, Equals, uint64(2))
+	c.Assert(err, NotNil)
+
+	resp.Error = &errorpb.Error{
+		EpochNotMatch: &errorpb.EpochNotMatch{
+			CurrentRegions: []*metapb.Region{
+				{
+					Id:       1,
+					StartKey: []byte{1},
+					EndKey:   []byte{3},
+					RegionEpoch: &metapb.RegionEpoch{
+						ConfVer: 1,
+						Version: 2,
+					},
+					Peers: []*metapb.Peer{{Id: 1}},
+				},
+			},
+		},
+	}
+	retryType, newRegion, err = local.isIngestRetryable(ctx, resp, region, meta)
+	c.Assert(retryType, Equals, retryWrite)
+	c.Assert(newRegion.Region.RegionEpoch.Version, Equals, uint64(2))
+	c.Assert(err, NotNil)
+
+	resp.Error = &errorpb.Error{Message: "raft: proposal dropped"}
+	retryType, newRegion, err = local.isIngestRetryable(ctx, resp, region, meta)
+	c.Assert(retryType, Equals, retryWrite)
+
+	resp.Error = &errorpb.Error{Message: "unknown error"}
+	retryType, newRegion, err = local.isIngestRetryable(ctx, resp, region, meta)
+	c.Assert(retryType, Equals, retryNone)
+	c.Assert(err, ErrorMatches, "non-retryable error: unknown error")
+}