bench: fix tenant benchmark #93616
bench: fix tenant benchmark #93616
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @andreimatei)
There was a problem hiding this comment.
Thanks for fixing this, I just noticed the failures.
Do you know if not accepting underscores is a new thing? Cause this worked yesterday...
Reviewable status:
pkg/bench/foreachdb.go line 72 at r1 (raw file):
// Get a SQL connection to the test tenant. sqlAddr := s.(*server.TestServer).TestingGetSQLAddrForTenant(ctx, tenantName) tenantDB := serverutils.OpenDBConn(b, sqlAddr, "bench", false, s.Stopper())
does this database exist for the tenant?
There was a problem hiding this comment.
Reviewable status:
pkg/bench/foreachdb.go line 60 at r1 (raw file):
s, db, _ := serverutils.StartServer( b, base.TestServerArgs{ UseDatabase: "bench",
remove this pls. Unless it somehow affects the tenant and is the reason why the db is created in the tenant?
There was a problem hiding this comment.
Reviewable status:
pkg/bench/foreachdb.go line 60 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
remove this pls. Unless it somehow affects the tenant and is the reason why the db is created in the tenant?
We would need to augment all benchmarks too because queries there do hard-code bench.<table_name>.
There was a problem hiding this comment.
Reviewable status:
pkg/bench/foreachdb.go line 60 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
We would need to augment all benchmarks too because queries there do hard-code
bench.<table_name>.
This only affects connections returned from this node, which are no longer used by the benchmarks. The benchmarks uise the pool created below, where the "bench" db is added to the URL in this patch.
Although the benchmarks appeared to work without it in the URL too. Or perhaps I had just ran BenchmarkSQL and not others.
pkg/bench/foreachdb.go line 72 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
does this database exist for the tenant?
oh the db is created below, nvm me
There was a problem hiding this comment.
Reviewable status:
pkg/bench/foreachdb.go line 60 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
This only affects connections returned from this node, which are no longer used by the benchmarks. The benchmarks uise the pool created below, where the "bench" db is added to the URL in this patch.
Although the benchmarks appeared to work without it in the URL too. Or perhaps I had just ranBenchmarkSQLand not others.
Oh, I think I misunderstood your suggestion - I thought you were saying to not use bench database and let everything run in defaultdb, nvm.
There was a problem hiding this comment.
Do you know if not accepting underscores is a new thing? Cause this worked yesterday...
Ah, it must have been this #93269
Btw, I think there used to be a time where benchmarks were run (maybe with benchtime=1x in CI) to prevent things like this. Now this (or at least something that fails) seems to be in Bazel extended. Perhaps we could move it?
Reviewable status:
There's some discussion in this link (internal). TFTRs! bors r=yuzefovich,andreimatei |
There was a problem hiding this comment.
Reviewable status:
pkg/bench/foreachdb.go line 60 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
Oh, I think I misunderstood your suggestion - I thought you were saying to not use
benchdatabase and let everything run indefaultdb, nvm.
Please remove this line; it's broken to the extent that it matters.
|
bors r- |
|
Canceled. |
renatolabs
force-pushed
the
fix-tenant-benchmark
branch
from
ec9bf48
to
908b95b
Compare
Fixes argument passed to `crdb_internal.create_tenant` to remove underscore, which is rejected as an invalid name; and also use the `bench` database in `tenantDB`. Relates to cockroachdb#93530. Epic: none Release note: None
renatolabs
force-pushed
the
fix-tenant-benchmark
branch
from
908b95b
to
d34ac1a
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/bench/foreachdb.go line 60 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
Please remove this line; it's broken to the extent that it matters.
Done.
There was a problem hiding this comment.
Reviewable status:
|
bors r=yuzefovich,andreimatei |
|
Build succeeded: |
Fixes argument passed to
crdb_internal.create_tenantto remove underscore, which is rejected as an invalid name; and also use thebenchdatabase intenantDB.Relates to #93530.
Epic: none
Release note: None