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

fix test_follower_slow_split #3797

Merged
merged 7 commits into from Nov 26, 2018

Conversation

Projects
None yet
7 participants
@hicqu
Contributor

hicqu commented Nov 16, 2018

Signed-off-by: qupeng qupeng@pingcap.com

What have you changed? (mandatory)

Fix test case test_follower_slow_split.

The case failed because we didn't call disable_default_operator. So after we removed a peer from a cluster with 3 replicas, the expected peer count is 2, but pd will add a new peer back immediatly so it's still 3. So the test case failed.

What are the type of the changes? (mandatory)

Improvement.

How has this PR been tested? (mandatory)

make dev.

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

No.

Does this PR affect tidb-ansible update? (mandatory)

No.

Refer to a related PR or issue link (optional)

Close #3793 .

fix test_follower_slow_split
Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu

This comment has been minimized.

Contributor

hicqu commented Nov 16, 2018

Using some println to show the reason.
Without disable_default_operator:

req.peers: [1002, 1003, 1004], self.peers: [id: 1 store_id: 1, id: 3 store_id: 3, id: 1000 store_id: 2]
req.peers: [1002, 1003, 1004], self.peers: [id: 1 store_id: 1, id: 3 store_id: 3, id: 1000 store_id: 2]

With disable_default_operator:

req.peers: [1001, 1002], self.peers: [id: 1 store_id: 1, id: 3 store_id: 3]
req.peers: [1001, 1002], self.peers: [id: 1 store_id: 1, id: 3 store_id: 3]

@hicqu hicqu requested review from breeswish and overvenus Nov 16, 2018

@hicqu

This comment has been minimized.

Contributor

hicqu commented Nov 16, 2018

Split shouldn't pass check_region_epoch if conf version doesn't match. So We need to fix this behavior and add an another test case.

fix split after conf chagne. should add a new test case.
Signed-off-by: qupeng <qupeng@pingcap.com>
remove useless test case.
Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu

This comment has been minimized.

Contributor

hicqu commented Nov 19, 2018

PTAL @BusyJay .

Show resolved Hide resolved src/raftstore/store/util.rs Outdated
@zhangjinpeng1987

This comment has been minimized.

Member

zhangjinpeng1987 commented Nov 20, 2018

Please split into two different PRs.

@hicqu

This comment has been minimized.

Contributor

hicqu commented Nov 20, 2018

Things about conf version is moved to #3807 . Please take a look @zhangjinpeng1987 .

Merge branch 'master' into fix-slow-split
Signed-off-by: qupeng <qupeng@pingcap.com>
@hicqu

This comment has been minimized.

Contributor

hicqu commented Nov 22, 2018

ping @BusyJay PTAL again, thanks.

@BusyJay

This comment has been minimized.

Contributor

BusyJay commented Nov 22, 2018

How can it still fail after #3807 ?

@hicqu

This comment has been minimized.

Contributor

hicqu commented Nov 22, 2018

@BusyJay The test success depends on MsgRequestPreVote count. So if there are 3 peers instead 2, they will send more such messages than the number expected. So we still needs disable_default_operator.

@MyonKeminta

LGTM

@overvenus

LGTM

@overvenus overvenus merged commit 38a4c6d into tikv:master Nov 26, 2018

3 checks passed

DCO All commits are signed off!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
jenkins-ci-tikv/build Jenkins job succeeded.
Details

@hicqu hicqu deleted the hicqu:fix-slow-split branch Nov 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment