Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Size configuration on the TIKV side may cause PD panic #8323

Closed
JmPotato opened this issue Jun 25, 2024 · 0 comments · Fixed by #8324
Closed

Size configuration on the TIKV side may cause PD panic #8323

JmPotato opened this issue Jun 25, 2024 · 0 comments · Fixed by #8324
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. impact/panic severity/major type/bug The issue is confirmed as a bug.

Comments

@JmPotato
Copy link
Member

JmPotato commented Jun 25, 2024

Bug Report

If set coprocessor.region-split-size to a value that is less than 1 MiB, it will cause PD panic due to the following code:

if smallSize := size % c.GetRegionSplitSize(); smallSize <= mergeSize && smallSize != 0 {

GetRegionSplitSize() will return a zero value parsed by ParseMBFromText(), which will return 0 if the given size is less than 1 MiB.

// ParseMBFromText parses MB from text.
func ParseMBFromText(text string, value uint64) uint64 {
b := ByteSize(0)
err := b.UnmarshalText([]byte(text))
if err != nil {
return value
}
return uint64(b / units.MiB)
}

[2024/06/25 14:30:37.095 +08:00] [FATAL] [log.go:72] [panic] [recover="\"integer divide by zero\""] [stack="[github.com/tikv/pd/pkg/logutil.LogPanic\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/pkg/logutil/log.go:72\nruntime.gopanic\n\t/usr/local/go/src/runtime/panic.go:884\nruntime.panicdivide\n\t/usr/local/go/src/runtime/panic.go:239\ngithub.com/tikv/pd/server/config.(*StoreConfig).CheckRegionSize\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/config/store_config.go:143\ngithub.com/tikv/pd/server/schedule/checker.(*MergeChecker).Check\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/schedule/checker/merge_checker.go:158\ngithub.com/tikv/pd/server/schedule/checker.(*Controller).CheckRegion\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/schedule/checker/checker_controller.go:124\ngithub.com/tikv/pd/server/cluster.(*coordinator).tryAddOperators\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/cluster/coordinator.go:265\ngithub.com/tikv/pd/server/cluster.(*coordinator).checkRegions\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/cluster/coordinator.go:168\ngithub.com/tikv/pd/server/cluster.(*coordinator).patrolRegions\n\t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/cluster/coordinator.go:143](http://github.com/tikv/pd/pkg/logutil.LogPanic/n/t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/pkg/logutil/log.go:72/nruntime.gopanic/n/t/usr/local/go/src/runtime/panic.go:884/nruntime.panicdivide/n/t/usr/local/go/src/runtime/panic.go:239/ngithub.com/tikv/pd/server/config.(*StoreConfig).CheckRegionSize/n/t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/config/store_config.go:143/ngithub.com/tikv/pd/server/schedule/checker.(*MergeChecker).Check/n/t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/schedule/checker/merge_checker.go:158/ngithub.com/tikv/pd/server/schedule/checker.(*Controller).CheckRegion/n/t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/schedule/checker/checker_controller.go:124/ngithub.com/tikv/pd/server/cluster.(*coordinator).tryAddOperators/n/t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/cluster/coordinator.go:265/ngithub.com/tikv/pd/server/cluster.(*coordinator).checkRegions/n/t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/cluster/coordinator.go:168/ngithub.com/tikv/pd/server/cluster.(*coordinator).patrolRegions/n/t/home/jenkins/agent/workspace/build-common/go/src/github.com/pingcap/pd/server/cluster/coordinator.go:143)"]
@JmPotato JmPotato added type/bug The issue is confirmed as a bug. severity/major labels Jun 25, 2024
@JmPotato JmPotato added affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. and removed may-affects-5.4 may-affects-6.1 may-affects-6.5 may-affects-7.1 may-affects-7.5 may-affects-8.1 labels Jun 25, 2024
ti-chi-bot bot pushed a commit that referenced this issue Jun 25, 2024
close #8323

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

Signed-off-by: JmPotato <ghzpotato@gmail.com>
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 25, 2024
close tikv#8323

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 25, 2024
close tikv#8323

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 25, 2024
close tikv#8323

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 25, 2024
close tikv#8323

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@JmPotato JmPotato added the affects-6.1 This bug affects the 6.1.x(LTS) versions. label Jun 25, 2024
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this issue Jun 25, 2024
close tikv#8323

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot bot pushed a commit that referenced this issue Jun 26, 2024
close #8323

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
Signed-off-by: JmPotato <ghzpotato@gmail.com>

Co-authored-by: JmPotato <ghzpotato@gmail.com>
ti-chi-bot bot pushed a commit that referenced this issue Jun 26, 2024
close #8323

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
Signed-off-by: JmPotato <ghzpotato@gmail.com>

Co-authored-by: JmPotato <ghzpotato@gmail.com>
ti-chi-bot bot pushed a commit that referenced this issue Jun 26, 2024
close #8323

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
Signed-off-by: JmPotato <ghzpotato@gmail.com>

Co-authored-by: JmPotato <ghzpotato@gmail.com>
ti-chi-bot bot pushed a commit that referenced this issue Jun 26, 2024
close #8323

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
Signed-off-by: JmPotato <ghzpotato@gmail.com>

Co-authored-by: JmPotato <ghzpotato@gmail.com>
ti-chi-bot bot pushed a commit that referenced this issue Jun 26, 2024
close #8323

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
Signed-off-by: JmPotato <ghzpotato@gmail.com>

Co-authored-by: JmPotato <ghzpotato@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. impact/panic severity/major type/bug The issue is confirmed as a bug.
Projects
Development

Successfully merging a pull request may close this issue.

2 participants