Skip to content

build: add crdb_bench flag #149517

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

herkolategan
Copy link
Collaborator

@herkolategan herkolategan commented Jul 4, 2025

A recent change (#148576) causes some test initializations to throw a panic if
the binary was not built with crdb_test.

./dev bench pkg/sql/catalog/lease -f=BenchmarkAcquireLeaseConcurrent
$ bazel test pkg/sql/catalog/lease:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkAcquireLeaseConcurrent --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=/Users/herko/Library/Caches/crdb-test-fixtures --sandbox_writable_path=/Users/herko/Library/Caches/crdb-test-fixtures --test_output streamed

panic: Testing override for schema_locked used in non-test binary.

goroutine 1 [running]:
github.com/cockroachdb/cockroach/pkg/sql.TestForceDisableCreateTableWithSchemaLocked(...)
	pkg/sql/exec_util.go:773
github.com/cockroachdb/cockroach/pkg/sql/catalog/lease_test.TestMain(0x1400132e620?)
	pkg/sql/catalog/lease/main_test.go:29 +0xcc
main.main()

This is problematic for benchmarks, which are not built with crdb_test.
Benchmarks are not built with the crdb_test flag, because this flag enables
metamorphic variables and extra assertions which could interfere with
performance testing.

This change adds a new crdb_bench flag, which is disabled by default. The
assertions added in #148576 are now also skipped when crdb_bench is enabled.

Informs: #148576
Fixes: #149339

Epic: None
Release note: None

A recent change (cockroachdb#148576) causes some test initializations to throw a panic if
the binary was not build with `crdb_test`.

```
./dev bench pkg/sql/catalog/lease -f=BenchmarkAcquireLeaseConcurrent
$ bazel test pkg/sql/catalog/lease:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkAcquireLeaseConcurrent --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=/Users/herko/Library/Caches/crdb-test-fixtures --sandbox_writable_path=/Users/herko/Library/Caches/crdb-test-fixtures --test_output streamed

panic: Testing override for schema_locked used in non-test binary.

goroutine 1 [running]:
github.com/cockroachdb/cockroach/pkg/sql.TestForceDisableCreateTableWithSchemaLocked(...)
	pkg/sql/exec_util.go:773
github.com/cockroachdb/cockroach/pkg/sql/catalog/lease_test.TestMain(0x1400132e620?)
	pkg/sql/catalog/lease/main_test.go:29 +0xcc
main.main()
```

This is problematic for benchmarks, which are not built with `crdb_test`.
Benchmarks are not built with the `crdb_test` flag, because this flag enables
metamorphic variables and extra assertions which could interfere with
performance testing.

This change adds a new `crdb_bench` flag, which is disabled by default. The
assertions added in cockroachdb#148576 is now also skipped when `crdb_bench` is enabled.

Informs: cockroachdb#148576
Fixes: cockroachdb#149339

Epic: None
Release note: None
Copy link

blathers-crl bot commented Jul 4, 2025

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@herkolategan herkolategan force-pushed the hbl/roachprod-microbench-crdb-test branch from a9bc036 to 16ba762 Compare July 4, 2025 09:40
Set the `crdb_bench` flag to `true` when running `dev bench`.

Epic: None
Release note: None
Allow testing override for schema_locked for benchmark builds.

Epic: None
Release note: None
@herkolategan herkolategan force-pushed the hbl/roachprod-microbench-crdb-test branch from 16ba762 to a477a2a Compare July 4, 2025 10:57
@herkolategan herkolategan marked this pull request as ready for review July 4, 2025 12:08
@herkolategan herkolategan requested a review from a team as a code owner July 4, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

roachprod-microbench: panic due to non-crdb-test build
2 participants