ddl: refine range validation after scaning regions (pingcap#59767)

close pingcap#59701

Author: tangenta

pkg/ddl/backfilling.go

@@ -511,7 +511,11 @@ func loadTableRanges(
 	maxSleep := 10000 // ms
 	bo := tikv.NewBackofferWithVars(ctx, maxSleep, nil)
 	var ranges []kv.KeyRange
-	err := util.RunWithRetry(util.DefaultMaxRetries, util.RetryInterval, func() (bool, error) {
+	maxRetryTimes := util.DefaultMaxRetries
+	failpoint.Inject("loadTableRangesNoRetry", func() {
+		maxRetryTimes = 1
+	})
+	err := util.RunWithRetry(maxRetryTimes, util.RetryInterval, func() (bool, error) {
 		logutil.DDLLogger().Info("load table ranges from PD",
 			zap.Int64("physicalTableID", t.GetPhysicalID()),
 			zap.String("start key", hex.EncodeToString(startKey)),
@@ -549,6 +553,9 @@ func loadTableRanges(
 }
 
 func validateAndFillRanges(ranges []kv.KeyRange, startKey, endKey []byte) error {
+	failpoint.Inject("validateAndFillRangesErr", func() {
+		failpoint.Return(dbterror.ErrInvalidSplitRegionRanges.GenWithStackByArgs("mock"))
+	})
 	if len(ranges) == 0 {
 		errMsg := fmt.Sprintf("cannot find region in range [%s, %s]",
 			hex.EncodeToString(startKey), hex.EncodeToString(endKey))
@@ -559,19 +566,26 @@ func validateAndFillRanges(ranges []kv.KeyRange, startKey, endKey []byte) error
 			s := r.StartKey
 			if len(s) == 0 || bytes.Compare(s, startKey) < 0 {
 				ranges[i].StartKey = startKey
+			} else if bytes.Compare(s, startKey) > 0 {
+				errMsg := fmt.Sprintf("get empty range at the beginning of ranges, expected %s, but got %s",
+					hex.EncodeToString(startKey), hex.EncodeToString(s))
+				return dbterror.ErrInvalidSplitRegionRanges.GenWithStackByArgs(errMsg)
 			}
 		}
 		if i == len(ranges)-1 {
 			e := r.EndKey
 			if len(e) == 0 || bytes.Compare(e, endKey) > 0 {
 				ranges[i].EndKey = endKey
 			}
+			// We don't need to check the end key because a limit may set before scanning regions.
 		}
 		if len(ranges[i].StartKey) == 0 || len(ranges[i].EndKey) == 0 {
-			return errors.Errorf("get empty start/end key in the middle of ranges")
+			return dbterror.ErrInvalidSplitRegionRanges.GenWithStackByArgs("get empty start/end key in the middle of ranges")
 		}
 		if i > 0 && !bytes.Equal(ranges[i-1].EndKey, ranges[i].StartKey) {
-			return errors.Errorf("ranges are not continuous")
+			errMsg := fmt.Sprintf("ranges are not continuous, last end key %s, next start key %s",
+				hex.EncodeToString(ranges[i-1].EndKey), hex.EncodeToString(ranges[i].StartKey))
+			return dbterror.ErrInvalidSplitRegionRanges.GenWithStackByArgs(errMsg)
 		}
 	}
 	return nil

pkg/ddl/backfilling_test.go

@@ -418,7 +418,7 @@ func TestValidateAndFillRanges(t *testing.T) {
 		mkRange("c", "d"),
 		mkRange("d", "e"),
 	}
-	err := validateAndFillRanges(ranges, []byte("a"), []byte("e"))
+	err := validateAndFillRanges(ranges, []byte("b"), []byte("e"))
 	require.NoError(t, err)
 	require.EqualValues(t, []kv.KeyRange{
 		mkRange("b", "c"),
@@ -480,6 +480,22 @@ func TestValidateAndFillRanges(t *testing.T) {
 	}
 	err = validateAndFillRanges(ranges, []byte("b"), []byte("f"))
 	require.Error(t, err)
+
+	ranges = []kv.KeyRange{
+		mkRange("b", "c"),
+		mkRange("c", "d"),
+		mkRange("d", "e"),
+	}
+	err = validateAndFillRanges(ranges, []byte("a"), []byte("e"))
+	require.Error(t, err)
+
+	ranges = []kv.KeyRange{
+		mkRange("b", "c"),
+		mkRange("c", "d"),
+		mkRange("d", "e"),
+	}
+	err = validateAndFillRanges(ranges, []byte("b"), []byte("f"))
+	require.NoError(t, err)
 }
 
 func TestTuneTableScanWorkerBatchSize(t *testing.T) {

pkg/ddl/ingest/BUILD.bazel

@@ -74,7 +74,7 @@ go_test(
     ],
     flaky = True,
     race = "on",
-    shard_count = 23,
+    shard_count = 24,
     deps = [
         ":ingest",
         "//pkg/config",

pkg/ddl/ingest/integration_test.go

@@ -525,3 +525,17 @@ func TestAddGlobalIndexInIngestWithUpdate(t *testing.T) {
 	rsTable := tk.MustQuery("select *,_tidb_rowid from t use index()").Sort()
 	require.Equal(t, rsGlobalIndex.String(), rsTable.String())
 }
+
+func TestAddIndexValidateRangesFailed(t *testing.T) {
+	store := testkit.CreateMockStore(t)
+	defer ingesttestutil.InjectMockBackendCtx(t, store)()
+	tk := testkit.NewTestKit(t, store)
+	tk.MustExec("use test")
+	tk.MustExec("create table t (a int primary key, b int);")
+	tk.MustExec("insert into t values (1, 1);")
+
+	testfailpoint.Enable(t, "github.com/pingcap/tidb/pkg/ddl/loadTableRangesNoRetry", "return")
+	testfailpoint.Enable(t, "github.com/pingcap/tidb/pkg/ddl/validateAndFillRangesErr", "2*return")
+	tk.MustExec("alter table t add index idx(b);")
+	tk.MustExec("admin check table t;")
+}

pkg/util/dbterror/ddl_terror.go

@@ -531,6 +531,7 @@ var ReorgRetryableErrCodes = map[uint16]struct{}{
 	mysql.ErrWriteConflictInTiDB:       {},
 	mysql.ErrTxnRetryable:              {},
 	mysql.ErrNotOwner:                  {},
+	mysql.ErrInvalidSplitRegionRanges:  {}, // PD client returns regions with no leader.
 
 	// Temporary network partitioning may cause pk commit failure.
 	uint16(terror.CodeResultUndetermined): {},

tests/realtikvtest/addindextest4/failure_test.go

@@ -33,6 +33,7 @@ import (
 )
 
 func TestAddIndexIngestRecoverPartition(t *testing.T) {
+	t.Skip("the test is too hacky, use another way to test")
 	partCnt := 0
 	block := make(chan struct{})
 	ExecuteBlocks(t, func() {