Repository: pingcap/tidb-lightning
Pull request #516 (2020-12-09 08:50:42)

backend/local: remove useless and buggy truncate key
<!--
Thank you for contributing to TiDB-Lightning! Please read the [CONTRIBUTING](https://github.com/pingcap/tidb-lightning/blob/master/CONTRIBUTING.md) document **BEFORE** filing this PR.
-->

### What problem does this PR solve? <!--add issue link with summary if exists-->
Remove the useless and buggy `truncateRowKey` call when generates region split keys because this action is not compatible with clustered index. And it's also useless even with clustered index disabled.

### What is changed and how it works?


### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

 - Unit test
 - Integration test
 - Manual test (add detailed scripts or steps below)
 - No code

Side effects

Related changes


glorv: /merge
ti-srebot: /run-all-tests

Patch:
diff --git a/lightning/backend/localhelper.go b/lightning/backend/localhelper.go
index 976010a..2128b5e 100644
--- a/lightning/backend/localhelper.go
+++ b/lightning/backend/localhelper.go
@@ -28,7 +28,6 @@ import (
 	sst "github.com/pingcap/kvproto/pkg/import_sstpb"
 	"github.com/pingcap/kvproto/pkg/metapb"
 	"github.com/pingcap/kvproto/pkg/pdpb"
-	"github.com/pingcap/tidb/tablecodec"
 	"github.com/pingcap/tidb/util/codec"
 	"go.uber.org/zap"
 
@@ -320,9 +319,9 @@ func getSplitKeysByRanges(ranges []Range, regions []*split.RegionInfo) map[uint6
 	var lastEnd []byte
 	for _, rg := range ranges {
 		if !bytes.Equal(lastEnd, rg.start) {
-			checkKeys = append(checkKeys, truncateRowKey(rg.start))
+			checkKeys = append(checkKeys, rg.start)
 		}
-		checkKeys = append(checkKeys, truncateRowKey(rg.end))
+		checkKeys = append(checkKeys, rg.end)
 		lastEnd = rg.end
 	}
 	return getSplitKeys(checkKeys, regions)
@@ -373,21 +372,6 @@ func needSplit(key []byte, regions []*split.RegionInfo) *split.RegionInfo {
 	return nil
 }
 
-var (
-	tablePrefix  = tablecodec.TablePrefix()
-	idLen        = 8
-	recordPrefix = []byte("_r")
-)
-
-func truncateRowKey(key []byte) []byte {
-	if bytes.HasPrefix(key, tablePrefix) &&
-		len(key) > tablecodec.RecordRowKeyLen &&
-		bytes.HasPrefix(key[len(tablePrefix)+idLen:], recordPrefix) {
-		return key[:tablecodec.RecordRowKeyLen]
-	}
-	return key
-}
-
 func beforeEnd(key []byte, end []byte) bool {
 	return bytes.Compare(key, end) < 0 || len(end) == 0
 }


Test patch:
diff --git a/lightning/backend/localhelper_test.go b/lightning/backend/localhelper_test.go
index a4388ad..10aec9c 100644
--- a/lightning/backend/localhelper_test.go
+++ b/lightning/backend/localhelper_test.go
@@ -23,6 +23,10 @@ import (
 	"github.com/pingcap/errors"
 	"github.com/pingcap/kvproto/pkg/metapb"
 	"github.com/pingcap/kvproto/pkg/pdpb"
+	"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/tikv/pd/server/core"
 	"github.com/tikv/pd/server/schedule/placement"
@@ -215,20 +219,19 @@ func (c *testClient) SetStoresLabel(ctx context.Context, stores []uint64, labelK
 }
 
 // region: [, aay), [aay, bba), [bba, bbh), [bbh, cca), [cca, )
-func initTestClient() *testClient {
+func initTestClient(keys [][]byte) *testClient {
 	peers := make([]*metapb.Peer, 1)
 	peers[0] = &metapb.Peer{
 		Id:      1,
 		StoreId: 1,
 	}
-	keys := [6]string{"", "aay", "bba", "bbh", "cca", ""}
 	regions := make(map[uint64]*restore.RegionInfo)
-	for i := uint64(1); i < 6; i++ {
-		startKey := []byte(keys[i-1])
+	for i := uint64(1); i < uint64(len(keys)); i++ {
+		startKey := keys[i-1]
 		if len(startKey) != 0 {
 			startKey = codec.EncodeBytes([]byte{}, startKey)
 		}
-		endKey := []byte(keys[i])
+		endKey := keys[i]
 		if len(endKey) != 0 {
 			endKey = codec.EncodeBytes([]byte{}, endKey)
 		}
@@ -245,7 +248,7 @@ func initTestClient() *testClient {
 	stores[1] = &metapb.Store{
 		Id: 1,
 	}
-	return newTestClient(stores, regions, 6)
+	return newTestClient(stores, regions, uint64(len(keys)))
 }
 
 func checkRegionRanges(c *C, regions []*restore.RegionInfo, keys [][]byte) {
@@ -265,7 +268,8 @@ func (s *localSuite) TestBatchSplitAndIngestRegion(c *C) {
 		maxBatchSplitKeys = oldLimit
 	}()
 
-	client := initTestClient()
+	keys := [][]byte{[]byte(""), []byte("aay"), []byte("bba"), []byte("bbh"), []byte("cca"), []byte("")}
+	client := initTestClient(keys)
 	local := &local{
 		splitCli: client,
 	}
@@ -274,7 +278,7 @@ func (s *localSuite) TestBatchSplitAndIngestRegion(c *C) {
 	// current region ranges: [, aay), [aay, bba), [bba, bbh), [bbh, cca), [cca, )
 	rangeStart := codec.EncodeBytes([]byte{}, []byte("b"))
 	rangeEnd := codec.EncodeBytes([]byte{}, []byte("c"))
-	regions, err := paginateScanRegion(ctx, client, rangeStart, rangeEnd, 1)
+	regions, err := paginateScanRegion(ctx, client, rangeStart, rangeEnd, 5)
 	c.Assert(err, IsNil)
 	// regions is: [aay, bba), [bba, bbh), [bbh, cca)
 	checkRegionRanges(c, regions, [][]byte{[]byte("aay"), []byte("bba"), []byte("bbh"), []byte("cca")})
@@ -303,7 +307,7 @@ func (s *localSuite) TestBatchSplitAndIngestRegion(c *C) {
 	c.Assert(client.splitCount, Equals, 7)
 
 	// check split ranges
-	regions, err = paginateScanRegion(ctx, client, rangeStart, rangeEnd, 1)
+	regions, err = paginateScanRegion(ctx, client, rangeStart, rangeEnd, 5)
 	c.Assert(err, IsNil)
 	result := [][]byte{[]byte("b"), []byte("ba"), []byte("bb"), []byte("bba"), []byte("bbh"), []byte("bc"),
 		[]byte("bd"), []byte("be"), []byte("bf"), []byte("bg"), []byte("bh"), []byte("bi"), []byte("bj"),
@@ -312,3 +316,47 @@ func (s *localSuite) TestBatchSplitAndIngestRegion(c *C) {
 		[]byte("by"), []byte("bz"), []byte("cca")}
 	checkRegionRanges(c, regions, result)
 }
+
+func (s *localSuite) TestBatchSplitAndIngestWithClusteredIndex(c *C) {
+	tableId := int64(1)
+	tableStartKey := tablecodec.EncodeTablePrefix(tableId)
+	tableEndKey := tablecodec.EncodeTablePrefix(tableId + 1)
+	keys := [][]byte{[]byte(""), tableStartKey, tableEndKey, []byte("")}
+	client := initTestClient(keys)
+	local := &local{
+		splitCli: client,
+	}
+	ctx := context.Background()
+
+	// we batch generate a batch of row keys for table 1 with common handle
+	keys = make([][]byte, 0, 20+1)
+	stmtCtx := new(stmtctx.StatementContext)
+	for i := int64(0); i < 2; i++ {
+		for j := int64(0); j < 10; j++ {
+			keyBytes, err := codec.EncodeKey(stmtCtx, nil, types.NewIntDatum(i), types.NewIntDatum(j*10000))
+			c.Assert(err, IsNil)
+			h, err := kv.NewCommonHandle(keyBytes)
+			c.Assert(err, IsNil)
+			key := tablecodec.EncodeRowKeyWithHandle(tableId, h)
+			keys = append(keys, key)
+		}
+	}
+
+	start := keys[0]
+	ranges := make([]Range, 0, len(keys)-1)
+	for _, e := range keys[1:] {
+		ranges = append(ranges, Range{start: start, end: e})
+		start = e
+	}
+
+	err := local.SplitAndScatterRegionByRanges(ctx, ranges)
+	c.Assert(err, IsNil)
+
+	startKey := codec.EncodeBytes([]byte{}, keys[0])
+	endKey := codec.EncodeBytes([]byte{}, keys[len(keys)-1])
+	// check split ranges
+	regions, err := paginateScanRegion(ctx, client, startKey, endKey, 5)
+	c.Assert(err, IsNil)
+	c.Assert(len(regions), Equals, len(ranges))
+	checkRegionRanges(c, regions, keys)
+}