Skip to content

Commit

Permalink
config: fix the panic caused by zero RegionSplitSizeMB (#8324) (#8328)
Browse files Browse the repository at this point in the history
close #8323

Bypass the case that `RegionSplitSizeMB` might be zero in `(*StoreConfig) CheckRegionSize`.

Signed-off-by: ti-chi-bot <[email protected]>
Signed-off-by: JmPotato <[email protected]>

Co-authored-by: JmPotato <[email protected]>
  • Loading branch information
ti-chi-bot and JmPotato authored Jun 26, 2024
1 parent 22c5c9e commit 50c70af
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 6 deletions.
16 changes: 12 additions & 4 deletions pkg/utils/typeutil/size_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,25 +40,33 @@ func TestSizeJSON(t *testing.T) {
}

func TestParseMbFromText(t *testing.T) {
const defaultValue = 2

t.Parallel()
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))
}
}
}
9 changes: 7 additions & 2 deletions server/config/store_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 6 additions & 0 deletions server/config/store_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/docker/go-units"
"github.com/stretchr/testify/require"
"github.com/tikv/pd/pkg/utils/typeutil"
)

func TestTiKVConfig(t *testing.T) {
Expand Down Expand Up @@ -165,4 +166,9 @@ func TestMergeCheck(t *testing.T) {
re.Error(config.CheckRegionKeys(v.keys, v.mergeKeys))
}
}
// Test CheckRegionSize when the region split size is 0.
config.RegionSplitSize = "100KiB"
config.RegionSplitSizeMB = typeutil.ParseMBFromText(config.RegionSplitSize, defaultRegionSplitSize)
re.Empty(config.GetRegionSplitSize())
re.NoError(config.CheckRegionSize(defaultRegionMaxSize, 50))
}

0 comments on commit 50c70af

Please sign in to comment.