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.

Sign up for GitHub

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

schedule: prevent removing voters directly in 2 replica raft group #4564

Merged
merged 5 commits into from
Jan 13, 2022
Merged

schedule: prevent removing voters directly in 2 replica raft group #4564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e594366
schedule: prevent removing voters directly in 2 replica raft group
HunDunDM Jan 12, 2022
ad46562
fix test
HunDunDM Jan 12, 2022
27d477e
add test
HunDunDM Jan 12, 2022
ee2a3a1
Merge branch 'master' into fix-4411
ti-chi-bot Jan 13, 2022
2788c41
add name
HunDunDM Jan 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions server/schedule/operator/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,10 @@ func (b *Builder) prepareBuild() (string, error) {
}
}

if len(b.toAdd)+len(b.toRemove)+len(b.toPromote) <= 1 && len(b.toDemote) == 0 {
// if only one peer changed and the change type is not demote, joint consensus is not used
if len(b.toAdd)+len(b.toRemove)+len(b.toPromote) <= 1 && len(b.toDemote) == 0 &&
!(len(b.toRemove) == 1 && len(b.targetPeers) == 1) {
// If only one peer changed and the change type is not demote, joint consensus is not used.
// Unless the changed is 2 voters to 1 voter, see https://github.com/tikv/pd/issues/4411 .
b.useJointConsensus = false
}

Expand Down
79 changes: 55 additions & 24 deletions server/schedule/operator/builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,39 +176,39 @@ func (s *testBuilderSuite) TestBuild(c *C) {
}
cases := []testCase{
{
"(disable JointConcensus) empty step",
"(disable JointConsensus) empty step",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
0,
[]OpStep{},
},
{
"(enable JointConcensus) empty step",
"(enable JointConsensus) empty step",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
0,
[]OpStep{},
},
{
"(disable JointConcensus) no valid leader",
"(disable JointConsensus) no valid leader",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1}},
[]*metapb.Peer{{Id: 10, StoreId: 10}},
0,
[]OpStep{},
},
{
"(enable JointConcensus) no valid leader",
"(enable JointConsensus) no valid leader",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}},
[]*metapb.Peer{{Id: 10, StoreId: 10}},
0,
[]OpStep{},
},
{
"(disable JointConcensus) promote 1 learner and transfer leader",
"(disable JointConsensus) promote 1 learner and transfer leader",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{Id: 2, StoreId: 2}, {Id: 1, StoreId: 1}},
Expand All @@ -219,7 +219,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) promote 1 learner and transfer leader",
"(enable JointConsensus) promote 1 learner and transfer leader",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{Id: 2, StoreId: 2}, {Id: 1, StoreId: 1}},
Expand All @@ -230,7 +230,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(disable JointConcensus) prefer replace",
"(disable JointConsensus) prefer replace",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{StoreId: 4}, {StoreId: 5, Role: metapb.PeerRole_Learner}},
Expand All @@ -246,7 +246,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) transfer leader in joint state",
"(enable JointConsensus) transfer leader in joint state",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{StoreId: 4}, {StoreId: 5, Role: metapb.PeerRole_Learner}},
Expand All @@ -269,7 +269,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(disable JointConcensus) transfer leader before remove leader",
"(disable JointConsensus) transfer leader before remove leader",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1}},
[]*metapb.Peer{{StoreId: 2}},
Expand All @@ -282,7 +282,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) transfer leader in joint state",
"(enable JointConsensus) transfer leader in joint state",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}},
[]*metapb.Peer{{StoreId: 2}},
Expand All @@ -302,7 +302,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(disable JointConcensus) replace voter with learner",
"(disable JointConsensus) replace voter with learner",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2, Role: metapb.PeerRole_Learner}},
Expand All @@ -313,7 +313,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) demote 1 peer directly",
"(enable JointConsensus) demote 1 peer directly",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
[]*metapb.Peer{{StoreId: 1}, {StoreId: 2, Role: metapb.PeerRole_Learner}},
Expand All @@ -326,7 +326,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(disable JointConcensus) prefer replace with nearest peer",
"(disable JointConsensus) prefer replace with nearest peer",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 6, StoreId: 6}, {Id: 8, StoreId: 8}},
// z1,h1 z1,h2 z2,h1
Expand All @@ -352,7 +352,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(disable JointConcensus) promote learner + demote voter + add learner",
"(disable JointConsensus) promote learner + demote voter + add learner",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1, Role: metapb.PeerRole_Voter}, {Id: 2, StoreId: 2, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{Id: 2, StoreId: 2, Role: metapb.PeerRole_Voter}, {Id: 1, StoreId: 1, Role: metapb.PeerRole_Learner}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
Expand All @@ -366,7 +366,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(disable JointConcensus) add learner + promote learner + remove voter",
"(disable JointConsensus) add learner + promote learner + remove voter",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1, Role: metapb.PeerRole_Voter}, {Id: 2, StoreId: 2, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{Id: 2, StoreId: 2, Role: metapb.PeerRole_Voter}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
Expand All @@ -379,7 +379,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(disable JointConcensus) add voter + demote voter + remove learner",
"(disable JointConsensus) add voter + demote voter + remove learner",
false,
[]*metapb.Peer{{Id: 1, StoreId: 1, Role: metapb.PeerRole_Voter}, {Id: 2, StoreId: 2, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{Id: 3, StoreId: 3, Role: metapb.PeerRole_Voter}, {Id: 1, StoreId: 1, Role: metapb.PeerRole_Learner}},
Expand All @@ -394,7 +394,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) transfer leader before entering joint state",
"(enable JointConsensus) transfer leader before entering joint state",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{Id: 2, StoreId: 2}, {Id: 3, StoreId: 3}},
Expand All @@ -413,7 +413,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) transfer leader after leaving joint state",
"(enable JointConsensus) transfer leader after leaving joint state",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{Id: 3, StoreId: 3}, {Id: 1, StoreId: 1}},
Expand All @@ -432,7 +432,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) add 1 peer(learner) should always build steps without joint consensus state",
"(enable JointConsensus) add 1 peer(learner) should always build steps without joint consensus state",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
Expand All @@ -442,7 +442,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) remove 1 peer(learner) should always build steps without joint consensus state",
"(enable JointConsensus) remove 1 peer(learner) should always build steps without joint consensus state",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2, Role: metapb.PeerRole_Learner}, {Id: 3, StoreId: 3}},
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 3, StoreId: 3}},
Expand All @@ -452,7 +452,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) add 1+ learners should not enter joint consensus state",
"(enable JointConsensus) add 1+ learners should not enter joint consensus state",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}},
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2, Role: metapb.PeerRole_Learner}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
Expand All @@ -463,7 +463,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) remove 1+ learners should not enter joint consensus state",
"(enable JointConsensus) remove 1+ learners should not enter joint consensus state",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2, Role: metapb.PeerRole_Learner}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
[]*metapb.Peer{{Id: 1, StoreId: 1}},
Expand All @@ -474,7 +474,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) demote 1 voter should enter JointConcensus, and TiKV will handle the leave step",
"(enable JointConsensus) demote 1 voter should enter JointConsensus, and TiKV will handle the leave step",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}, {Id: 3, StoreId: 3}},
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
Expand All @@ -487,7 +487,7 @@ func (s *testBuilderSuite) TestBuild(c *C) {
},
},
{
"(enable JointConcensus) add 1 learner should goto to buildStepsWithoutJointConsensus",
"(enable JointConsensus) add 1 learner should goto to buildStepsWithoutJointConsensus",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}, {Id: 3, StoreId: 3, Role: metapb.PeerRole_Learner}},
Expand All @@ -496,6 +496,37 @@ func (s *testBuilderSuite) TestBuild(c *C) {
AddLearner{ToStore: 3},
},
},
{
// issue: https://github.com/tikv/pd/issues/4411
"(enable JointConsensus) remove 1 voter from 2 voter replicas raft group",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
[]*metapb.Peer{{Id: 1, StoreId: 1}},
OpRegion,
[]OpStep{
ChangePeerV2Enter{
PromoteLearners: []PromoteLearner{},
DemoteVoters: []DemoteVoter{{ToStore: 2}},
},
RemovePeer{FromStore: 2},
},
},
{
// issue: https://github.com/tikv/pd/issues/4411
"(enable JointConsensus) remove 1 voter from 2 voter replicas raft group, with transfer leader",
true,
[]*metapb.Peer{{Id: 1, StoreId: 1}, {Id: 2, StoreId: 2}},
[]*metapb.Peer{{Id: 2, StoreId: 2}},
OpLeader | OpRegion,
[]OpStep{
TransferLeader{FromStore: 1, ToStore: 2},
ChangePeerV2Enter{
PromoteLearners: []PromoteLearner{},
DemoteVoters: []DemoteVoter{{ToStore: 1}},
},
RemovePeer{FromStore: 1},
},
},
}

for _, tc := range cases {
Expand Down