roachtest: fix benchmark failing tests #135633
Merged
+57
−62
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
sambhav-jain-16
requested review from
srosenberg and
nameisbhaskar
and removed request for
a team
|
This change is |
sambhav-jain-16
force-pushed
the
fix-tests
branch
from
f121edf
to
da21b79
Compare
This change intends to fix 3 failures detected in benchmark runs after cockroachdb#133035 1. admission-control/elastic-io due to one wrong flag. 2. tpcc/headroom/n4cpu16 due to wrong passing of labels in case of openmetrics, removed suite label since it is not required. 3. tpccbench/* erroring out with nil pointer error in case of openmetrics, reverted the changes in this PR. Will fix in a different PR. Epic: none Fixes: cockroachdb#135393
sambhav-jain-16
force-pushed
the
fix-tests
branch
from
da21b79
to
d7ff3ad
Compare
srosenberg
reviewed
Nov 19, 2024
| @@ -413,7 +413,6 @@ func GetOpenmetricsLabelMap( | |||
| defaultMap := map[string]string{ | |||
| "cloud": c.Cloud().String(), | |||
| "owner": string(t.Spec().(*registry.TestSpec).Owner), | |||
| "suite": t.Spec().(*registry.TestSpec).Suites.String(), | |||
There was a problem hiding this comment.
Can you say more about this change? It's not immediately clear why it's needed by this hotfix.
There was a problem hiding this comment.
tpcc/headroom/n4cpu16 started failing due to the following error.
./cockroach workload run tpcc {pgurl:1-3} --duration 2h0m0s --histogram-export-format openmetrics --histograms perf/stats.om --openmetrics-labels cloud="gce",database="",duration="2h0m0s",owner="test-eng",ramp="5m0s",subtest_1="headroom",subtest_2="n4cpu16",suite="nightly,release_qualification",test="tpcc",test_run_id="teamcity-17804790",warehouses="875" --pprofport 33333 --prometheus-port 2112 --ramp 5m0s --warehouses 875
| | ```
| | stdout: <empty>
| | stderr:I241118 01:55:30.585513 1 workload/cli/run.go:665 [-] 1 random seed: 3344266357670910272
| | Error: error creating metrics exporter: invalid histogram label "release_qualification"
turns out, suites can be multiple values and this caused problems of a label with multiple values. Also, suites is not a necessary label so removed it in this change.
srosenberg
approved these changes
Nov 19, 2024
|
TFTR! bors r=@srosenberg |
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.
This change intends to fix 3 failures detected in benchmark runs after #133035
admission-control/elastic-iodue to one wrong flag.tpcc/headroom/n4cpu16due to wrong passing of labels in case of openmetrics, removed suite label since it is not required.tpccbench/*erroring out with nil pointer error in case of openmetrics, reverted the changes in this PR. Will fix in a different PR.Epic: none
Fixes: #135393