Merge branch 'release-6.5' into cherry-pick-6108-to-release-6.5

server/schedule/filter/filters.go

@@ -615,7 +615,7 @@ func (f *ruleFitFilter) Source(_ *config.PersistOptions, _ *core.StoreInfo) *pla
 // the replaced store can match the source rule.
 // RegionA:[1,2,3], move peer1 --> peer2 will not allow, because it's count not match the rule.
 // but transfer role peer1 --> peer2, it will support.
-func (f *ruleFitFilter) Target(options *config.PersistOptions, store *core.StoreInfo) *plan.Status {
+func (f *ruleFitFilter) Target(_ *config.PersistOptions, store *core.StoreInfo) *plan.Status {
 	if f.oldFit.Replace(f.srcStore, store) {
 		return statusOK
 	}

server/schedule/placement/fit.go

@@ -58,15 +58,27 @@ func (f *RegionFit) IsCached() bool {
 func (f *RegionFit) Replace(srcStoreID uint64, dstStore *core.StoreInfo) bool {
 	fit := f.getRuleFitByStoreID(srcStoreID)
 	// check the target store is fit all constraints.
-	if fit == nil || !MatchLabelConstraints(dstStore, fit.Rule.LabelConstraints) {
+	if fit == nil {
 		return false
 	}
-
 	// the members of the rule are same, it shouldn't check the score.
+	// it is used to transfer role like transfer leader.
 	if fit.contain(dstStore.GetID()) {
 		return true
 	}
 
+	// it should be allowed to exchange role if both the source and target's rule role is same.
+	// e.g. transfer leader from store 1 to store 2, and store-1 and store-2 are both voter.
+	targetFit := f.getRuleFitByStoreID(dstStore.GetID())
+	if targetFit != nil && targetFit.Rule.Role == fit.Rule.Role {
+		return true
+	}
+
+	// the target store should be fit all constraints.
+	if !MatchLabelConstraints(dstStore, fit.Rule.LabelConstraints) {
+		return false
+	}
+
 	score := isolationStoreScore(srcStoreID, dstStore, fit.stores, fit.Rule.LocationLabels)
 	// restore the source store.
 	return fit.IsolationScore <= score

server/schedule/placement/fit_test.go

@@ -131,13 +131,18 @@ func TestReplace(t *testing.T) {
 		{"1111,2111,3111,1115_learner", []string{"3/voter//zone", "1/learner/engine=tiflash/host"}, 1115, 2115, true},
 		// replace failed when the target store is not tiflash
 		{"1111,2111,3111,1115_learner", []string{"3/voter//zone", "1/learner/engine=tiflash/host"}, 1115, 1112, false},
-		{"1111_lead,2111,3111", []string{"1/leader/zone=zone1/zone", "2/voter//zone"}, 1111, 1112, true},
+		{"1111_leader,2111,3111", []string{"1/leader/zone=zone1/zone", "2/voter//zone"}, 1111, 1112, true},
 		// replace failed when the leader is not match the leader constraint.
 		{"1111_leader,2111,3111", []string{"1/leader/zone=zone1/zone", "2/voter//zone"}, 1111, 2112, false},
 		// transfer leader
-		{"1111_leader,1121,1131", []string{"1/leader/host=host1+host2/host", "3/voter//host"}, 1111, 1121, true},
+		{"1111_leader,1121,1131", []string{"1/leader/host=host1+host2/host", "2/voter//host"}, 1111, 1121, true},
 		// replace failed when the leader is not match the leader constraint.
 		{"1111_leader,1121,1131", []string{"1/leader/host=host1+host2/host", "2/voter//host"}, 1111, 1131, false},
+
+		// transfer leader success with different rule when the role is the same.
+		{"1111_leader,1121,1131", []string{"1/voter/host=host1/host", "1/voter/host=host2/host", "1/voter/host=host3/host"}, 1111, 1121, true},
+		//// transfer leader failed with different rule, but the role isn't same.
+		{"1111_leader,1121,1131", []string{"1/leader/host=host1/host", "1/voter/host=host2/host", "1/voter/host=host3/host"}, 1111, 1121, false},
 	}
 	for _, tc := range testCases {
 		region := makeRegion(tc.region)
@@ -146,6 +151,7 @@ func TestReplace(t *testing.T) {
 			rules = append(rules, makeRule(r))
 		}
 		rf := fitRegion(stores.GetStores(), region, rules, false)
+		re.True(rf.IsSatisfied())
 		rf.regionStores = stores.GetStores()
 		re.Equal(rf.Replace(tc.srcStoreID, stores.GetStore(tc.dstStoreID)), tc.ok)
 	}

server/schedule/region_scatterer.go

@@ -394,12 +394,14 @@ func allowLeader(fit *placement.RegionFit, peer *metapb.Peer) bool {
 	if peer.IsWitness {
 		return false
 	}
-
-	rule := fit.GetRuleFit(peer.GetId()).Rule
-	if rule.IsWitness {
+	peerFit := fit.GetRuleFit(peer.GetId())
+	if peerFit == nil || peerFit.Rule == nil {
+		return false
+	}
+	if peerFit.Rule.IsWitness {
 		return false
 	}
-	switch rule.Role {
+	switch peerFit.Rule.Role {
 	case placement.Voter, placement.Leader:
 		return true
 	}

server/schedule/region_scatterer_test.go

@@ -758,6 +758,11 @@ func TestBalanceRegion(t *testing.T) {
 		re.Equal(uint64(150), scatterer.ordinaryEngine.selectedPeer.Get(i, group))
 		re.Equal(uint64(50), scatterer.ordinaryEngine.selectedLeader.Get(i, group))
 	}
+	// Test for unhealthy region
+	// ref https://github.com/tikv/pd/issues/6099
+	region := tc.AddLeaderRegion(1500, 2, 3, 4, 6)
+	op := scatterer.scatterRegion(region, group)
+	re.False(isPeerCountChanged(op))
 }
 
 func isPeerCountChanged(op *operator.Operator) bool {