From 23fc03c9376b7658a7ddbdd0ed2cfe2623d1b38f Mon Sep 17 00:00:00 2001 From: nolouch Date: Sun, 8 Apr 2018 11:22:57 +0800 Subject: [PATCH 1/4] server: fix frequent alloc id --- server/schedule/replica_checker.go | 7 ++++--- server/schedulers/balance_region.go | 12 ++++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/server/schedule/replica_checker.go b/server/schedule/replica_checker.go index 0e9d1574c84..dc891860f0d 100644 --- a/server/schedule/replica_checker.go +++ b/server/schedule/replica_checker.go @@ -83,7 +83,7 @@ func (r *ReplicaChecker) Check(region *core.RegionInfo) *Operator { // SelectBestReplacedPeerToAddReplica returns a new peer that to be used to replace the old peer and distinct score. func (r *ReplicaChecker) SelectBestReplacedPeerToAddReplica(region *core.RegionInfo, oldPeer *metapb.Peer, filters ...Filter) *metapb.Peer { - storeID, _ := r.selectBestReplacementStore(region, oldPeer, filters...) + storeID, _ := r.SelectBestReplacementStore(region, oldPeer, filters...) if storeID == 0 { log.Debugf("[region %d] no best store to add replica", region.GetId()) return nil @@ -95,7 +95,8 @@ func (r *ReplicaChecker) SelectBestReplacedPeerToAddReplica(region *core.RegionI return newPeer } -func (r *ReplicaChecker) selectBestReplacementStore(region *core.RegionInfo, oldPeer *metapb.Peer, filters ...Filter) (uint64, float64) { +// SelectBestReplacementStore returns a store id that to be used to replace the old peer and distinct score. +func (r *ReplicaChecker) SelectBestReplacementStore(region *core.RegionInfo, oldPeer *metapb.Peer, filters ...Filter) (uint64, float64) { filters = append(filters, NewExcludedFilter(nil, region.GetStoreIds())) newRegion := region.Clone() newRegion.RemoveStorePeer(oldPeer.GetStoreId()) @@ -214,7 +215,7 @@ func (r *ReplicaChecker) checkBestReplacement(region *core.RegionInfo) *Operator checkerCounter.WithLabelValues("replica_checker", "all_right").Inc() return nil } - storeID, newScore := r.selectBestReplacementStore(region, oldPeer) + storeID, newScore := r.SelectBestReplacementStore(region, oldPeer) if storeID == 0 { checkerCounter.WithLabelValues("replica_checker", "no_replacement_store").Inc() return nil diff --git a/server/schedulers/balance_region.go b/server/schedulers/balance_region.go index 72438a0b641..c01f563d35b 100644 --- a/server/schedulers/balance_region.go +++ b/server/schedulers/balance_region.go @@ -137,13 +137,13 @@ func (s *balanceRegionScheduler) transferPeer(cluster schedule.Cluster, region * scoreGuard := schedule.NewDistinctScoreFilter(cluster.GetLocationLabels(), stores, source) checker := schedule.NewReplicaChecker(cluster, nil) - newPeer := checker.SelectBestReplacedPeerToAddReplica(region, oldPeer, scoreGuard) - if newPeer == nil { - schedulerCounter.WithLabelValues(s.GetName(), "no_peer").Inc() + storeID, _ := checker.SelectBestReplacementStore(region, oldPeer, scoreGuard) + if storeID == 0 { + schedulerCounter.WithLabelValues(s.GetName(), "no_store").Inc() return nil } - target := cluster.GetStore(newPeer.GetStoreId()) + target := cluster.GetStore(storeID) log.Debugf("[region %d] source store id is %v, target store id is %v", region.GetId(), source.GetId(), target.GetId()) sourceSize := source.RegionSize + int64(opInfluence.GetStoreInfluence(source.GetId()).RegionSize) @@ -154,6 +154,10 @@ func (s *balanceRegionScheduler) transferPeer(cluster schedule.Cluster, region * schedulerCounter.WithLabelValues(s.GetName(), "skip").Inc() return nil } + newPeer, err := cluster.AllocPeer(storeID) + if err != nil { + schedulerCounter.WithLabelValues(s.GetName(), "no_peer").Inc() + } return schedule.CreateMovePeerOperator("balance-region", cluster, region, schedule.OpBalance, oldPeer.GetStoreId(), newPeer.GetStoreId(), newPeer.GetId()) } From b6fa3f73438180d31bdfb25645c7e6d826118657 Mon Sep 17 00:00:00 2001 From: nolouch Date: Sun, 8 Apr 2018 15:20:50 +0800 Subject: [PATCH 2/4] address comments --- server/schedule/replica_checker.go | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/server/schedule/replica_checker.go b/server/schedule/replica_checker.go index dc891860f0d..b9000a2d80c 100644 --- a/server/schedule/replica_checker.go +++ b/server/schedule/replica_checker.go @@ -81,20 +81,6 @@ func (r *ReplicaChecker) Check(region *core.RegionInfo) *Operator { return r.checkBestReplacement(region) } -// SelectBestReplacedPeerToAddReplica returns a new peer that to be used to replace the old peer and distinct score. -func (r *ReplicaChecker) SelectBestReplacedPeerToAddReplica(region *core.RegionInfo, oldPeer *metapb.Peer, filters ...Filter) *metapb.Peer { - storeID, _ := r.SelectBestReplacementStore(region, oldPeer, filters...) - if storeID == 0 { - log.Debugf("[region %d] no best store to add replica", region.GetId()) - return nil - } - newPeer, err := r.cluster.AllocPeer(storeID) - if err != nil { - return nil - } - return newPeer -} - // SelectBestReplacementStore returns a store id that to be used to replace the old peer and distinct score. func (r *ReplicaChecker) SelectBestReplacementStore(region *core.RegionInfo, oldPeer *metapb.Peer, filters ...Filter) (uint64, float64) { filters = append(filters, NewExcludedFilter(nil, region.GetStoreIds())) @@ -198,9 +184,14 @@ func (r *ReplicaChecker) checkOfflinePeer(region *core.RegionInfo) *Operator { return CreateRemovePeerOperator("removePendingOfflineReplica", r.cluster, OpReplica, region, peer.GetStoreId()) } - newPeer := r.SelectBestReplacedPeerToAddReplica(region, peer) - if newPeer == nil { - log.Debugf("[region %d] no best peer to add replica", region.GetId()) + storeID, _ := r.SelectBestReplacementStore(region, peer) + if storeID == 0 { + log.Debugf("[region %d] no best store to add replica", region.GetId()) + return nil + } + newPeer, err := r.cluster.AllocPeer(storeID) + if err != nil { + log.Info("alloc peer meet error:", err) return nil } return CreateMovePeerOperator("makeUpOfflineReplica", r.cluster, region, OpReplica, peer.GetStoreId(), newPeer.GetStoreId(), newPeer.GetId()) @@ -228,6 +219,7 @@ func (r *ReplicaChecker) checkBestReplacement(region *core.RegionInfo) *Operator } newPeer, err := r.cluster.AllocPeer(storeID) if err != nil { + log.Info("alloc peer meet error:", err) return nil } checkerCounter.WithLabelValues("replica_checker", "new_operator").Inc() From b7ae9f4f28d11f98faa5212e4ac5866924ad86bc Mon Sep 17 00:00:00 2001 From: nolouch Date: Sun, 8 Apr 2018 16:40:19 +0800 Subject: [PATCH 3/4] address comments --- server/schedule/replica_checker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/schedule/replica_checker.go b/server/schedule/replica_checker.go index b9000a2d80c..8e0d7664375 100644 --- a/server/schedule/replica_checker.go +++ b/server/schedule/replica_checker.go @@ -191,7 +191,7 @@ func (r *ReplicaChecker) checkOfflinePeer(region *core.RegionInfo) *Operator { } newPeer, err := r.cluster.AllocPeer(storeID) if err != nil { - log.Info("alloc peer meet error:", err) + log.Error("alloc peer meet error:", err) return nil } return CreateMovePeerOperator("makeUpOfflineReplica", r.cluster, region, OpReplica, peer.GetStoreId(), newPeer.GetStoreId(), newPeer.GetId()) @@ -219,7 +219,7 @@ func (r *ReplicaChecker) checkBestReplacement(region *core.RegionInfo) *Operator } newPeer, err := r.cluster.AllocPeer(storeID) if err != nil { - log.Info("alloc peer meet error:", err) + log.Error("alloc peer meet error:", err) return nil } checkerCounter.WithLabelValues("replica_checker", "new_operator").Inc() From b82c162eb5876dd2ad21fbbe56f6fa7769ca74e8 Mon Sep 17 00:00:00 2001 From: nolouch Date: Mon, 9 Apr 2018 20:24:01 +0800 Subject: [PATCH 4/4] address comments --- server/schedule/replica_checker.go | 2 -- server/schedulers/balance_region.go | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/server/schedule/replica_checker.go b/server/schedule/replica_checker.go index 8e0d7664375..cdc43a483d6 100644 --- a/server/schedule/replica_checker.go +++ b/server/schedule/replica_checker.go @@ -191,7 +191,6 @@ func (r *ReplicaChecker) checkOfflinePeer(region *core.RegionInfo) *Operator { } newPeer, err := r.cluster.AllocPeer(storeID) if err != nil { - log.Error("alloc peer meet error:", err) return nil } return CreateMovePeerOperator("makeUpOfflineReplica", r.cluster, region, OpReplica, peer.GetStoreId(), newPeer.GetStoreId(), newPeer.GetId()) @@ -219,7 +218,6 @@ func (r *ReplicaChecker) checkBestReplacement(region *core.RegionInfo) *Operator } newPeer, err := r.cluster.AllocPeer(storeID) if err != nil { - log.Error("alloc peer meet error:", err) return nil } checkerCounter.WithLabelValues("replica_checker", "new_operator").Inc() diff --git a/server/schedulers/balance_region.go b/server/schedulers/balance_region.go index c01f563d35b..280f62253d9 100644 --- a/server/schedulers/balance_region.go +++ b/server/schedulers/balance_region.go @@ -157,6 +157,7 @@ func (s *balanceRegionScheduler) transferPeer(cluster schedule.Cluster, region * newPeer, err := cluster.AllocPeer(storeID) if err != nil { schedulerCounter.WithLabelValues(s.GetName(), "no_peer").Inc() + return nil } return schedule.CreateMovePeerOperator("balance-region", cluster, region, schedule.OpBalance, oldPeer.GetStoreId(), newPeer.GetStoreId(), newPeer.GetId())