Skip to content

Commit

Permalink
schedule: prevent removing voters directly in 2 replica raft group (t…
Browse files Browse the repository at this point in the history 
…ikv#4564)

* schedule: prevent removing voters directly in 2 replica raft group

close tikv#4411

Signed-off-by: HunDunDM <hundundm@gmail.com>

* fix test

Signed-off-by: HunDunDM <hundundm@gmail.com>

* add test

Signed-off-by: HunDunDM <hundundm@gmail.com>

* add name

Signed-off-by: HunDunDM <hundundm@gmail.com>

Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
  • Loading branch information
@HunDunDM @ti-chi-bot @disksing
2 people authored and disksing committed Feb 8, 2022
1 parent c50b950 commit 5f2112f
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 26 deletions.
6 changes: 4 additions & 2 deletions server/schedule/operator/builder.go
View file
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
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

0 comments on commit 5f2112f

Please sign in to comment.