From 0b5ed0fce6620c0824c9d82fa6493d643003092e Mon Sep 17 00:00:00 2001 From: okJiang <819421878@qq.com> Date: Wed, 17 Jul 2024 14:10:30 +0800 Subject: [PATCH] client: report error when scan region encounter hole region (#8375) close tikv/pd#8358 client: report error when scan region encounter hole region - add a input parameter(OutputMustContainAllKeyRange) in the BatchScanRegionsRequest - when the new param enable and find the result doesn't contain all key range(input), it will return an error to user - add Merge() method to merge the continuous KeyRanges - pull out the scanRegion function and add unit tests for it Signed-off-by: okJiang <819421878@qq.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> --- client/client.go | 19 ++-- client/go.mod | 2 +- client/go.sum | 4 +- go.mod | 4 +- go.sum | 4 +- pkg/core/basic_cluster.go | 55 +++++++++- pkg/core/basic_cluster_test.go | 93 +++++++++++++++++ pkg/core/region.go | 100 ++++++++++++++---- pkg/core/region_test.go | 60 +++++++++++ server/grpc_service.go | 18 +++- tests/integrations/client/client_test.go | 127 ++++++++++++++++------- tests/integrations/go.mod | 2 +- tests/integrations/go.sum | 4 +- tools/go.mod | 2 +- tools/go.sum | 4 +- 15 files changed, 417 insertions(+), 81 deletions(-) create mode 100644 pkg/core/basic_cluster_test.go diff --git a/client/client.go b/client/client.go index 8c8299daeab..aafe4aba77f 100644 --- a/client/client.go +++ b/client/client.go @@ -214,8 +214,9 @@ func WithSkipStoreLimit() RegionsOption { // GetRegionOp represents available options when getting regions. type GetRegionOp struct { - needBuckets bool - allowFollowerHandle bool + needBuckets bool + allowFollowerHandle bool + outputMustContainAllKeyRange bool } // GetRegionOption configures GetRegionOp. @@ -231,6 +232,11 @@ func WithAllowFollowerHandle() GetRegionOption { return func(op *GetRegionOp) { op.allowFollowerHandle = true } } +// WithOutputMustContainAllKeyRange means the output must contain all key ranges. +func WithOutputMustContainAllKeyRange() GetRegionOption { + return func(op *GetRegionOp) { op.outputMustContainAllKeyRange = true } +} + var ( // errUnmatchedClusterID is returned when found a PD with a different cluster ID. errUnmatchedClusterID = errors.New("[pd] unmatched cluster id") @@ -1193,10 +1199,11 @@ func (c *client) BatchScanRegions(ctx context.Context, ranges []KeyRange, limit pbRanges = append(pbRanges, &pdpb.KeyRange{StartKey: r.StartKey, EndKey: r.EndKey}) } req := &pdpb.BatchScanRegionsRequest{ - Header: c.requestHeader(), - NeedBuckets: options.needBuckets, - Ranges: pbRanges, - Limit: int32(limit), + Header: c.requestHeader(), + NeedBuckets: options.needBuckets, + Ranges: pbRanges, + Limit: int32(limit), + ContainAllKeyRange: options.outputMustContainAllKeyRange, } serviceClient, cctx := c.getRegionAPIClientAndContext(scanCtx, options.allowFollowerHandle && c.option.getEnableFollowerHandle()) if serviceClient == nil { diff --git a/client/go.mod b/client/go.mod index 7c782695539..8dc706a4540 100644 --- a/client/go.mod +++ b/client/go.mod @@ -10,7 +10,7 @@ require ( github.com/opentracing/opentracing-go v1.2.0 github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c github.com/pingcap/failpoint v0.0.0-20210918120811-547c13e3eb00 - github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4 + github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7 github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 github.com/prometheus/client_golang v1.18.0 github.com/stretchr/testify v1.8.2 diff --git a/client/go.sum b/client/go.sum index 8f85f5ce7ed..20c154c30dc 100644 --- a/client/go.sum +++ b/client/go.sum @@ -46,8 +46,8 @@ github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c h1:xpW9bvK+HuuTm github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c/go.mod h1:X2r9ueLEUZgtx2cIogM0v4Zj5uvvzhuuiu7Pn8HzMPg= github.com/pingcap/failpoint v0.0.0-20210918120811-547c13e3eb00 h1:C3N3itkduZXDZFh4N3vQ5HEtld3S+Y+StULhWVvumU0= github.com/pingcap/failpoint v0.0.0-20210918120811-547c13e3eb00/go.mod h1:4qGtCB0QK0wBzKtFEGDhxXnSnbQApw1gc9siScUl8ew= -github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4 h1:6aIKNB2YGAec4IUDLw6G2eDECiGiufZcgEbZSCELBx0= -github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= +github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7 h1:V9XS3FQ/P6u+kFaoSyY5DBswIA774BMpIOLDBMrpxKc= +github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 h1:HR/ylkkLmGdSSDaD8IDP+SZrdhV1Kibl9KrHxJ9eciw= github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3/go.mod h1:DWQW5jICDR7UJh4HtxXSM20Churx4CQL0fwL/SoOSA4= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/go.mod b/go.mod index 1ef14f416e8..5b8074d285b 100644 --- a/go.mod +++ b/go.mod @@ -34,7 +34,7 @@ require ( github.com/pingcap/errcode v0.3.0 github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c github.com/pingcap/failpoint v0.0.0-20210918120811-547c13e3eb00 - github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4 + github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7 github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 github.com/pingcap/sysutil v1.0.1-0.20230407040306-fb007c5aff21 github.com/pingcap/tidb-dashboard v0.0.0-20240612100141-91f6c281e441 @@ -173,7 +173,7 @@ require ( go.etcd.io/bbolt v1.3.9 // indirect go.uber.org/dig v1.9.0 // indirect go.uber.org/fx v1.12.0 // indirect - go.uber.org/multierr v1.11.0 // indirect + go.uber.org/multierr v1.11.0 golang.org/x/arch v0.3.0 // indirect golang.org/x/crypto v0.21.0 // indirect golang.org/x/image v0.10.0 // indirect diff --git a/go.sum b/go.sum index 659cd116e9c..baffeb0d6b7 100644 --- a/go.sum +++ b/go.sum @@ -371,8 +371,8 @@ github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c/go.mod h1:X2r9ue github.com/pingcap/failpoint v0.0.0-20210918120811-547c13e3eb00 h1:C3N3itkduZXDZFh4N3vQ5HEtld3S+Y+StULhWVvumU0= github.com/pingcap/failpoint v0.0.0-20210918120811-547c13e3eb00/go.mod h1:4qGtCB0QK0wBzKtFEGDhxXnSnbQApw1gc9siScUl8ew= github.com/pingcap/kvproto v0.0.0-20191211054548-3c6b38ea5107/go.mod h1:WWLmULLO7l8IOcQG+t+ItJ3fEcrL5FxF0Wu+HrMy26w= -github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4 h1:6aIKNB2YGAec4IUDLw6G2eDECiGiufZcgEbZSCELBx0= -github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= +github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7 h1:V9XS3FQ/P6u+kFaoSyY5DBswIA774BMpIOLDBMrpxKc= +github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= github.com/pingcap/log v0.0.0-20210625125904-98ed8e2eb1c7/go.mod h1:8AanEdAHATuRurdGxZXBz0At+9avep+ub7U1AGYLIMM= github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 h1:HR/ylkkLmGdSSDaD8IDP+SZrdhV1Kibl9KrHxJ9eciw= github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3/go.mod h1:DWQW5jICDR7UJh4HtxXSM20Churx4CQL0fwL/SoOSA4= diff --git a/pkg/core/basic_cluster.go b/pkg/core/basic_cluster.go index ea78c4ccf9c..f0b23bd6434 100644 --- a/pkg/core/basic_cluster.go +++ b/pkg/core/basic_cluster.go @@ -14,6 +14,8 @@ package core +import "bytes" + // BasicCluster provides basic data member and interface for a tikv cluster. type BasicCluster struct { *StoresInfo @@ -97,7 +99,29 @@ type RegionSetInformer interface { GetAdjacentRegions(region *RegionInfo) (*RegionInfo, *RegionInfo) ScanRegions(startKey, endKey []byte, limit int) []*RegionInfo GetRegionByKey(regionKey []byte) *RegionInfo - BatchScanRegions(keyRanges *KeyRanges, limit int) []*RegionInfo + BatchScanRegions(keyRanges *KeyRanges, opts ...BatchScanRegionsOptionFunc) ([]*RegionInfo, error) +} + +type batchScanRegionsOptions struct { + limit int + outputMustContainAllKeyRange bool +} + +// BatchScanRegionsOptionFunc is the option function for BatchScanRegions. +type BatchScanRegionsOptionFunc func(*batchScanRegionsOptions) + +// WithLimit is an option for batchScanRegionsOptions. +func WithLimit(limit int) BatchScanRegionsOptionFunc { + return func(opt *batchScanRegionsOptions) { + opt.limit = limit + } +} + +// WithOutputMustContainAllKeyRange is an option for batchScanRegionsOptions. +func WithOutputMustContainAllKeyRange() BatchScanRegionsOptionFunc { + return func(opt *batchScanRegionsOptions) { + opt.outputMustContainAllKeyRange = true + } } // StoreSetInformer provides access to a shared informer of stores. @@ -136,7 +160,7 @@ func NewKeyRange(startKey, endKey string) KeyRange { } } -// KeyRanges is a slice of KeyRange. +// KeyRanges is a slice of monotonically increasing KeyRange. type KeyRanges struct { krs []*KeyRange } @@ -163,3 +187,30 @@ func (rs *KeyRanges) Ranges() []*KeyRange { } return rs.krs } + +// Merge merges the continuous KeyRanges. +func (rs *KeyRanges) Merge() { + if len(rs.krs) == 0 { + return + } + merged := make([]*KeyRange, 0, len(rs.krs)) + start := rs.krs[0].StartKey + end := rs.krs[0].EndKey + for _, kr := range rs.krs[1:] { + if bytes.Equal(end, kr.StartKey) { + end = kr.EndKey + } else { + merged = append(merged, &KeyRange{ + StartKey: start, + EndKey: end, + }) + start = kr.StartKey + end = kr.EndKey + } + } + merged = append(merged, &KeyRange{ + StartKey: start, + EndKey: end, + }) + rs.krs = merged +} diff --git a/pkg/core/basic_cluster_test.go b/pkg/core/basic_cluster_test.go new file mode 100644 index 00000000000..3d74dd49eea --- /dev/null +++ b/pkg/core/basic_cluster_test.go @@ -0,0 +1,93 @@ +// Copyright 2024 TiKV Project Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package core + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestMergeKeyRanges(t *testing.T) { + re := require.New(t) + + testCases := []struct { + name string + input []*KeyRange + expect []*KeyRange + }{ + { + name: "empty", + input: []*KeyRange{}, + expect: []*KeyRange{}, + }, + { + name: "single", + input: []*KeyRange{ + {StartKey: []byte("a"), EndKey: []byte("b")}, + }, + expect: []*KeyRange{ + {StartKey: []byte("a"), EndKey: []byte("b")}, + }, + }, + { + name: "non-overlapping", + input: []*KeyRange{ + {StartKey: []byte("a"), EndKey: []byte("b")}, + {StartKey: []byte("c"), EndKey: []byte("d")}, + }, + expect: []*KeyRange{ + {StartKey: []byte("a"), EndKey: []byte("b")}, + {StartKey: []byte("c"), EndKey: []byte("d")}, + }, + }, + { + name: "continuous", + input: []*KeyRange{ + {StartKey: []byte("a"), EndKey: []byte("b")}, + {StartKey: []byte("b"), EndKey: []byte("c")}, + }, + expect: []*KeyRange{ + {StartKey: []byte("a"), EndKey: []byte("c")}, + }, + }, + { + name: "boundless 1", + input: []*KeyRange{ + {StartKey: nil, EndKey: []byte("b")}, + {StartKey: []byte("b"), EndKey: []byte("c")}, + }, + expect: []*KeyRange{ + {StartKey: nil, EndKey: []byte("c")}, + }, + }, + { + name: "boundless 2", + input: []*KeyRange{ + {StartKey: []byte("a"), EndKey: []byte("b")}, + {StartKey: []byte("b"), EndKey: nil}, + }, + expect: []*KeyRange{ + {StartKey: []byte("a"), EndKey: nil}, + }, + }, + } + + for _, tc := range testCases { + rs := &KeyRanges{krs: tc.input} + rs.Merge() + re.Equal(tc.expect, rs.Ranges(), tc.name) + } +} diff --git a/pkg/core/region.go b/pkg/core/region.go index f0c78f443bd..eb8b89aecff 100644 --- a/pkg/core/region.go +++ b/pkg/core/region.go @@ -1826,37 +1826,91 @@ func (r *RegionsInfo) ScanRegions(startKey, endKey []byte, limit int) []*RegionI // BatchScanRegions scans regions in given key pairs, returns at most `limit` regions. // limit <= 0 means no limit. // The given key pairs should be non-overlapping. -func (r *RegionsInfo) BatchScanRegions(keyRanges *KeyRanges, limit int) []*RegionInfo { - r.t.RLock() - defer r.t.RUnlock() - +func (r *RegionsInfo) BatchScanRegions(keyRanges *KeyRanges, opts ...BatchScanRegionsOptionFunc) ([]*RegionInfo, error) { + keyRanges.Merge() krs := keyRanges.Ranges() res := make([]*RegionInfo, 0, len(krs)) - var lastRegion *RegionInfo + + scanOptions := &batchScanRegionsOptions{} + for _, opt := range opts { + opt(scanOptions) + } + + r.t.RLock() + defer r.t.RUnlock() for _, keyRange := range krs { - if limit > 0 && len(res) >= limit { - return res + if scanOptions.limit > 0 && len(res) >= scanOptions.limit { + res = res[:scanOptions.limit] + return res, nil } - if lastRegion != nil { - if lastRegion.Contains(keyRange.EndKey) { - continue - } else if lastRegion.Contains(keyRange.StartKey) { - keyRange.StartKey = lastRegion.GetEndKey() - } + + regions, err := scanRegion(r.tree, keyRange, scanOptions.limit, scanOptions.outputMustContainAllKeyRange) + if err != nil { + return nil, err } - r.tree.scanRange(keyRange.StartKey, func(region *RegionInfo) bool { - if len(keyRange.EndKey) > 0 && bytes.Compare(region.GetStartKey(), keyRange.EndKey) >= 0 { - return false - } - if limit > 0 && len(res) >= limit { + if len(res) > 0 && len(regions) > 0 && res[len(res)-1].meta.Id == regions[0].meta.Id { + // skip the region that has been scanned + regions = regions[1:] + } + res = append(res, regions...) + } + return res, nil +} + +func scanRegion(regionTree *regionTree, keyRange *KeyRange, limit int, outputMustContainAllKeyRange bool) ([]*RegionInfo, error) { + var ( + res []*RegionInfo + lastRegion = &RegionInfo{ + meta: &metapb.Region{EndKey: keyRange.StartKey}, + } + exceedLimit = func() bool { return limit > 0 && len(res) >= limit } + err error + ) + regionTree.scanRange(keyRange.StartKey, func(region *RegionInfo) bool { + if len(keyRange.EndKey) > 0 && len(region.GetStartKey()) > 0 && + bytes.Compare(region.GetStartKey(), keyRange.EndKey) >= 0 { + return false + } + if exceedLimit() { + return false + } + if len(lastRegion.GetEndKey()) > 0 && len(region.GetStartKey()) > 0 && + bytes.Compare(region.GetStartKey(), lastRegion.GetEndKey()) > 0 { + 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(), + region.GetStartKey(), region.GetEndKey()) + log.Warn("scan regions failed", zap.Bool("outputMustContainAllKeyRange", + outputMustContainAllKeyRange), zap.Error(err)) + if outputMustContainAllKeyRange { return false } - lastRegion = region - res = append(res, region) - return true - }) + } + + lastRegion = region + res = append(res, region) + return true + }) + if outputMustContainAllKeyRange && err != nil { + return nil, err } - return res + + if !(exceedLimit()) && len(keyRange.EndKey) > 0 && len(lastRegion.GetEndKey()) > 0 && + bytes.Compare(lastRegion.GetEndKey(), keyRange.EndKey) < 0 { + 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) + log.Warn("scan regions failed", zap.Bool("outputMustContainAllKeyRange", + outputMustContainAllKeyRange), zap.Error(err)) + if outputMustContainAllKeyRange { + return nil, err + } + } + + return res, nil } // ScanRegionWithIterator scans from the first region containing or behind start key, diff --git a/pkg/core/region_test.go b/pkg/core/region_test.go index 816bba4efae..845944780e4 100644 --- a/pkg/core/region_test.go +++ b/pkg/core/region_test.go @@ -27,6 +27,7 @@ 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" ) @@ -1141,3 +1142,62 @@ func TestCntRefAfterResetRegionCache(t *testing.T) { regions.CheckAndPutRegion(region) re.Equal(int32(2), region.GetRef()) } + +func TestScanRegion(t *testing.T) { + var ( + re = require.New(t) + tree = newRegionTree() + needContainAllRanges = true + regions []*RegionInfo + err error + ) + scanError := func(startKey, endKey []byte, limit int) { + regions, err = scanRegion(tree, &KeyRange{StartKey: startKey, EndKey: endKey}, limit, needContainAllRanges) + re.Error(err) + } + scanNoError := func(startKey, endKey []byte, limit int) []*RegionInfo { + regions, err = scanRegion(tree, &KeyRange{StartKey: startKey, EndKey: endKey}, limit, needContainAllRanges) + re.NoError(err) + return regions + } + // region1 + // [a, b) + updateNewItem(tree, NewTestRegionInfo(1, 1, []byte("a"), []byte("b"))) + re.Len(scanNoError([]byte("a"), []byte("b"), 0), 1) + scanError([]byte("a"), []byte("c"), 0) + re.Len(scanNoError([]byte("a"), []byte("c"), 1), 1) + + // region1 | region2 + // [a, b) | [b, c) + updateNewItem(tree, NewTestRegionInfo(2, 1, []byte("b"), []byte("c"))) + 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"))) + scanError([]byte("a"), []byte("e"), 0) + scanError([]byte("c"), []byte("e"), 0) + + // region1 | region2 | region3 | region4 + // [a, b) | [b, c) | [d, f) | [f, i) + updateNewItem(tree, NewTestRegionInfo(4, 1, []byte("f"), []byte("i"))) + scanError([]byte("c"), []byte("g"), 0) + re.Len(scanNoError([]byte("g"), []byte("h"), 0), 1) + re.Equal(uint64(4), regions[0].GetID()) + // test error type + scanError([]byte(string('a'-1)), []byte("g"), 0) + re.True(errs.ErrRegionNotAdjacent.Equal(err)) + + // 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) + + // ********** needContainAllRanges = false ********** + // Tests that previously reported errors will no longer report errors. + needContainAllRanges = false + re.Len(scanNoError([]byte("a"), []byte("e"), 0), 3) + re.Len(scanNoError([]byte("c"), []byte("e"), 0), 1) +} diff --git a/server/grpc_service.go b/server/grpc_service.go index d3f58dfe1ab..7b18be47fde 100644 --- 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" @@ -1680,7 +1681,22 @@ func (s *GrpcServer) BatchScanRegions(ctx context.Context, request *pdpb.BatchSc } keyRanges.Append(reqRange.StartKey, reqRange.EndKey) } - res := rc.BatchScanRegions(keyRanges, int(limit)) + + scanOptions := []core.BatchScanRegionsOptionFunc{core.WithLimit(int(limit))} + if request.ContainAllKeyRange { + scanOptions = append(scanOptions, core.WithOutputMustContainAllKeyRange()) + } + res, err := rc.BatchScanRegions(keyRanges, scanOptions...) + if err != 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 { leader := r.GetLeader() diff --git a/tests/integrations/client/client_test.go b/tests/integrations/client/client_test.go index 2e51c7080e9..e2cb2758b78 100644 --- a/tests/integrations/client/client_test.go +++ b/tests/integrations/client/client_test.go @@ -2001,9 +2001,13 @@ func waitLeaderChange(re *require.Assertions, cluster *tests.TestCluster, old st } func (suite *clientTestSuite) TestBatchScanRegions() { - re := suite.Require() - regionLen := 10 - regions := make([]*metapb.Region, 0, regionLen) + var ( + re = suite.Require() + ctx = context.Background() + regionLen = 10 + regions = make([]*metapb.Region, 0, regionLen) + ) + for i := 0; i < regionLen; i++ { regionID := regionIDAllocator.alloc() r := &metapb.Region{ @@ -2028,7 +2032,7 @@ func (suite *clientTestSuite) TestBatchScanRegions() { // Wait for region heartbeats. testutil.Eventually(re, func() bool { - scanRegions, err := suite.client.BatchScanRegions(context.Background(), []pd.KeyRange{{StartKey: []byte{0}, EndKey: nil}}, 10) + scanRegions, err := suite.client.BatchScanRegions(ctx, []pd.KeyRange{{StartKey: []byte{0}, EndKey: nil}}, 10) return err == nil && len(scanRegions) == 10 }) @@ -2049,39 +2053,45 @@ func (suite *clientTestSuite) TestBatchScanRegions() { suite.srv.GetRaftCluster().HandleRegionHeartbeat(region6) t := suite.T() + var outputMustContainAllKeyRangeOptions []bool check := func(ranges []pd.KeyRange, limit int, expect []*metapb.Region) { for _, bucket := range []bool{false, true} { - var opts []pd.GetRegionOption - if bucket { - opts = append(opts, pd.WithBuckets()) - } - scanRegions, err := suite.client.BatchScanRegions(context.Background(), ranges, limit, opts...) - re.NoError(err) - re.Len(scanRegions, len(expect)) - t.Log("scanRegions", scanRegions) - t.Log("expect", expect) - for i := range expect { - re.Equal(expect[i], scanRegions[i].Meta) - - if scanRegions[i].Meta.GetId() == region3.GetID() { - re.Equal(&metapb.Peer{}, scanRegions[i].Leader) - } else { - re.Equal(expect[i].Peers[0], scanRegions[i].Leader) + for _, outputMustContainAllKeyRange := range outputMustContainAllKeyRangeOptions { + var opts []pd.GetRegionOption + if bucket { + opts = append(opts, pd.WithBuckets()) } - - if scanRegions[i].Meta.GetId() == region4.GetID() { - re.Equal([]*metapb.Peer{expect[i].Peers[1]}, scanRegions[i].DownPeers) + if outputMustContainAllKeyRange { + opts = append(opts, pd.WithOutputMustContainAllKeyRange()) } + scanRegions, err := suite.client.BatchScanRegions(ctx, ranges, limit, opts...) + re.NoError(err) + t.Log("scanRegions", scanRegions) + t.Log("expect", expect) + re.Len(scanRegions, len(expect)) + for i := range expect { + re.Equal(expect[i], scanRegions[i].Meta) + + if scanRegions[i].Meta.GetId() == region3.GetID() { + re.Equal(&metapb.Peer{}, scanRegions[i].Leader) + } else { + re.Equal(expect[i].Peers[0], scanRegions[i].Leader) + } - if scanRegions[i].Meta.GetId() == region5.GetID() { - re.Equal([]*metapb.Peer{expect[i].Peers[1], expect[i].Peers[2]}, scanRegions[i].PendingPeers) - } + if scanRegions[i].Meta.GetId() == region4.GetID() { + re.Equal([]*metapb.Peer{expect[i].Peers[1]}, scanRegions[i].DownPeers) + } - if scanRegions[i].Meta.GetId() == region6.GetID() { - if !bucket { - re.Nil(scanRegions[i].Buckets) - } else { - re.Equal(scanRegions[i].Buckets, region6.GetBuckets()) + if scanRegions[i].Meta.GetId() == region5.GetID() { + re.Equal([]*metapb.Peer{expect[i].Peers[1], expect[i].Peers[2]}, scanRegions[i].PendingPeers) + } + + if scanRegions[i].Meta.GetId() == region6.GetID() { + if !bucket { + re.Nil(scanRegions[i].Buckets) + } else { + re.Equal(scanRegions[i].Buckets, region6.GetBuckets()) + } } } } @@ -2089,6 +2099,7 @@ func (suite *clientTestSuite) TestBatchScanRegions() { } // valid ranges + outputMustContainAllKeyRangeOptions = []bool{false, true} check([]pd.KeyRange{{StartKey: []byte{0}, EndKey: nil}}, 10, regions) check([]pd.KeyRange{{StartKey: []byte{1}, EndKey: nil}}, 5, regions[1:6]) check([]pd.KeyRange{ @@ -2105,6 +2116,8 @@ func (suite *clientTestSuite) TestBatchScanRegions() { {StartKey: []byte{6}, EndKey: []byte{7}}, {StartKey: []byte{8}, EndKey: []byte{9}}, }, 3, []*metapb.Region{regions[0], regions[2], regions[4]}) + + outputMustContainAllKeyRangeOptions = []bool{false} check([]pd.KeyRange{ {StartKey: []byte{0}, EndKey: []byte{0, 1}}, // non-continuous ranges in a region {StartKey: []byte{0, 2}, EndKey: []byte{0, 3}}, @@ -2112,14 +2125,56 @@ func (suite *clientTestSuite) TestBatchScanRegions() { {StartKey: []byte{0, 5}, EndKey: []byte{0, 6}}, {StartKey: []byte{0, 7}, EndKey: []byte{3}}, {StartKey: []byte{4}, EndKey: []byte{5}}, - }, 2, []*metapb.Region{regions[0], regions[1]}) + }, 10, []*metapb.Region{regions[0], regions[1], regions[2], regions[4]}) + outputMustContainAllKeyRangeOptions = []bool{false} + check([]pd.KeyRange{ + {StartKey: []byte{9}, EndKey: []byte{10, 1}}, + }, 10, []*metapb.Region{regions[9]}) // invalid ranges - _, err := suite.client.BatchScanRegions(context.Background(), []pd.KeyRange{{StartKey: []byte{1}, EndKey: []byte{0}}}, 10) - re.Error(err, "invalid key range, start key > end key") - _, err = suite.client.BatchScanRegions(context.Background(), []pd.KeyRange{ + _, err := suite.client.BatchScanRegions( + ctx, + []pd.KeyRange{{StartKey: []byte{1}, EndKey: []byte{0}}}, + 10, + pd.WithOutputMustContainAllKeyRange(), + ) + re.ErrorContains(err, "invalid key range, start key > end key") + _, err = suite.client.BatchScanRegions(ctx, []pd.KeyRange{ {StartKey: []byte{0}, EndKey: []byte{2}}, {StartKey: []byte{1}, EndKey: []byte{3}}, }, 10) - re.Error(err, "invalid key range, ranges overlapped") + re.ErrorContains(err, "invalid key range, ranges overlapped") + _, err = suite.client.BatchScanRegions( + ctx, + []pd.KeyRange{{StartKey: []byte{9}, EndKey: []byte{10, 1}}}, + 10, + pd.WithOutputMustContainAllKeyRange(), + ) + re.ErrorContains(err, "found a hole region in the last") + req := &pdpb.RegionHeartbeatRequest{ + Header: newHeader(suite.srv), + Region: &metapb.Region{ + Id: 100, + RegionEpoch: &metapb.RegionEpoch{ + ConfVer: 1, + Version: 1, + }, + StartKey: []byte{100}, + EndKey: []byte{101}, + Peers: peers, + }, + Leader: peers[0], + } + re.NoError(suite.regionHeartbeat.Send(req)) + + // Wait for region heartbeats. + testutil.Eventually(re, func() bool { + _, err = suite.client.BatchScanRegions( + ctx, + []pd.KeyRange{{StartKey: []byte{9}, EndKey: []byte{101}}}, + 10, + pd.WithOutputMustContainAllKeyRange(), + ) + return err != nil && strings.Contains(err.Error(), "found a hole region between") + }) } diff --git a/tests/integrations/go.mod b/tests/integrations/go.mod index 8a570d52458..a9f996417a4 100644 --- a/tests/integrations/go.mod +++ b/tests/integrations/go.mod @@ -14,7 +14,7 @@ require ( github.com/go-sql-driver/mysql v1.7.0 github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c - github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4 + github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7 github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 github.com/prometheus/client_golang v1.19.0 github.com/prometheus/client_model v0.6.0 diff --git a/tests/integrations/go.sum b/tests/integrations/go.sum index c88919f6571..b46c01e77cc 100644 --- a/tests/integrations/go.sum +++ b/tests/integrations/go.sum @@ -368,8 +368,8 @@ github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c/go.mod h1:X2r9ue github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c h1:CgbKAHto5CQgWM9fSBIvaxsJHuGP0uM74HXtv3MyyGQ= github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c/go.mod h1:4qGtCB0QK0wBzKtFEGDhxXnSnbQApw1gc9siScUl8ew= github.com/pingcap/kvproto v0.0.0-20191211054548-3c6b38ea5107/go.mod h1:WWLmULLO7l8IOcQG+t+ItJ3fEcrL5FxF0Wu+HrMy26w= -github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4 h1:6aIKNB2YGAec4IUDLw6G2eDECiGiufZcgEbZSCELBx0= -github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= +github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7 h1:V9XS3FQ/P6u+kFaoSyY5DBswIA774BMpIOLDBMrpxKc= +github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= github.com/pingcap/log v0.0.0-20210625125904-98ed8e2eb1c7/go.mod h1:8AanEdAHATuRurdGxZXBz0At+9avep+ub7U1AGYLIMM= github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 h1:HR/ylkkLmGdSSDaD8IDP+SZrdhV1Kibl9KrHxJ9eciw= github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3/go.mod h1:DWQW5jICDR7UJh4HtxXSM20Churx4CQL0fwL/SoOSA4= diff --git a/tools/go.mod b/tools/go.mod index f424f12458e..af187b4999c 100644 --- a/tools/go.mod +++ b/tools/go.mod @@ -22,7 +22,7 @@ require ( github.com/mattn/go-shellwords v1.0.12 github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c github.com/pingcap/failpoint v0.0.0-20210918120811-547c13e3eb00 - github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4 + github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7 github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.19.0 diff --git a/tools/go.sum b/tools/go.sum index c2656b3e656..f508ca92384 100644 --- a/tools/go.sum +++ b/tools/go.sum @@ -365,8 +365,8 @@ github.com/pingcap/errors v0.11.5-0.20211224045212-9687c2b0f87c/go.mod h1:X2r9ue github.com/pingcap/failpoint v0.0.0-20210918120811-547c13e3eb00 h1:C3N3itkduZXDZFh4N3vQ5HEtld3S+Y+StULhWVvumU0= github.com/pingcap/failpoint v0.0.0-20210918120811-547c13e3eb00/go.mod h1:4qGtCB0QK0wBzKtFEGDhxXnSnbQApw1gc9siScUl8ew= github.com/pingcap/kvproto v0.0.0-20191211054548-3c6b38ea5107/go.mod h1:WWLmULLO7l8IOcQG+t+ItJ3fEcrL5FxF0Wu+HrMy26w= -github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4 h1:6aIKNB2YGAec4IUDLw6G2eDECiGiufZcgEbZSCELBx0= -github.com/pingcap/kvproto v0.0.0-20240620063548-118a4cab53e4/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= +github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7 h1:V9XS3FQ/P6u+kFaoSyY5DBswIA774BMpIOLDBMrpxKc= +github.com/pingcap/kvproto v0.0.0-20240716095229-5f7ffec83ea7/go.mod h1:rXxWk2UnwfUhLXha1jxRWPADw9eMZGWEWCg92Tgmb/8= github.com/pingcap/log v0.0.0-20210625125904-98ed8e2eb1c7/go.mod h1:8AanEdAHATuRurdGxZXBz0At+9avep+ub7U1AGYLIMM= github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3 h1:HR/ylkkLmGdSSDaD8IDP+SZrdhV1Kibl9KrHxJ9eciw= github.com/pingcap/log v1.1.1-0.20221110025148-ca232912c9f3/go.mod h1:DWQW5jICDR7UJh4HtxXSM20Churx4CQL0fwL/SoOSA4=