diff --git a/pkg/schedule/filter/filters.go b/pkg/schedule/filter/filters.go index 2bda9bee9c0..9e239905c7b 100644 --- a/pkg/schedule/filter/filters.go +++ b/pkg/schedule/filter/filters.go @@ -639,7 +639,7 @@ func (f *ruleFitFilter) Source(_ config.Config, _ *core.StoreInfo) *plan.Status // 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.Config, store *core.StoreInfo) *plan.Status { +func (f *ruleFitFilter) Target(_ config.Config, store *core.StoreInfo) *plan.Status { if f.oldFit.Replace(f.srcStore, store) { return statusOK } diff --git a/pkg/schedule/placement/fit.go b/pkg/schedule/placement/fit.go index e3f6edb7e90..ed940160bbd 100644 --- a/pkg/schedule/placement/fit.go +++ b/pkg/schedule/placement/fit.go @@ -39,15 +39,27 @@ type RegionFit struct { 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 diff --git a/pkg/schedule/placement/fit_test.go b/pkg/schedule/placement/fit_test.go index 65743451a2b..d7b3c91163e 100644 --- a/pkg/schedule/placement/fit_test.go +++ b/pkg/schedule/placement/fit_test.go @@ -133,13 +133,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) @@ -148,6 +153,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) }