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

checker: fix rule checker panic (#2160) #2161

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented Feb 26, 2020

cherry-pick #2160 to release-3.1


Signed-off-by: disksing i@disksing.com

What problem does this PR solve?

when no more store to replace, it panics:

[2020/02/26 11:46:45.244 +08:00] [FATAL] [log.go:294] [panic] [recover="\"invalid memory address or nil pointer dereference\""] [stack="github.com/pingcap/log.Fatal\n\t/home/birdstorm/go/pkg/mod/github.com/pingcap/log@v0.0.0-2020011      7041106-d28c14d3b1cd/global.go:59\ngit.luolix.top/pingcap/pd/v3/pkg/logutil.LogPanic\n\t/home/birdstorm/go/src/github.com/pingcap/pd/pkg/logutil/log.go:294\nruntime.gopanic\n\t/usr/local/go/src/runtime/panic.go:679\nruntime.panicmem\n\t      /usr/local/go/src/runtime/panic.go:199\nruntime.sigpanic\n\t/usr/local/go/src/runtime/signal_unix.go:394\ngit.luolix.top/pingcap/kvproto/pkg/metapb.(*Store).GetId\n\t/home/birdstorm/go/pkg/mod/github.com/pingcap/kvproto@v0.0.0-202002130      74014-83e827908584/pkg/metapb/metapb.pb.go:221\ngit.luolix.top/pingcap/pd/v3/server/core.DistinctScore\n\t/home/birdstorm/go/src/github.com/pingcap/pd/server/core/store.go:453\ngit.luolix.top/pingcap/pd/v3/server/schedule/checker.(*RuleChec      ker).fixBetterLocation\n\t/home/birdstorm/go/src/github.com/pingcap/pd/server/schedule/checker/rule_checker.go:187\ngit.luolix.top/pingcap/pd/v3/server/schedule/checker.(*RuleChecker).fixRulePeer\n\t/home/birdstorm/go/src/github.com/pin      gcap/pd/server/schedule/checker/rule_checker.go:110\ngit.luolix.top/pingcap/pd/v3/server/schedule/checker.(*RuleChecker).Check\n\t/home/birdstorm/go/src/github.com/pingcap/pd/server/schedule/checker/rule_checker.go:59\ngit.luolix.top/pingca      p/pd/v3/server/schedule.(*CheckerController).CheckRegion\n\t/home/birdstorm/go/src/github.com/pingcap/pd/server/schedule/checker_controller.go:58\ngit.luolix.top/pingcap/pd/v3/server/cluster.(*coordinator).patrolRegions\n\t/home/birdsto      rm/go/src/github.com/pingcap/pd/server/cluster/coordinator.go:125"]

What is changed and how it works?

  • check nil
  • add unit test

Check List

Tests

  • Unit test

Related changes

  • Need to cherry-pick to the release branch (release-3.1)

Signed-off-by: disksing <i@disksing.com>
@codecov-io
Copy link

Codecov Report

Merging #2161 into release-3.1 will decrease coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release-3.1    #2161      +/-   ##
===============================================
- Coverage        74.94%   74.87%   -0.08%     
===============================================
  Files              194      194              
  Lines            19982    19985       +3     
===============================================
- Hits             14975    14963      -12     
- Misses            3875     3891      +16     
+ Partials          1132     1131       -1
Impacted Files Coverage Δ
server/schedule/checker/rule_checker.go 76.07% <100%> (+0.44%) ⬆️
server/region_syncer/client.go 81.03% <0%> (-3.45%) ⬇️
server/kv/etcd_kv.go 72.72% <0%> (-2.6%) ⬇️
server/core/storage.go 71.85% <0%> (-1.01%) ⬇️
server/config/option.go 87.44% <0%> (-0.87%) ⬇️
server/grpc_service.go 56.65% <0%> (-0.86%) ⬇️
pkg/btree/btree.go 86.84% <0%> (-0.81%) ⬇️
server/server.go 64.32% <0%> (-0.72%) ⬇️
server/handler.go 51.61% <0%> (+0.43%) ⬆️
server/member/leader.go 76.53% <0%> (+1.02%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 824ae7f...75db5d4. Read the comment docs.

Copy link
Member

@rleungx rleungx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nolouch
Copy link
Contributor

nolouch commented Feb 26, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 26, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented Feb 26, 2020

/run-all-tests

@sre-bot sre-bot merged commit 7f88319 into tikv:release-3.1 Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/schedule Scheduling logic. status/can-merge Indicates a PR has been approved by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants