New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

May be we can optimize this function? #1359

Open
yangwenmai opened this Issue Dec 4, 2018 · 2 comments

Comments

Projects
None yet
4 participants
@yangwenmai
Copy link
Contributor

yangwenmai commented Dec 4, 2018

  1. Whether can be optimized?

May be we can optimize this logic.

https://github.com/pingcap/pd/blob/master/server/schedulers/hot_region.go#L374

func (h *balanceHotRegionsScheduler) selectDestStore(candidateStoreIDs []uint64, regionFlowBytes uint64, srcStoreID uint64, storesStat core.StoreHotRegionsStat) (destStoreID uint64) {
	sr := storesStat[srcStoreID]
	srcFlowBytes := sr.TotalFlowBytes
	srcHotRegionsCount := sr.RegionsStat.Len()

	var (
		minFlowBytes    uint64 = math.MaxUint64
		minRegionsCount        = int(math.MaxInt32)
	)
	for _, storeID := range candidateStoreIDs {
		if s, ok := storesStat[storeID]; ok {
			if srcHotRegionsCount-s.RegionsStat.Len() > 1 && minRegionsCount > s.RegionsStat.Len() {
				destStoreID = storeID
				minFlowBytes = s.TotalFlowBytes
				minRegionsCount = s.RegionsStat.Len()
				continue
			}
			if minRegionsCount == s.RegionsStat.Len() && minFlowBytes > s.TotalFlowBytes &&
				uint64(float64(srcFlowBytes)*hotRegionScheduleFactor) > s.TotalFlowBytes+2*regionFlowBytes {
				minFlowBytes = s.TotalFlowBytes
				destStoreID = storeID
			}
		} else {
			destStoreID = storeID
			return
		}
	}
	return
}

When looping through candidateStoreIDs, if there are multiple matches, then the last one will always overwrite the data that has been stored in destStoreID. Finally, the destStoreID we get may not be optimal.

  1. What version of PD are you using (pd-server -V)?
Release Version: v2.1.0-rc.2-64-g8e2ec1e
Git Commit Hash: 8e2ec1e832198ea81c8b03717037f4eb2261affc
Git Branch: master
UTC Build Time:  2018-12-04 09:30:07
@disksing

This comment has been minimized.

Copy link
Member

disksing commented Dec 4, 2018

PTAL @nolouch

@nolouch

This comment has been minimized.

Copy link
Member

nolouch commented Dec 4, 2018

@yangwenmai Yes, you are right. storesStat can be updated after the operator finished. so we use the limiter mechanism to ensure the scheduler not produce multiple operators base the same storesStat. maybe you can help up do more better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment