-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
release-22.2: assert on CPU in rebalance/by-load and reduce noise #105616
Merged
kvoli
merged 3 commits into
cockroachdb:release-22.2
from
kvoli:backport_release-22.2-rebalance-by-load
Jun 27, 2023
Merged
release-22.2: assert on CPU in rebalance/by-load and reduce noise #105616
kvoli
merged 3 commits into
cockroachdb:release-22.2
from
kvoli:backport_release-22.2-rebalance-by-load
Jun 27, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, the utility functions for retrieving stats from the timeseries database in roachtest wouldn't allow specifying a sample period nor sources. This commit enhances ts_util.go to enable specifying a sample period via `getMetricsWithSamplePeriod` and updates the `tsQuery` to have a source field - which is applied on queries. Release note: None
The `rebalance/by-load/replicas` roachtests periodically flake due to a known limitation, where a cluster with a small number of ranges may not be properly balanced due to heterogeneous localities (including multi-store) cockroachdb#88829. This commit updates the total number of ranges from 1 per-store, to 5 per-store for `rebalance/by-load/*` roachtests. The roachtest asserted on the lease count, as a proxy for load, assuming that the load evenly hits each lease for the created ranges. However, the principal indicator of load in a read heavy workload is CPU. This commit updates the test assertion to require that every store's CPU is within 10% of the cluster mean. The test assertion previously required that the max-min lease count delta was 0, when no outside splits occurred; or 1 when the number of ranges was greater than the number of stores. The logging format is updated for easier debugging: ``` cpu outside bounds mean=23.9 tolerance=10.0% (±2.4) bounds=[21.5, 26.3] below = [] within = [s1: 22 (-6.7%), s2: 22 (-6.3%)] above = [s3: 26 (+13.0%)] ... cpu within bounds mean=25.7 tolerance=10.0% (±2.6) bounds=[23.2, 28.3] stores=[s1: 24 (-3.0%), s2: 24 (-2.9%), s3: 27 (+5.9%)] ``` resolves: cockroachdb#102801 resolves: cockroachdb#102823 Release note: None
The rebalance-by-load tests assert that the normalized [0,1] CPU utilization of each node is within some threshold of the mean. The threshold was previously 10%, however it is not unexpected that despite replica load being within this threshold, that total node load is not. The current balancing implementation only concerns itself with replica load. Bump the tolerance from 10% to 15% to reduce noise. Additionally, the test did not wait for 3x replication prior to beginning the workload. This is bound to introduce flakes eventually. Wait for 3x replication before beginning. Resolves: cockroachdb#104854 Resolves: cockroachdb#104386 Release note: None
kvoli
changed the title
roachtest: add sample period and source to ts_util
release-22.2: assert on CPU in rebalance/by-load and reduce noise
Jun 27, 2023
aliher1911
approved these changes
Jun 27, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
TYFTR |
This was referenced Jun 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 2/3 commits from #102824 on behalf of @kvoli.
Backport 1/1 commits from #104885 on behalf of @kvoli.
/cc @cockroachdb/release
The rebalance/by-load/replicas roachtests periodically flake due to a
known limitation, where a cluster with a small number of ranges may not
be properly balanced due to heterogeneous localities (including
multi-store) #88829.
This PR updates the total number of ranges from 1 per-store, to 5
per-store for rebalance/by-load/* roachtests.
The roachtest asserted on the lease count, as a proxy for load, assuming
that the load evenly hits each lease for the created ranges. However,
the principal indicator of load in a read heavy workload is CPU.
This PR updates the test assertion to require that every store's CPU
is within 10% of the cluster mean. The test assertion previously
required that the max-min lease count delta was 0, when no outside
splits occurred; or 1 when the number of ranges was greater than the
number of stores.
The logging format is updated for easier debugging:
Release note: None
The rebalance-by-load tests assert that the normalized [0,1] CPU utilization of each node is within some threshold of the mean. The threshold was previously 10%, however it is not unexpected that despite replica load being within this threshold, that total node load is not.
The current balancing implementation only concerns itself with replica load.
Bump the tolerance from 10% to 15% to reduce noise.
Additionally, the test did not wait for 3x replication prior to beginning the workload. This is bound to introduce flakes eventually. Wait for 3x replication before beginning.
Release note: None
Resolves: #105258
Resolves: #100816
Release justification: Test only change for deflake.