diff --git a/server/checker/merge_checker.go b/server/checker/merge_checker.go index b5f78fe8b4b..b86578d5048 100644 --- a/server/checker/merge_checker.go +++ b/server/checker/merge_checker.go @@ -118,6 +118,7 @@ func (m *MergeChecker) Check(region *core.RegionInfo) []*operator.Operator { log.Debug("try to merge region", zap.Stringer("from", core.RegionToHexMeta(region.GetMeta())), zap.Stringer("to", core.RegionToHexMeta(target.GetMeta()))) ops, err := operator.CreateMergeRegionOperator("merge-region", m.cluster, region, target, operator.OpMerge) if err != nil { + log.Warn("create merge region operator failed", zap.Error(err)) return nil } checkerCounter.WithLabelValues("merge_checker", "new_operator").Inc() diff --git a/server/checker/merge_checker_test.go b/server/checker/merge_checker_test.go index 54bfe9e9d75..3af1b1d2ce4 100644 --- a/server/checker/merge_checker_test.go +++ b/server/checker/merge_checker_test.go @@ -26,6 +26,7 @@ import ( "github.com/pingcap/pd/server/namespace" "github.com/pingcap/pd/server/schedule" "github.com/pingcap/pd/server/schedule/operator" + "github.com/pingcap/pd/server/schedule/opt" ) func TestChecker(t *testing.T) { @@ -44,7 +45,18 @@ func (s *testMergeCheckerSuite) SetUpTest(c *C) { cfg := mockoption.NewScheduleOptions() cfg.MaxMergeRegionSize = 2 cfg.MaxMergeRegionKeys = 2 + cfg.LabelProperties = map[string][]*metapb.StoreLabel{ + opt.RejectLeader: {{Key: "reject", Value: "leader"}}, + } s.cluster = mockcluster.NewCluster(cfg) + stores := map[uint64][]string{ + 1: {}, 2: {}, 3: {}, 4: {}, 5: {}, 6: {}, + 7: {"reject", "leader"}, + 8: {"reject", "leader"}, + } + for storeID, labels := range stores { + s.cluster.PutStoreWithLabels(storeID, labels...) + } s.regions = []*core.RegionInfo{ core.NewRegionInfo( &metapb.Region{ @@ -157,14 +169,19 @@ func (s *testMergeCheckerSuite) checkSteps(c *C, op *operator.Operator, steps [] func (s *testMergeCheckerSuite) TestMatchPeers(c *C) { // partial store overlap not including leader ops := s.mc.Check(s.regions[2]) + c.Assert(ops, NotNil) s.checkSteps(c, ops[0], []operator.OpStep{ - operator.TransferLeader{FromStore: 6, ToStore: 5}, - operator.AddLearner{ToStore: 1, PeerID: 1}, - operator.PromoteLearner{ToStore: 1, PeerID: 1}, + operator.AddLearner{ToStore: 4, PeerID: 1}, + operator.PromoteLearner{ToStore: 4, PeerID: 1}, + operator.RemovePeer{FromStore: 2}, - operator.AddLearner{ToStore: 4, PeerID: 2}, - operator.PromoteLearner{ToStore: 4, PeerID: 2}, + + operator.AddLearner{ToStore: 1, PeerID: 2}, + operator.PromoteLearner{ToStore: 1, PeerID: 2}, + + operator.TransferLeader{FromStore: 6, ToStore: 5}, operator.RemovePeer{FromStore: 6}, + operator.MergeRegion{ FromRegion: s.regions[2].GetMeta(), ToRegion: s.regions[1].GetMeta(), @@ -241,16 +258,109 @@ func (s *testMergeCheckerSuite) TestMatchPeers(c *C) { s.cluster.PutRegion(s.regions[2]) ops = s.mc.Check(s.regions[2]) s.checkSteps(c, ops[0], []operator.OpStep{ - operator.AddLearner{ToStore: 1, PeerID: 4}, - operator.PromoteLearner{ToStore: 1, PeerID: 4}, + operator.AddLearner{ToStore: 4, PeerID: 4}, + operator.PromoteLearner{ToStore: 4, PeerID: 4}, + operator.RemovePeer{FromStore: 3}, - operator.AddLearner{ToStore: 4, PeerID: 5}, - operator.PromoteLearner{ToStore: 4, PeerID: 5}, + + operator.AddLearner{ToStore: 1, PeerID: 5}, + operator.PromoteLearner{ToStore: 1, PeerID: 5}, + operator.RemovePeer{FromStore: 6}, + operator.AddLearner{ToStore: 5, PeerID: 6}, operator.PromoteLearner{ToStore: 5, PeerID: 6}, + + operator.TransferLeader{FromStore: 2, ToStore: 4}, + operator.RemovePeer{FromStore: 2}, + + operator.MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: false, + }, + }) + s.checkSteps(c, ops[1], []operator.OpStep{ + operator.MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: true, + }, + }) + + // no overlap with reject leader label + s.regions[1] = s.regions[1].Clone( + core.SetPeers([]*metapb.Peer{ + {Id: 112, StoreId: 7}, + {Id: 113, StoreId: 8}, + {Id: 114, StoreId: 1}, + }), + core.WithLeader(&metapb.Peer{Id: 114, StoreId: 1}), + ) + s.cluster.PutRegion(s.regions[1]) + ops = s.mc.Check(s.regions[2]) + s.checkSteps(c, ops[0], []operator.OpStep{ + operator.AddLearner{ToStore: 1, PeerID: 7}, + operator.PromoteLearner{ToStore: 1, PeerID: 7}, + + operator.RemovePeer{FromStore: 3}, + + operator.AddLearner{ToStore: 7, PeerID: 8}, + operator.PromoteLearner{ToStore: 7, PeerID: 8}, + + operator.RemovePeer{FromStore: 6}, + + operator.AddLearner{ToStore: 8, PeerID: 9}, + operator.PromoteLearner{ToStore: 8, PeerID: 9}, + operator.TransferLeader{FromStore: 2, ToStore: 1}, operator.RemovePeer{FromStore: 2}, + + operator.MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: false, + }, + }) + s.checkSteps(c, ops[1], []operator.OpStep{ + operator.MergeRegion{ + FromRegion: s.regions[2].GetMeta(), + ToRegion: s.regions[1].GetMeta(), + IsPassive: true, + }, + }) + + // overlap with reject leader label + s.regions[1] = s.regions[1].Clone( + core.SetPeers([]*metapb.Peer{ + {Id: 115, StoreId: 7}, + {Id: 116, StoreId: 8}, + {Id: 117, StoreId: 1}, + }), + core.WithLeader(&metapb.Peer{Id: 117, StoreId: 1}), + ) + s.regions[2] = s.regions[2].Clone( + core.SetPeers([]*metapb.Peer{ + {Id: 118, StoreId: 7}, + {Id: 119, StoreId: 3}, + {Id: 120, StoreId: 2}, + }), + core.WithLeader(&metapb.Peer{Id: 120, StoreId: 2}), + ) + s.cluster.PutRegion(s.regions[1]) + ops = s.mc.Check(s.regions[2]) + s.checkSteps(c, ops[0], []operator.OpStep{ + operator.AddLearner{ToStore: 1, PeerID: 10}, + operator.PromoteLearner{ToStore: 1, PeerID: 10}, + + operator.RemovePeer{FromStore: 3}, + + operator.AddLearner{ToStore: 8, PeerID: 11}, + operator.PromoteLearner{ToStore: 8, PeerID: 11}, + + operator.TransferLeader{FromStore: 2, ToStore: 1}, + operator.RemovePeer{FromStore: 2}, + operator.MergeRegion{ FromRegion: s.regions[2].GetMeta(), ToRegion: s.regions[1].GetMeta(), diff --git a/server/schedule/operator/operator.go b/server/schedule/operator/operator.go index f232b1d04c3..1777a45411b 100644 --- a/server/schedule/operator/operator.go +++ b/server/schedule/operator/operator.go @@ -626,7 +626,7 @@ func (o *Operator) History() []OpHistory { // CreateAddPeerOperator creates an operator that adds a new peer. func CreateAddPeerOperator(desc string, cluster Cluster, region *core.RegionInfo, peerID uint64, toStoreID uint64, kind OpKind) *Operator { - steps := CreateAddPeerSteps(toStoreID, peerID, cluster) + steps := CreateAddPeerSteps(cluster, toStoreID, peerID) brief := fmt.Sprintf("add peer: store %v", toStoreID) return NewOperator(desc, brief, region.GetID(), region.GetRegionEpoch(), kind|OpRegion, steps...) } @@ -659,7 +659,7 @@ func CreateRemovePeerOperator(desc string, cluster Cluster, kind OpKind, region } // CreateAddPeerSteps creates an OpStep list that add a new peer. -func CreateAddPeerSteps(newStore uint64, peerID uint64, cluster Cluster) []OpStep { +func CreateAddPeerSteps(cluster Cluster, newStore uint64, peerID uint64) []OpStep { var st []OpStep if cluster.IsRaftLearnerEnabled() { st = []OpStep{ @@ -675,7 +675,7 @@ func CreateAddPeerSteps(newStore uint64, peerID uint64, cluster Cluster) []OpSte } // CreateAddLightPeerSteps creates an OpStep list that add a new peer without considering the influence. -func CreateAddLightPeerSteps(newStore uint64, peerID uint64, cluster Cluster) []OpStep { +func CreateAddLightPeerSteps(cluster Cluster, newStore uint64, peerID uint64) []OpStep { var st []OpStep if cluster.IsRaftLearnerEnabled() { st = []OpStep{ @@ -699,41 +699,128 @@ func CreateTransferLeaderOperator(desc string, region *core.RegionInfo, sourceSt // CreateMoveRegionOperator creates an operator that moves a region to specified stores. func CreateMoveRegionOperator(desc string, cluster Cluster, region *core.RegionInfo, kind OpKind, storeIDs map[uint64]struct{}) (*Operator, error) { - var steps []OpStep + mvkind, steps, err := moveRegionSteps(cluster, region, storeIDs) + if err != nil { + return nil, err + } + kind |= mvkind + brief := fmt.Sprintf("mv region: stores %v to %v", u64Set(region.GetStoreIds()), u64Set(storeIDs)) + return NewOperator(desc, brief, region.GetID(), region.GetRegionEpoch(), kind, steps...), nil +} + +// moveRegionSteps returns steps to move a region to specific stores. +// +// The first store in the slice will not have RejectLeader label. +// If all of the stores have RejectLeader label, it returns an error. +func moveRegionSteps(cluster Cluster, region *core.RegionInfo, stores map[uint64]struct{}) (OpKind, []OpStep, error) { + storeIDs := make([]uint64, 0, len(stores)) + for id := range stores { + storeIDs = append(storeIDs, id) + } + + i, _ := findNoLabelProperty(cluster, opt.RejectLeader, storeIDs) + if i < 0 { + return 0, nil, errors.New("all of the stores have RejectLeader label") + } + + storeIDs[0], storeIDs[i] = storeIDs[i], storeIDs[0] + return orderedMoveRegionSteps(cluster, region, storeIDs) +} + +// orderedMoveRegionSteps returns steps to move peers of a region to specific stores in order. +// +// If the current leader is not in storeIDs, it will be transferred to a follower which +// do not need to move if there is one, otherwise the first suitable new added follower. +// New peers will be added in the same order in storeIDs. +// NOTE: orderedMoveRegionSteps does NOT check duplicate stores. +func orderedMoveRegionSteps(cluster Cluster, region *core.RegionInfo, storeIDs []uint64) (OpKind, []OpStep, error) { + var kind OpKind + + oldStores := make([]uint64, 0, len(storeIDs)) + newStores := make([]uint64, 0, len(storeIDs)) + + sourceStores := region.GetStoreIds() + targetStores := make(map[uint64]struct{}, len(storeIDs)) + // Add missing peers. - for id := range storeIDs { - if region.GetStorePeer(id) != nil { + var addPeerSteps [][]OpStep + for _, id := range storeIDs { + targetStores[id] = struct{}{} + if _, ok := sourceStores[id]; ok { + oldStores = append(oldStores, id) continue } + newStores = append(newStores, id) peer, err := cluster.AllocPeer(id) if err != nil { - return nil, err - } - if cluster.IsRaftLearnerEnabled() { - steps = append(steps, - AddLearner{ToStore: id, PeerID: peer.Id}, - PromoteLearner{ToStore: id, PeerID: peer.Id}, - ) - } else { - steps = append(steps, AddPeer{ToStore: id, PeerID: peer.Id}) + log.Debug("peer alloc failed", zap.Error(err)) + return kind, nil, err } + addPeerSteps = append(addPeerSteps, CreateAddPeerSteps(cluster, id, peer.Id)) + kind |= OpRegion } + // Transferring leader to a new added follower may be refused by TiKV. + // Ref: https://github.com/tikv/tikv/issues/3819 + // So, the new leader should be a follower that do not need to move if there is one, + // otherwise the first suitable new added follower. + orderedStoreIDs := append(oldStores, newStores...) + // Remove redundant peers. + var rmPeerSteps [][]OpStep + // Transfer leader as late as possible to prevent transferring to a new added follower. + var mvLeaderSteps []OpStep for _, peer := range region.GetPeers() { - if _, ok := storeIDs[peer.GetStoreId()]; ok { + id := peer.GetStoreId() + if _, ok := targetStores[id]; ok { continue } - if region.GetLeader().GetId() == peer.GetId() { - for targetID := range storeIDs { - steps = append(steps, TransferLeader{FromStore: peer.GetStoreId(), ToStore: targetID}) - break + if region.GetLeader().GetStoreId() == id { + tlkind, tlsteps, err := transferLeaderToSuitableSteps(cluster, id, orderedStoreIDs) + if err != nil { + log.Debug("move region to stores failed", zap.Uint64("region-id", region.GetID()), zap.Uint64s("store-ids", orderedStoreIDs), zap.Error(err)) + return kind, nil, err } + mvLeaderSteps = append(tlsteps, RemovePeer{FromStore: id}) + kind |= tlkind + } else { + rmPeerSteps = append(rmPeerSteps, []OpStep{RemovePeer{FromStore: id}}) } - steps = append(steps, RemovePeer{FromStore: peer.GetStoreId()}) + kind |= OpRegion } - brief := fmt.Sprintf("mv region: stores %v to %v", u64Set(region.GetStoreIds()), u64Set(storeIDs)) - return NewOperator(desc, brief, region.GetID(), region.GetRegionEpoch(), kind|OpRegion, steps...), nil + + // Interleaving makes the operator add and remove peers one by one, so that there won't have + // too many additional peers if the operator fails in the half. + hint := len(addPeerSteps)*2 + len(rmPeerSteps) + len(mvLeaderSteps) + steps := interleaveStepGroups(addPeerSteps, rmPeerSteps, hint) + + steps = append(steps, mvLeaderSteps...) + + return kind, steps, nil +} + +// interleaveStepGroups interleaves two slice of step groups. For example: +// +// a = [[opA1, opA2], [opA3], [opA4, opA5, opA6]] +// b = [[opB1], [opB2], [opB3, opB4], [opB5, opB6]] +// c = interleaveStepGroups(a, b, 0) +// c == [opA1, opA2, opB1, opA3, opB2, opA4, opA5, opA6, opB3, opB4, opB5, opB6] +// +// sizeHint is a hint for the capacity of returned slice. +func interleaveStepGroups(a, b [][]OpStep, sizeHint int) []OpStep { + steps := make([]OpStep, 0, sizeHint) + i, j := 0, 0 + for ; i < len(a) && j < len(b); i, j = i+1, j+1 { + steps = append(steps, a[i]...) + steps = append(steps, b[j]...) + } + for ; i < len(a); i++ { + steps = append(steps, a[i]...) + } + for ; j < len(b); j++ { + steps = append(steps, b[j]...) + } + return steps } // CreateMovePeerOperator creates an operator that replaces an old peer with a new peer. @@ -742,7 +829,7 @@ func CreateMovePeerOperator(desc string, cluster Cluster, region *core.RegionInf if err != nil { return nil, err } - st := CreateAddPeerSteps(newStore, peerID, cluster) + st := CreateAddPeerSteps(cluster, newStore, peerID) steps = append(st, steps...) brief := fmt.Sprintf("mv peer: store %v to %v", oldStore, newStore) return NewOperator(desc, brief, region.GetID(), region.GetRegionEpoch(), removeKind|kind|OpRegion, steps...), nil @@ -754,7 +841,7 @@ func CreateMoveLeaderOperator(desc string, cluster Cluster, region *core.RegionI if err != nil { return nil, err } - st := CreateAddPeerSteps(newStore, peerID, cluster) + st := CreateAddPeerSteps(cluster, newStore, peerID) st = append(st, TransferLeader{ToStore: newStore, FromStore: oldStore}) steps = append(st, steps...) brief := fmt.Sprintf("mv leader: store %v to %v", oldStore, newStore) @@ -783,21 +870,8 @@ func getRegionFollowerIDs(region *core.RegionInfo) []uint64 { // removePeerSteps returns the steps to safely remove a peer. It prevents removing leader by transfer its leadership first. func removePeerSteps(cluster Cluster, region *core.RegionInfo, storeID uint64, followerIDs []uint64) (kind OpKind, steps []OpStep, err error) { if region.GetLeader() != nil && region.GetLeader().GetStoreId() == storeID { - var follower *core.StoreInfo - for _, id := range followerIDs { - follower = cluster.GetStore(id) - if follower == nil { - log.Error("failed to get the store", zap.Uint64("store-id", id)) - continue - } - if !cluster.CheckLabelProperty(opt.RejectLeader, follower.GetLabels()) { - steps = append(steps, TransferLeader{FromStore: storeID, ToStore: id}) - kind = OpLeader - break - } - } - if len(steps) == 0 { - err = errors.New("no suitable follower to become region leader") + kind, steps, err = transferLeaderToSuitableSteps(cluster, storeID, followerIDs) + if err != nil { log.Debug("failed to create remove peer operator", zap.Uint64("region-id", region.GetID()), zap.Error(err)) return } @@ -807,9 +881,34 @@ func removePeerSteps(cluster Cluster, region *core.RegionInfo, storeID uint64, f return } +// findNoLabelProperty finds the first store without given label property. +func findNoLabelProperty(cluster Cluster, prop string, storeIDs []uint64) (int, uint64) { + for i, id := range storeIDs { + store := cluster.GetStore(id) + if store != nil { + if !cluster.CheckLabelProperty(prop, store.GetLabels()) { + return i, id + } + } else { + log.Debug("nil store", zap.Uint64("store-id", id)) + } + } + return -1, 0 +} + +// transferLeaderToSuitableSteps returns the first suitable store to become region leader. +// Returns an error if there is no suitable store. +func transferLeaderToSuitableSteps(cluster Cluster, leaderID uint64, storeIDs []uint64) (OpKind, []OpStep, error) { + _, id := findNoLabelProperty(cluster, opt.RejectLeader, storeIDs) + if id != 0 { + return OpLeader, []OpStep{TransferLeader{FromStore: leaderID, ToStore: id}}, nil + } + return 0, nil, errors.New("no suitable store to become region leader") +} + // CreateMergeRegionOperator creates an operator that merge two region into one. func CreateMergeRegionOperator(desc string, cluster Cluster, source *core.RegionInfo, target *core.RegionInfo, kind OpKind) ([]*Operator, error) { - steps, kinds, err := matchPeerSteps(cluster, source, target) + kinds, steps, err := matchPeerSteps(cluster, source, target) if err != nil { return nil, err } @@ -832,8 +931,7 @@ func CreateMergeRegionOperator(desc string, cluster Cluster, source *core.Region } // matchPeerSteps returns the steps to match the location of peer stores of source region with target's. -func matchPeerSteps(cluster Cluster, source *core.RegionInfo, target *core.RegionInfo) ([]OpStep, OpKind, error) { - var steps []OpStep +func matchPeerSteps(cluster Cluster, source *core.RegionInfo, target *core.RegionInfo) (OpKind, []OpStep, error) { var kind OpKind sourcePeers := source.GetPeers() @@ -841,115 +939,26 @@ func matchPeerSteps(cluster Cluster, source *core.RegionInfo, target *core.Regio // make sure the peer count is same if len(sourcePeers) != len(targetPeers) { - return nil, kind, errors.New("mismatch count of peer") - } - - // There is a case that a follower is added and transfer leader to it, - // and the apply process of it is slow so leader regards it as voter - // but actually it is still learner. Once that, the follower can't be leader, - // but old leader can't know that so there is no leader to serve for a while. - // So target leader should be the first added follower if there is no transection stores. - var targetLeader uint64 - var toAdds [][]OpStep - - // get overlapped part of the peers of two regions - intersection := getIntersectionStores(sourcePeers, targetPeers) - for _, peer := range targetPeers { - storeID := peer.GetStoreId() - // find missing peers. - if _, found := intersection[storeID]; !found { - var addSteps []OpStep - - peer, err := cluster.AllocPeer(storeID) - if err != nil { - log.Debug("peer alloc failed", zap.Error(err)) - return nil, kind, err - } - if cluster.IsRaftLearnerEnabled() { - addSteps = append(addSteps, - AddLearner{ToStore: storeID, PeerID: peer.Id}, - PromoteLearner{ToStore: storeID, PeerID: peer.Id}, - ) - } else { - addSteps = append(addSteps, AddPeer{ToStore: storeID, PeerID: peer.Id}) - } - toAdds = append(toAdds, addSteps) - - // record the first added peer - if targetLeader == 0 { - targetLeader = storeID - } - kind |= OpRegion - } - } - - leaderID := source.GetLeader().GetStoreId() - for storeID := range intersection { - // if leader belongs to overlapped part, no need to transfer - if storeID == leaderID { - targetLeader = 0 - break - } - targetLeader = storeID + return kind, nil, errors.New("mismatch count of peer") } - // if intersection is not empty and leader doesn't belong to intersection, transfer leader to store in overlapped part - if len(intersection) != 0 && targetLeader != 0 { - steps = append(steps, TransferLeader{FromStore: source.GetLeader().GetStoreId(), ToStore: targetLeader}) - kind |= OpLeader - targetLeader = 0 + targetLeader := target.GetLeader().GetStoreId() + if targetLeader == 0 { + return kind, nil, errors.New("target does not have a leader") } - index := 0 - // remove redundant peers. - for _, peer := range sourcePeers { - if _, found := intersection[peer.GetStoreId()]; found { - continue - } - - // the leader should be the last to remove - if targetLeader != 0 && peer.GetStoreId() == leaderID { - continue - } - - steps = append(steps, toAdds[index]...) - steps = append(steps, RemovePeer{FromStore: peer.GetStoreId()}) - kind |= OpRegion - index++ - } + // The target leader store must not have RejectLeader. + targetStores := make([]uint64, 1, len(targetPeers)) + targetStores[0] = targetLeader - // transfer leader before remove leader - if targetLeader != 0 { - steps = append(steps, toAdds[index]...) - steps = append(steps, TransferLeader{FromStore: leaderID, ToStore: targetLeader}) - steps = append(steps, RemovePeer{FromStore: leaderID}) - kind |= OpLeader | OpRegion - index++ - } - - if index != len(toAdds) { - return nil, kind, errors.New("wrong count of add steps") - } - - return steps, kind, nil -} - -// getIntersectionStores returns the stores included in two region's peers. -func getIntersectionStores(a []*metapb.Peer, b []*metapb.Peer) map[uint64]struct{} { - intersection := make(map[uint64]struct{}) - hash := make(map[uint64]struct{}) - - for _, peer := range a { - hash[peer.GetStoreId()] = struct{}{} - } - - for _, peer := range b { - if _, found := hash[peer.GetStoreId()]; found { - intersection[peer.GetStoreId()] = struct{}{} + for _, peer := range targetPeers { + id := peer.GetStoreId() + if id != targetLeader { + targetStores = append(targetStores, id) } } - return intersection + return orderedMoveRegionSteps(cluster, source, targetStores) } // CreateScatterRegionOperator creates an operator that scatters the specified region. @@ -981,7 +990,7 @@ func CreateScatterRegionOperator(desc string, cluster Cluster, origin *core.Regi // Creates the first step if _, ok := originStoreIDs[targetLeaderPeer.GetStoreId()]; !ok { - st := CreateAddLightPeerSteps(targetLeaderPeer.GetStoreId(), targetLeaderPeer.GetId(), cluster) + st := CreateAddLightPeerSteps(cluster, targetLeaderPeer.GetStoreId(), targetLeaderPeer.GetId()) steps = append(steps, st...) // Do not transfer leader to the newly added peer // Ref: https://github.com/tikv/tikv/issues/3819 @@ -1005,13 +1014,13 @@ func CreateScatterRegionOperator(desc string, cluster Cluster, origin *core.Regi continue } if replacedPeers[j].GetStoreId() == originLeaderStoreID { - st := CreateAddLightPeerSteps(peer.GetStoreId(), peer.GetId(), cluster) + st := CreateAddLightPeerSteps(cluster, peer.GetStoreId(), peer.GetId()) st = append(st, RemovePeer{FromStore: replacedPeers[j].GetStoreId()}) deferSteps = append(deferSteps, st...) kind |= OpRegion | OpLeader continue } - st := CreateAddLightPeerSteps(peer.GetStoreId(), peer.GetId(), cluster) + st := CreateAddLightPeerSteps(cluster, peer.GetStoreId(), peer.GetId()) steps = append(steps, st...) steps = append(steps, RemovePeer{FromStore: replacedPeers[j].GetStoreId()}) kind |= OpRegion diff --git a/server/schedule/operator/operator_test.go b/server/schedule/operator/operator_test.go index b90be4e1fea..72d4ecefae1 100644 --- a/server/schedule/operator/operator_test.go +++ b/server/schedule/operator/operator_test.go @@ -21,7 +21,10 @@ import ( . "github.com/pingcap/check" "github.com/pingcap/kvproto/pkg/metapb" + "github.com/pingcap/pd/pkg/mock/mockcluster" + "github.com/pingcap/pd/pkg/mock/mockoption" "github.com/pingcap/pd/server/core" + "github.com/pingcap/pd/server/schedule/opt" ) func Test(t *testing.T) { @@ -30,7 +33,27 @@ func Test(t *testing.T) { var _ = Suite(&testOperatorSuite{}) -type testOperatorSuite struct{} +type testOperatorSuite struct { + cluster *mockcluster.Cluster +} + +func (s *testOperatorSuite) SetUpTest(c *C) { + cfg := mockoption.NewScheduleOptions() + cfg.MaxMergeRegionSize = 2 + cfg.MaxMergeRegionKeys = 2 + cfg.LabelProperties = map[string][]*metapb.StoreLabel{ + opt.RejectLeader: {{Key: "reject", Value: "leader"}}, + } + s.cluster = mockcluster.NewCluster(cfg) + stores := map[uint64][]string{ + 1: {}, 2: {}, 3: {}, 4: {}, 5: {}, 6: {}, + 7: {"reject", "leader"}, + 8: {"reject", "leader"}, + } + for storeID, labels := range stores { + s.cluster.PutStoreWithLabels(storeID, labels...) + } +} func (s *testOperatorSuite) newTestRegion(regionID uint64, leaderPeer uint64, peers ...[2]uint64) *core.RegionInfo { var ( @@ -52,6 +75,39 @@ func (s *testOperatorSuite) newTestRegion(regionID uint64, leaderPeer uint64, pe return regionInfo } +func genAddPeers(store uint64, groups [][]uint64) [][]OpStep { + ret := make([][]OpStep, len(groups)) + for i, grp := range groups { + steps := make([]OpStep, len(grp)) + for j, id := range grp { + steps[j] = AddPeer{ToStore: store, PeerID: id} + } + ret[i] = steps + } + return ret +} + +func (s *testOperatorSuite) TestInterleaveStepGroups(c *C) { + a := genAddPeers(1, [][]uint64{{1, 2}, {3}, {4, 5, 6}}) + b := genAddPeers(1, [][]uint64{{11}, {12}, {13, 14}, {15, 16}}) + ans := genAddPeers(1, [][]uint64{{1, 2, 11, 3, 12, 4, 5, 6, 13, 14, 15, 16}}) + res := interleaveStepGroups(a, b, 12) + c.Assert(res, DeepEquals, ans[0]) +} + +func (s *testOperatorSuite) TestFindNoLabelProperty(c *C) { + stores := []uint64{8, 7, 3, 4, 7, 3, 1, 5, 6} + i, id := findNoLabelProperty(s.cluster, opt.RejectLeader, stores) + c.Assert(i, Equals, 2) + c.Assert(id, Equals, uint64(3)) + i, id = findNoLabelProperty(s.cluster, opt.RejectLeader, stores[2:]) + c.Assert(i, Equals, 0) + c.Assert(id, Equals, uint64(3)) + i, id = findNoLabelProperty(s.cluster, opt.RejectLeader, stores[:2]) + c.Assert(i, Less, 0) + c.Assert(id, Equals, uint64(0)) +} + func (s *testOperatorSuite) TestOperatorStep(c *C) { region := s.newTestRegion(1, 1, [2]uint64{1, 1}, [2]uint64{2, 2}) c.Assert(TransferLeader{FromStore: 1, ToStore: 2}.IsFinish(region), IsFalse)