kvserver: use qps for hot ranges sorting #99716
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @kvoli)
pkg/kv/kvserver/store.go line 2692 at r1 (raw file):
// and rebalancing. By default rebalancing uses CPU whilst the UI will use // QPS. rankingsAccumulator := NewReplicaAccumulator(load.CPU, load.Queries)
I'd suggest to rely on s.rebalanceObjManager.Objective().ToDimension() and provide it as an argument to NewReplicaAccumulator. It would guarantee that in case of new objectives added,
it doesn't need any changes.
NewReplicaAccumulator(s.rebalanceObjManager.Objective().ToDimension(), load.Queries)Also it can introduce cases where s.rebalanceObjManager.Objective().ToDimension() is set to qps so NewReplicaAccumulator should eliminate provided objective's duplicates.
I'm not sure how critical so curious to know what do you think?
pkg/kv/kvserver/store_rebalancer_test.go line 505 at r1 (raw file):
func loadRanges(rr *ReplicaRankings, s *Store, ranges []testRange) { // Track both CPU and QPS by default, the ordering the consumer uses will // deepend on the current rebalance objective.
typo: "depend"
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/store.go line 2692 at r1 (raw file):
Previously, koorosh (Andrii Vorobiov) wrote…
I'd suggest to rely on
s.rebalanceObjManager.Objective().ToDimension()and provide it as an argument toNewReplicaAccumulator. It would guarantee that in case of new objectives added,
it doesn't need any changes.NewReplicaAccumulator(s.rebalanceObjManager.Objective().ToDimension(), load.Queries)Also it can introduce cases where
s.rebalanceObjManager.Objective().ToDimension()is set toqpssoNewReplicaAccumulatorshould eliminate provided objective's duplicates.
I'm not sure how critical so curious to know what do you think?
I'm not sure if that would work, this PR purposefully introduces tracking for multiple dimensions at once. I'd rather keep it explicit which dimensions we are tracking. Unless we want to move all tracking into the tenant struct for UI purposes.
pkg/kv/kvserver/store_rebalancer_test.go line 505 at r1 (raw file):
Previously, koorosh (Andrii Vorobiov) wrote…
typo: "depend"
Updated.
kvoli
force-pushed
the
230327.heapk-hottest
branch
3 times, most recently
from
e54df13
to
d00eb94
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:
kvoli
force-pushed
the
230327.heapk-hottest
branch
2 times, most recently
from
38200dc
to
eb423f3
Compare
We introduced CPU balancing by default in cockroachdb#97424. This had the side effect of changing the hot ranges api to return the hottest replicas by CPU, rather than QPS. This patch updates the replica rankings struct to support tracking both by CPU and QPS simultaneously. The hot ranges API collects the top k by QPS and the store rebalancer collects depending on the setting of `kv.allocator.load_based_rebalancing.objective`, which is by default `cpu`. Resolves: cockroachdb#99605 Release note (bug fix): The hot ranges UI page would show hot ranges by CPU and not QPS, depending on the value of `kv.allocator.load_based_rebalancing.objective` (default `cpu`). Now the UI page will always collect based on QPS.
kvoli
force-pushed
the
230327.heapk-hottest
branch
from
eb423f3
to
41e3497
Compare
|
TYFTR bors r=koorosh |
|
Build succeeded: |
99108: server: deflake test hot ranges response r=koorosh a=koorosh Flaky tests for HotRanges api were caused after changing source for getting stats per replica in `mapToHotReplicasInfo` func. `mapToHotReplicasInfo` func calls `Repl().LoadStats()` that internally relies on calculates average values per second in current moment, but values could be recorded earlier. Now, `mapToHotReplicasInfo` func relies on `RangeUsageInfo()` func that keeps average stats at the time stats were recorded. Related change to this fix: #99716 Release note: None Fixes: #98619 Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com>
We introduced CPU balancing by default in #97424. This had the side effect of changing the hot ranges api to return the hottest replicas by CPU, rather than QPS.
This patch updates the replica rankings struct to support tracking both by CPU and QPS simultaneously. The hot ranges API collects the top k by QPS and the store rebalancer collects depending on the setting of
kv.allocator.load_based_rebalancing.objective, which is by defaultcpu.Resolves: #99605