fix comment and add more tests

Signed-off-by: okJiang <[email protected]>
tikv · Jul 11, 2024 · d4f4a8e · d4f4a8e
1 parent 607cca3
commit d4f4a8e
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 31 deletions.
diff --git a/pkg/core/region.go b/pkg/core/region.go
@@ -1881,7 +1881,7 @@ func scanRegion(regionTree *regionTree, keyRange *KeyRange, limit int) ([]*Regio
 		}
 		if len(lastRegion.GetEndKey()) > 0 && len(region.GetStartKey()) > 0 &&
 			bytes.Compare(region.GetStartKey(), lastRegion.GetEndKey()) > 0 {
-			err = multierr.Append(err, errors.Errorf(
+			err = multierr.Append(err, errs.ErrRegionNotAdjacent.FastGen(
 				"key range[%x, %x) found a hole region between region[%x, %x) and region[%x, %x)",
 				keyRange.StartKey, keyRange.EndKey,
 				lastRegion.GetStartKey(), lastRegion.GetEndKey(),
@@ -1895,12 +1895,17 @@ func scanRegion(regionTree *regionTree, keyRange *KeyRange, limit int) ([]*Regio
 
 	if !(exceedLimit()) && len(keyRange.EndKey) > 0 && len(lastRegion.GetEndKey()) > 0 &&
 		bytes.Compare(lastRegion.GetEndKey(), keyRange.EndKey) < 0 {
-		err = multierr.Append(err, errors.Errorf(
+		err = multierr.Append(err, errs.ErrRegionNotAdjacent.FastGen(
 			"key range[%x, %x) found a hole region in the last, the last scanned region is [%x, %x), [%x, %x) is missing",
 			keyRange.StartKey, keyRange.EndKey,
 			lastRegion.GetStartKey(), lastRegion.GetEndKey(),
 			lastRegion.GetEndKey(), keyRange.EndKey))
 	}
+
+	const errLimit = 3
+	if multiErr := multierr.Errors(err); len(multiErr) > errLimit {
+		err = multierr.Combine(multiErr[:errLimit]...)
+	}
 	return res, err
 }
 

diff --git a/pkg/core/region_test.go b/pkg/core/region_test.go
@@ -27,8 +27,10 @@ import (
 	"github.com/pingcap/kvproto/pkg/metapb"
 	"github.com/pingcap/kvproto/pkg/pdpb"
 	"github.com/stretchr/testify/require"
+	"github.com/tikv/pd/pkg/errs"
 	"github.com/tikv/pd/pkg/id"
 	"github.com/tikv/pd/pkg/mock/mockid"
+	"go.uber.org/multierr"
 )
 
 func TestNeedMerge(t *testing.T) {
@@ -1145,54 +1147,64 @@ func TestCntRefAfterResetRegionCache(t *testing.T) {
 func TestScanRegion(t *testing.T) {
 	re := require.New(t)
 	tree := newRegionTree()
+	scanError := func(startKey, endKey []byte, limit int) ([]*RegionInfo, error) {
+		regions, err := scanRegion(tree, &KeyRange{StartKey: startKey, EndKey: endKey}, limit)
+		re.Error(err)
+		return regions, err
+	}
+	scanNoError := func(startKey, endKey []byte, limit int) []*RegionInfo {
+		regions, err := scanRegion(tree, &KeyRange{StartKey: startKey, EndKey: endKey}, limit)
+		re.NoError(err)
+		return regions
+	}
 	// region1
 	// [a, b)
 	updateNewItem(tree, NewTestRegionInfo(1, 1, []byte("a"), []byte("b")))
-	regions, err := scanRegion(tree, &KeyRange{StartKey: []byte("a"), EndKey: []byte("b")}, 0)
-	re.NoError(err)
-	re.Len(regions, 1)
-	regions, err = scanRegion(tree, &KeyRange{StartKey: []byte("a"), EndKey: []byte("c")}, 0)
-	re.Error(err)
-	re.Len(regions, 1)
-	regions, err = scanRegion(tree, &KeyRange{StartKey: []byte("a"), EndKey: []byte("c")}, 1)
-	re.NoError(err)
+	re.Len(scanNoError([]byte("a"), []byte("b"), 0), 1)
+	regions, _ := scanError([]byte("a"), []byte("c"), 0)
 	re.Len(regions, 1)
+	re.Len(scanNoError([]byte("a"), []byte("c"), 1), 1)
 
 	// region1 | region2
 	// [a, b)  | [b, c)
 	updateNewItem(tree, NewTestRegionInfo(2, 1, []byte("b"), []byte("c")))
-	regions, err = scanRegion(tree, &KeyRange{StartKey: []byte("a"), EndKey: []byte("c")}, 0)
-	re.NoError(err)
-	re.Len(regions, 2)
-	regions, err = scanRegion(tree, &KeyRange{StartKey: []byte("a"), EndKey: []byte("c")}, 1)
-	re.NoError(err)
-	re.Len(regions, 1)
+	re.Len(scanNoError([]byte("a"), []byte("c"), 0), 2)
+	re.Len(scanNoError([]byte("a"), []byte("c"), 1), 1)
 
 	// region1 | region2 | region3
 	// [a, b)  | [b, c)  | [d, f)
 	updateNewItem(tree, NewTestRegionInfo(3, 1, []byte("d"), []byte("f")))
-	regions, err = scanRegion(tree, &KeyRange{StartKey: []byte("a"), EndKey: []byte("e")}, 0)
-	re.Error(err)
+	regions, _ = scanError([]byte("a"), []byte("e"), 0)
 	re.Len(regions, 3)
-	regions, err = scanRegion(tree, &KeyRange{StartKey: []byte("c"), EndKey: []byte("e")}, 0)
-	re.Error(err)
+	regions, _ = scanError([]byte("c"), []byte("e"), 0)
 	re.Len(regions, 1)
 	re.Equal(uint64(3), regions[0].GetID())
 
 	// region1 | region2 | region3 | region4
-	// [a, b)  | [b, c)  | [d, f)  | [f, +∞)
-	updateNewItem(tree, NewTestRegionInfo(4, 1, []byte("f"), nil))
-	regions, err = scanRegion(tree, &KeyRange{StartKey: []byte("c"), EndKey: []byte("g")}, 0)
-	re.Error(err)
+	// [a, b)  | [b, c)  | [d, f)  | [f, i)
+	updateNewItem(tree, NewTestRegionInfo(4, 1, []byte("f"), []byte("i")))
+	regions, _ = scanError([]byte("c"), []byte("g"), 0)
 	re.Len(regions, 2)
 	re.Equal(uint64(3), regions[0].GetID())
 	re.Equal(uint64(4), regions[1].GetID())
-	regions, err = scanRegion(tree, &KeyRange{StartKey: []byte("g"), EndKey: []byte("h")}, 0)
-	re.NoError(err)
-	re.Len(regions, 1)
-	re.Equal(uint64(4), regions[0].GetID())
-	regions, err = scanRegion(tree, &KeyRange{StartKey: []byte("g"), EndKey: nil}, 0)
-	re.NoError(err)
+
+	regions = scanNoError([]byte("g"), []byte("h"), 0)
 	re.Len(regions, 1)
 	re.Equal(uint64(4), regions[0].GetID())
+	// test multil error and error type
+	regions, err := scanError([]byte(string('a'-1)), []byte("g"), 0)
+	multiErr := multierr.Errors(err)
+	re.Len(multiErr, 2)
+	re.True(errs.ErrRegionNotAdjacent.Equal(multiErr[0]))
+	re.Len(regions, 4)
+
+	// region1 | region2 | region3 | region4 | region5 | region6
+	// [a, b)  | [b, c)  | [d, f)  | [f, i)  | [j, k)  | [l, +∞)]
+	updateNewItem(tree, NewTestRegionInfo(6, 1, []byte("l"), nil))
+	// test boundless
+	re.Len(scanNoError([]byte("m"), nil, 0), 1)
+	// test errLimit
+	_, err = scanError([]byte(string('a'-1)), []byte("m"), 0)
+	multiErr = multierr.Errors(err)
+	re.Len(multiErr, 3)
 }
diff --git a/server/grpc_service.go b/server/grpc_service.go
@@ -48,6 +48,7 @@ import (
 	"github.com/tikv/pd/pkg/versioninfo"
 	"github.com/tikv/pd/server/cluster"
 	"go.etcd.io/etcd/clientv3"
+	"go.uber.org/multierr"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
@@ -1687,7 +1688,14 @@ func (s *GrpcServer) BatchScanRegions(ctx context.Context, request *pdpb.BatchSc
 	}
 	res, err := rc.BatchScanRegions(keyRanges, scanOptions...)
 	if err != nil {
-		return &pdpb.BatchScanRegionsResponse{Header: s.wrapErrorToHeader(pdpb.ErrorType_REGIONS_NOT_CONTAIN_ALL_KEY_RANGE, err.Error())}, nil
+		if errs.ErrRegionNotAdjacent.Equal(multierr.Errors(err)[0]) {
+			return &pdpb.BatchScanRegionsResponse{
+				Header: s.wrapErrorToHeader(pdpb.ErrorType_REGIONS_NOT_CONTAIN_ALL_KEY_RANGE, err.Error()),
+			}, nil
+		}
+		return &pdpb.BatchScanRegionsResponse{
+			Header: s.wrapErrorToHeader(pdpb.ErrorType_UNKNOWN, err.Error()),
+		}, nil
 	}
 	regions := make([]*pdpb.Region, 0, len(res))
 	for _, r := range res {