cherry pick #3133 to release-4.0 (#3208)

Signed-off-by: ti-srebot <ti-srebot@pingcap.com> Co-authored-by: disksing <i@disksing.com>
tikv · Nov 25, 2020 · 6a666f5 · 6a666f5
1 parent 820d1e1
commit 6a666f5
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 6 deletions.
diff --git a/server/schedule/checker/rule_checker.go b/server/schedule/checker/rule_checker.go
@@ -137,11 +137,19 @@ func (c *RuleChecker) fixLooseMatchPeer(region *core.RegionInfo, fit *placement.
 		checkerCounter.WithLabelValues("rule_checker", "fix-peer-role").Inc()
 		return operator.CreatePromoteLearnerOperator("fix-peer-role", c.cluster, region, peer)
 	}
-	if region.GetLeader().GetId() == peer.GetId() && rf.Rule.Role == placement.Follower {
+	if region.GetLeader().GetId() != peer.GetId() && rf.Rule.Role == placement.Leader {
 		checkerCounter.WithLabelValues("rule_checker", "fix-leader-role").Inc()
+		if c.allowLeader(fit, peer) {
+			return operator.CreateTransferLeaderOperator("fix-leader-role", c.cluster, region, region.GetLeader().StoreId, peer.GetStoreId(), 0)
+		}
+		checkerCounter.WithLabelValues("rule_checker", "not-allow-leader")
+		return nil, errors.New("peer cannot be leader")
+	}
+	if region.GetLeader().GetId() == peer.GetId() && rf.Rule.Role == placement.Follower {
+		checkerCounter.WithLabelValues("rule_checker", "fix-follower-role").Inc()
 		for _, p := range region.GetPeers() {
 			if c.allowLeader(fit, p) {
-				return operator.CreateTransferLeaderOperator("fix-peer-role", c.cluster, region, peer.GetStoreId(), p.GetStoreId(), 0)
+				return operator.CreateTransferLeaderOperator("fix-follower-role", c.cluster, region, peer.GetStoreId(), p.GetStoreId(), 0)
 			}
 		}
 		checkerCounter.WithLabelValues("rule_checker", "no-new-leader").Inc()

diff --git a/server/schedule/checker/rule_checker_test.go b/server/schedule/checker/rule_checker_test.go
@@ -150,17 +150,17 @@ func (s *testRuleCheckerSuite) TestFixRole(c *C) {
 func (s *testRuleCheckerSuite) TestFixRoleLeader(c *C) {
 	s.cluster.AddLabelsStore(1, 1, map[string]string{"role": "follower"})
 	s.cluster.AddLabelsStore(2, 1, map[string]string{"role": "follower"})
-	s.cluster.AddLabelsStore(3, 1, map[string]string{"role": "leader"})
+	s.cluster.AddLabelsStore(3, 1, map[string]string{"role": "voter"})
 	s.cluster.AddLeaderRegionWithRange(1, "", "", 1, 2, 3)
 	s.ruleManager.SetRule(&placement.Rule{
 		GroupID:  "pd",
 		ID:       "r1",
 		Index:    100,
 		Override: true,
-		Role:     placement.Leader,
+		Role:     placement.Voter,
 		Count:    1,
 		LabelConstraints: []placement.LabelConstraint{
-			{Key: "role", Op: "in", Values: []string{"leader"}},
+			{Key: "role", Op: "in", Values: []string{"voter"}},
 		},
 	})
 	s.ruleManager.SetRule(&placement.Rule{
@@ -175,10 +175,42 @@ func (s *testRuleCheckerSuite) TestFixRoleLeader(c *C) {
 	})
 	op := s.rc.Check(s.cluster.GetRegion(1))
 	c.Assert(op, NotNil)
-	c.Assert(op.Desc(), Equals, "fix-peer-role")
+	c.Assert(op.Desc(), Equals, "fix-follower-role")
 	c.Assert(op.Step(0).(operator.TransferLeader).ToStore, Equals, uint64(3))
 }
 
+func (s *testRuleCheckerSuite) TestFixRoleLeaderIssue3130(c *C) {
+	s.cluster.AddLabelsStore(1, 1, map[string]string{"role": "follower"})
+	s.cluster.AddLabelsStore(2, 1, map[string]string{"role": "leader"})
+	s.cluster.AddLeaderRegion(1, 1, 2)
+	s.ruleManager.SetRule(&placement.Rule{
+		GroupID:  "pd",
+		ID:       "r1",
+		Index:    100,
+		Override: true,
+		Role:     placement.Leader,
+		Count:    1,
+		LabelConstraints: []placement.LabelConstraint{
+			{Key: "role", Op: "in", Values: []string{"leader"}},
+		},
+	})
+	op := s.rc.Check(s.cluster.GetRegion(1))
+	c.Assert(op, NotNil)
+	c.Assert(op.Desc(), Equals, "fix-leader-role")
+	c.Assert(op.Step(0).(operator.TransferLeader).ToStore, Equals, uint64(2))
+
+	s.cluster.SetStoreBusy(2, true)
+	op = s.rc.Check(s.cluster.GetRegion(1))
+	c.Assert(op, IsNil)
+	s.cluster.SetStoreBusy(2, false)
+
+	s.cluster.AddLeaderRegion(1, 2, 1)
+	op = s.rc.Check(s.cluster.GetRegion(1))
+	c.Assert(op, NotNil)
+	c.Assert(op.Desc(), Equals, "remove-orphan-peer")
+	c.Assert(op.Step(0).(operator.RemovePeer).FromStore, Equals, uint64(1))
+}
+
 func (s *testRuleCheckerSuite) TestBetterReplacement(c *C) {
 	s.cluster.AddLabelsStore(1, 1, map[string]string{"host": "host1"})
 	s.cluster.AddLabelsStore(2, 1, map[string]string{"host": "host1"})