scheduler: introduce the rank interface #8353

rleungx · 2024-07-02T09:08:48Z

What problem does this PR solve?

Issue Number: Ref #5691

What is changed and how does it work?

Check List

Tests

Unit test

Release note

None.

lhy1024 · 2024-07-02T10:19:00Z

pkg/schedule/schedulers/hot_region_rank_v1.go

@@ -0,0 +1,243 @@
+// Copyright 2017 TiKV Project Authors.


Suggested change

// Copyright 2017 TiKV Project Authors.

// Copyright 2024 TiKV Project Authors.

rleungx · 2024-07-03T02:45:32Z

/test ?

ti-chi-bot · 2024-07-03T02:45:35Z

@rleungx: The following commands are available to trigger required jobs:

/test build

The following commands are available to trigger optional jobs:

/test pull-integration-copr-test
/test pull-integration-realcluster-test

Use /test all to run the following jobs that were automatically triggered:

tikv/pd/ghpr_build
tikv/pd/pull_integration_realcluster_test

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rleungx · 2024-07-03T02:48:55Z

/test pull-integration-realcluster-test

okJiang · 2024-07-03T08:50:35Z

What problem does this PR solve?🤔

codecov · 2024-07-09T05:21:49Z

Codecov Report

Attention: Patch coverage is 84.91379% with 35 lines in your changes missing coverage. Please review.

Project coverage is 77.36%. Comparing base (624b6f3) to head (aae0f36).
Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8353      +/-   ##
==========================================
+ Coverage   77.34%   77.36%   +0.01%     
==========================================
  Files         471      472       +1     
  Lines       61914    61903      -11     
==========================================
  Hits        47890    47890              
+ Misses      10448    10434      -14     
- Partials     3576     3579       +3

Flag	Coverage Δ
unittests	`77.36% <84.91%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

okJiang · 2024-07-10T08:17:01Z

pkg/schedule/schedulers/hot_region.go

+	// for metrics
 	for storeID, info := range storeInfos {
 		storeLabel := strconv.FormatUint(storeID, 10)
 		if infl := info.PendingSum; infl != nil && len(infl.Loads) != 0 {
 			utils.ForeachRegionStats(func(rwTy utils.RWType, dim int, kind utils.RegionStatKind) {
-				setHotPendingInfluenceMetrics(storeLabel, rwTy.String(), utils.DimToString(dim), infl.Loads[kind])
+				HotPendingSum.WithLabelValues(storeLabel, rwTy.String(), utils.DimToString(dim)).Set(infl.Loads[kind])
 			})
 		}
 	}
 }



It is better to wrap a function for these codes

I think it's unnecessary.

okJiang · 2024-07-10T08:22:49Z

pkg/schedule/schedulers/hot_region.go

@@ -533,19 +535,8 @@ type balanceSolver struct {
 	maxPeerNum    int
 	minHotDegree  int

-	firstPriorityV2Ratios  *rankV2Ratios
-	secondPriorityV2Ratios *rankV2Ratios
-
 	// The rank correlation function used according to the version


Is this note still needed?

okJiang · 2024-07-10T08:23:43Z

pkg/schedule/schedulers/hot_region.go

 	default:
-		bs.initRankV2()
+		bs.rank = initRankV2(bs, bs.greatDecRatio)


Why not input bs only?

okJiang · 2024-07-10T08:25:26Z

pkg/schedule/schedulers/hot_region.go

-	rankToDimString             func() string
-	checkByPriorityAndTolerance func(loads []float64, f func(int) bool) bool
-	checkHistoryLoadsByPriority func(loads [][]float64, f func(int) bool) bool
+	rank rank


How about this?

Suggested change

rank rank

rank

okJiang · 2024-07-10T08:33:52Z

pkg/schedule/schedulers/hot_region_rank_v1.go

+}
+
+func (*rankV1) needSearchRevertRegions() bool { return false }
+func (*rankV1) setSearchRevertRegions()       {}


🤔

Suggested change

func (*rankV1) setSearchRevertRegions() {}

func (*rankV1) setSearchRevertRegions() {}

okJiang · 2024-07-10T08:36:46Z

pkg/schedule/schedulers/hot_region_rank_v2.go

@@ -59,7 +51,7 @@ type balanceChecker struct {
 // If the ratio is between the two, it is considered to be in the pre-balanced state.

 // TODO: Unified with stddevThreshold.
-type rankV2Ratios struct {
+type rankRatios struct {


please update all rankV2Ratios to rankRatios

okJiang · 2024-07-10T08:39:05Z

pkg/schedule/schedulers/hot_region_rank_v2.go

 }

 // pickCheckPolicyV2 will set checkByPriorityAndTolerance to the corresponding function.
 // Note: PolicyV2 will search more possible solutions than PolicyV1.
 // so it allows to schedule when any of the two dimensions is not balanced.
-func (bs *balanceSolver) pickCheckPolicyV2() {
+func (r *rankV2) checkHistoryLoadsByPriority(loads [][]float64, f func(int) bool) bool {


please update comment

okJiang · 2024-07-10T08:39:14Z

pkg/schedule/schedulers/hot_region_rank_v2.go

 	}
 }

 // filterUniformStoreV2 filters stores by stddev.
 // stddev is the standard deviation of the store's load for all stores.
-func (bs *balanceSolver) filterUniformStoreV2() (string, bool) {
-	if !bs.enableExpectation() {
+func (r *rankV2) filterUniformStore() (string, bool) {


okJiang · 2024-07-10T08:39:39Z

pkg/schedule/schedulers/hot_region_rank_v2.go

-	bs.cur.calcPeersRate(bs.firstPriority, bs.secondPriority)
-	if bs.cur.getPeersRateFromCache(bs.firstPriority) < bs.getMinRate(bs.firstPriority) &&
-		bs.cur.getPeersRateFromCache(bs.secondPriority) < bs.getMinRate(bs.secondPriority) {
+func (r *rankV2) calcProgressiveRank() {


ti-chi-bot · 2024-07-19T06:36:51Z

@okJiang: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: Ryan Leung <rleungx@gmail.com>

HuSharp · 2024-07-24T14:04:47Z

pkg/schedule/schedulers/hot_region_rank_v2.go

 		futureChecker:  futureStateChecker,
 		currentChecker: currentStateChecker,
 		perceivedRatio: perceivedRatio, minHotRatio: minHotRatio}

 	return rs
 }

-func (bs *balanceSolver) initRankV2() {
-	bs.firstPriorityV2Ratios = newRankV2Ratios(bs.sche.conf.GetGreatDecRatio(), firstPriorityPerceivedRatio, firstPriorityMinHotRatio)
-	// The second priority is less demanding. Set the preBalancedRatio of the first priority to the balancedRatio of the second dimension.


Do we need to keep this comment?

Signed-off-by: Ryan Leung <rleungx@gmail.com>

ti-chi-bot · 2024-07-25T02:42:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: HuSharp, lhy1024, okJiang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [HuSharp,lhy1024]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-07-25T02:42:07Z

[LGTM Timeline notifier]

Timeline:

2024-07-02 10:20:45.879557509 +0000 UTC m=+1319772.365046341: ☑️ agreed by lhy1024.
2024-07-25 02:42:06.449384542 +0000 UTC m=+1099348.440326012: ☑️ agreed by HuSharp.

ti-chi-bot bot added do-not-merge/needs-linked-issue dco-signoff: yes Indicates the PR's author has signed the dco. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 2, 2024

lhy1024 approved these changes Jul 2, 2024

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Jul 2, 2024

rleungx requested a review from nolouch July 3, 2024 02:27

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Jul 3, 2024

rleungx force-pushed the introduce-rank-interface branch from 998fffa to a48ee63 Compare July 4, 2024 09:51

okJiang reviewed Jul 10, 2024

View reviewed changes

okJiang approved these changes Jul 19, 2024

View reviewed changes

rleungx added 4 commits July 23, 2024 11:58

introduce the rank interface

ae98917

Signed-off-by: Ryan Leung <rleungx@gmail.com>

fix

cbfb7f1

Signed-off-by: Ryan Leung <rleungx@gmail.com>

fix

45e8b74

Signed-off-by: Ryan Leung <rleungx@gmail.com>

fix test

f5e0b4b

Signed-off-by: Ryan Leung <rleungx@gmail.com>

rleungx force-pushed the introduce-rank-interface branch from c477490 to f5e0b4b Compare July 23, 2024 03:59

HuSharp reviewed Jul 24, 2024

View reviewed changes

address the comment

aae0f36

Signed-off-by: Ryan Leung <rleungx@gmail.com>

HuSharp approved these changes Jul 25, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 25, 2024

Merge branch 'master' into introduce-rank-interface

23a9d88

ti-chi-bot bot merged commit 1f14057 into tikv:master Jul 25, 2024
21 checks passed

rleungx deleted the introduce-rank-interface branch July 25, 2024 03:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

scheduler: introduce the rank interface #8353

scheduler: introduce the rank interface #8353

rleungx commented Jul 2, 2024 •

edited

Loading

lhy1024 Jul 2, 2024

rleungx commented Jul 3, 2024

ti-chi-bot bot commented Jul 3, 2024

rleungx commented Jul 3, 2024

okJiang commented Jul 3, 2024

codecov bot commented Jul 9, 2024 •

edited

Loading

okJiang Jul 10, 2024

rleungx Jul 18, 2024

okJiang Jul 10, 2024

okJiang Jul 10, 2024

okJiang Jul 10, 2024

okJiang Jul 10, 2024

okJiang Jul 10, 2024

okJiang Jul 10, 2024

okJiang Jul 10, 2024

okJiang Jul 10, 2024

ti-chi-bot bot commented Jul 19, 2024

HuSharp Jul 24, 2024

ti-chi-bot bot commented Jul 25, 2024

ti-chi-bot bot commented Jul 25, 2024

	// Copyright 2017 TiKV Project Authors.
	// Copyright 2024 TiKV Project Authors.

	func (*rankV1) setSearchRevertRegions() {}
	func (*rankV1) setSearchRevertRegions() {}

scheduler: introduce the rank interface #8353

scheduler: introduce the rank interface #8353

Conversation

rleungx commented Jul 2, 2024 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Choose a reason for hiding this comment

rleungx commented Jul 3, 2024

ti-chi-bot bot commented Jul 3, 2024

rleungx commented Jul 3, 2024

okJiang commented Jul 3, 2024

codecov bot commented Jul 9, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Jul 19, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Jul 25, 2024

ti-chi-bot bot commented Jul 25, 2024

[LGTM Timeline notifier]

rleungx commented Jul 2, 2024 •

edited

Loading

codecov bot commented Jul 9, 2024 •

edited

Loading