From b5f6ef6ab76b8cb024788e7d0aaa0bad510259da Mon Sep 17 00:00:00 2001 From: JmPotato Date: Tue, 25 Jun 2024 17:52:21 +0800 Subject: [PATCH] This is an automated cherry-pick of #8324 close tikv/pd#8323 Signed-off-by: ti-chi-bot --- pkg/schedule/config/store_config_test.go | 75 ++++++++++++++++++++++++ pkg/utils/typeutil/size_test.go | 19 ++++-- server/config/store_config.go | 9 ++- 3 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 pkg/schedule/config/store_config_test.go diff --git a/pkg/schedule/config/store_config_test.go b/pkg/schedule/config/store_config_test.go new file mode 100644 index 00000000000..3f4789a74c9 --- /dev/null +++ b/pkg/schedule/config/store_config_test.go @@ -0,0 +1,75 @@ +// Copyright 2022 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 config + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestMergeCheck(t *testing.T) { + re := require.New(t) + testdata := []struct { + size uint64 + mergeSize uint64 + keys uint64 + mergeKeys uint64 + pass bool + }{{ + // case 1: the merged region size is smaller than the max region size + size: 96 + 20, + mergeSize: 20, + keys: 1440000 + 200000, + mergeKeys: 200000, + pass: true, + }, { + // case 2: the smallest region is 68MiB, it can't be merged again. + size: 144 + 20, + mergeSize: 20, + keys: 1440000 + 200000, + mergeKeys: 200000, + pass: true, + }, { + // case 3: the smallest region is 50MiB, it can be merged again. + size: 144 + 2, + mergeSize: 50, + keys: 1440000 + 20000, + mergeKeys: 500000, + pass: false, + }, { + // case4: the smallest region is 51MiB, it can't be merged again. + size: 144 + 3, + mergeSize: 50, + keys: 1440000 + 30000, + mergeKeys: 500000, + pass: true, + }} + config := &StoreConfig{} + for _, v := range testdata { + if v.pass { + re.NoError(config.CheckRegionSize(v.size, v.mergeSize)) + re.NoError(config.CheckRegionKeys(v.keys, v.mergeKeys)) + } else { + re.Error(config.CheckRegionSize(v.size, v.mergeSize)) + re.Error(config.CheckRegionKeys(v.keys, v.mergeKeys)) + } + } + // Test CheckRegionSize when the region split size is 0. + config.RegionSplitSize = "100KiB" + config.Adjust() + re.Empty(config.GetRegionSplitSize()) + re.NoError(config.CheckRegionSize(defaultRegionMaxSize, 50)) +} diff --git a/pkg/utils/typeutil/size_test.go b/pkg/utils/typeutil/size_test.go index 57c246953e4..1c2aa2b217a 100644 --- a/pkg/utils/typeutil/size_test.go +++ b/pkg/utils/typeutil/size_test.go @@ -40,25 +40,36 @@ func TestSizeJSON(t *testing.T) { } func TestParseMbFromText(t *testing.T) { +<<<<<<< HEAD t.Parallel() +======= + const defaultValue = 2 + +>>>>>>> 6fbe73796 (config: fix the panic caused by zero RegionSplitSizeMB (#8324)) re := require.New(t) testCases := []struct { body []string size uint64 }{{ body: []string{"10Mib", "10MiB", "10M", "10MB"}, - size: uint64(10), + size: 10, }, { body: []string{"10GiB", "10Gib", "10G", "10GB"}, - size: uint64(10 * units.GiB / units.MiB), + size: 10 * units.GiB / units.MiB, + }, { + body: []string{"1024KiB", "1048576"}, + size: 1, + }, { + body: []string{"100KiB", "1023KiB", "1048575", "0"}, + size: 0, }, { body: []string{"10yiB", "10aib"}, - size: uint64(1), + size: defaultValue, }} for _, testCase := range testCases { for _, b := range testCase.body { - re.Equal(int(testCase.size), int(ParseMBFromText(b, 1))) + re.Equal(testCase.size, ParseMBFromText(b, defaultValue)) } } } diff --git a/server/config/store_config.go b/server/config/store_config.go index 8f4baa91522..b7ad34a6b51 100644 --- a/server/config/store_config.go +++ b/server/config/store_config.go @@ -164,9 +164,14 @@ func (c *StoreConfig) CheckRegionSize(size, mergeSize uint64) error { if size < c.GetRegionMaxSize() { return nil } - + // This could happen when the region split size is set to a value less than 1MiB, + // which is a very extreme case, we just pass the check here to prevent panic. + regionSplitSize := c.GetRegionSplitSize() + if regionSplitSize == 0 { + return nil + } // the smallest of the split regions can not be merge again, so it's size should less merge size. - if smallSize := size % c.GetRegionSplitSize(); smallSize <= mergeSize && smallSize != 0 { + if smallSize := size % regionSplitSize; smallSize <= mergeSize && smallSize != 0 { log.Debug("region size is too small", zap.Uint64("size", size), zap.Uint64("merge-size", mergeSize), zap.Uint64("small-size", smallSize)) return errs.ErrCheckerMergeAgain.FastGenByArgs("the smallest region of the split regions is less than max-merge-region-size, " + "it will be merged again")