From 5f2112fd2893176fe1031b6391bf7902b22e2f0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=B7=B7=E6=B2=8CDM?= Date: Thu, 13 Jan 2022 18:27:43 +0800 Subject: [PATCH] schedule: prevent removing voters directly in 2 replica raft group (#4564) * schedule: prevent removing voters directly in 2 replica raft group close #4411 Signed-off-by: HunDunDM * fix test Signed-off-by: HunDunDM * add test Signed-off-by: HunDunDM * add name Signed-off-by: HunDunDM Co-authored-by: Ti Chi Robot --- server/schedule/operator/builder.go | 6 +- server/schedule/operator/builder_test.go | 79 +++++++++++++++++------- 2 files changed, 59 insertions(+), 26 deletions(-) diff --git a/server/schedule/operator/builder.go b/server/schedule/operator/builder.go index 31b280fc884b..6d56afefde66 100644 --- a/server/schedule/operator/builder.go +++ b/server/schedule/operator/builder.go @@ -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 } diff --git a/server/schedule/operator/builder_test.go b/server/schedule/operator/builder_test.go index b398eb9d3924..22fac017ec00 100644 --- a/server/schedule/operator/builder_test.go +++ b/server/schedule/operator/builder_test.go @@ -176,7 +176,7 @@ 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}}, @@ -184,7 +184,7 @@ func (s *testBuilderSuite) TestBuild(c *C) { []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}}, @@ -192,7 +192,7 @@ func (s *testBuilderSuite) TestBuild(c *C) { []OpStep{}, }, { - "(disable JointConcensus) no valid leader", + "(disable JointConsensus) no valid leader", false, []*metapb.Peer{{Id: 1, StoreId: 1}}, []*metapb.Peer{{Id: 10, StoreId: 10}}, @@ -200,7 +200,7 @@ func (s *testBuilderSuite) TestBuild(c *C) { []OpStep{}, }, { - "(enable JointConcensus) no valid leader", + "(enable JointConsensus) no valid leader", true, []*metapb.Peer{{Id: 1, StoreId: 1}}, []*metapb.Peer{{Id: 10, StoreId: 10}}, @@ -208,7 +208,7 @@ func (s *testBuilderSuite) TestBuild(c *C) { []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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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 @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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}}, @@ -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 {