roachtest: add a variant of tpch_concurrency with no admission control #75764
Conversation
|
This change is |
yuzefovich
force-pushed
the
tpch_concurrency
branch
from
59e3ea0
to
f110ca1
Compare
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @michae2 and @yuzefovich)
pkg/cmd/roachtest/tests/tpch_concurrency.go, line 196 at r1 (raw file):
for _, disableTxnStatsSampling := range []bool{false, true} { for _, disableAdmissionControl := range []bool{false, true} {
Do we need to multiply these configurations by each other? Does disabling both sampling and admission control at the same time tell us something different than disabling each individually?
What I'm getting at is: if these options are orthogonal to each other, then it seems like busywork to test all combinations of {on,off} for all options. Better to test each option disabled by itself, and then we can compare with the default configuration to know the effect of each option.
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/roachtest/tests/tpch_concurrency.go, line 196 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
Do we need to multiply these configurations by each other? Does disabling both sampling and admission control at the same time tell us something different than disabling each individually?
What I'm getting at is: if these options are orthogonal to each other, then it seems like busywork to test all combinations of {on,off} for all options. Better to test each option disabled by itself, and then we can compare with the default configuration to know the effect of each option.
That's true; however, I do not intend to keep all these options around for long - at the moment I'm just curious in several more datapoints for the Streamer work, and I don't think these configs are that useful on their own. I'll probably remove no_sampling option entirely too since the performance dashboards seem to be about the same, so it's not providing much value either.
Release note: None
yuzefovich
force-pushed
the
tpch_concurrency
branch
from
f110ca1
to
306c3c5
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/roachtest/tests/tpch_concurrency.go, line 196 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
That's true; however, I do not intend to keep all these options around for long - at the moment I'm just curious in several more datapoints for the Streamer work, and I don't think these configs are that useful on their own. I'll probably remove
no_samplingoption entirely too since the performance dashboards seem to be about the same, so it's not providing much value either.
Removed the case when both are disabled.
There was a problem hiding this comment.
Reviewable status:
|
Build failed (retrying...): |
|
Build succeeded: |
Informs: #74836.
Release note: None