Skip to content

Commit 9919801

Browse files
craig[bot]herkolategantbg
committed
149517: build: add crdb_bench flag r=herkolategan a=herkolategan 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 150847: asim: introduce eval configurations and a chart viewer r=tbg a=tbg This PR - replaces the manual plot command with the automatic generation of all relevant charts. - moves from static PNG image outputs to a single JSON file per run, which can be viewed and analyzed in an interactive HTML viewer. - excludes those generated files from the repository. This significantly reduces churn. - introduces eval configurations (e.g., sma, mma, both) that automatically set the appropriate cluster settings for a test run. - consolidates numerous redundant and messy test files into fewer, cleaner tests that leverage the new configuration system. - add validation to warn against unrealistic test parameters. - address all such warnings. I think some more work is needed to tell, based on the datadriven files along, whether the configured allocator is "doing a good enough job". But at least there are fewer test scenarios now, and none of them configure obviously bogus loads or capacities. Epic: CRDB-52631 151122: kvserver: dump memFS when TestCheckConsistencyInconsistent fails r=tbg a=tbg Informs #150660. Epic: none Co-authored-by: Herko Lategan <[email protected]> Co-authored-by: Tobias Grieger <[email protected]>
4 parents 585a797 + 32cc242 + 13987c8 + 3b06bb3 commit 9919801

File tree

70 files changed

+1333
-1632
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

70 files changed

+1333
-1632
lines changed

.bazelrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ startup --host_jvm_args=-DBAZEL_TRACK_SOURCE_DIRECTORIES=1
2222
build --flag_alias=bazel_code_coverage=//build/toolchains:bazel_code_coverage_flag
2323
build --flag_alias=crdb_test=//build/toolchains:crdb_test_flag
2424
build --flag_alias=crdb_test_off=//build/toolchains:crdb_test_off_flag
25+
build --flag_alias=crdb_bench=//build/toolchains:crdb_bench_flag
2526
build --flag_alias=cross=//build/toolchains:cross_flag
2627
build --flag_alias=dev=//build/toolchains:dev_flag
2728
build --flag_alias=force_build_cdeps=//build/toolchains:force_build_cdeps_flag

build/teamcity/cockroach/nightlies/microbenchmark_build_support.sh

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,13 @@ function build_and_upload_binaries() {
2424
return
2525
fi
2626

27-
config_args="--config=crosslinux --crdb_test_off"
27+
# Check if crdb_bench flag is supported, since we could be building an older
28+
# version that does not support it.
29+
if grep -q "crdb_bench" .bazelrc; then
30+
config_args="--config=crosslinux --crdb_test_off --crdb_bench"
31+
else
32+
config_args="--config=crosslinux --crdb_test_off"
33+
fi
2834
bazel clean
2935
go_test_targets=$(bazel query kind\(go_test, //$BENCH_PACKAGE:all\))
3036
bazel build $config_args $go_test_targets

build/toolchains/BUILD.bazel

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,19 @@ config_setting(
299299
},
300300
)
301301

302+
bool_flag(
303+
name = "crdb_bench_flag",
304+
build_setting_default = False,
305+
visibility = ["//visibility:public"],
306+
)
307+
308+
config_setting(
309+
name = "crdb_bench",
310+
flag_values = {
311+
":crdb_bench_flag": "true",
312+
},
313+
)
314+
302315
bool_flag(
303316
name = "dev_flag",
304317
build_setting_default = False,

dev

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ fi
88
set -euo pipefail
99

1010
# Bump this counter to force rebuilding `dev` on all machines.
11-
DEV_VERSION=111
11+
DEV_VERSION=112
1212

1313
THIS_DIR=$(cd "$(dirname "$0")" && pwd)
1414
BINARY_DIR=$THIS_DIR/bin/dev-versions

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,6 @@ require (
244244
golang.org/x/perf v0.0.0-20230113213139-801c7ef9e5c5
245245
golang.org/x/term v0.30.0
246246
gonum.org/v1/gonum v0.15.1
247-
gonum.org/v1/plot v0.14.0
248247
google.golang.org/genproto/googleapis/api v0.0.0-20230525234035-dd9d682886f9
249248
gopkg.in/yaml.v2 v2.4.0
250249
gopkg.in/yaml.v3 v3.0.1
@@ -458,6 +457,7 @@ require (
458457
golang.org/x/image v0.21.0 // indirect
459458
golang.org/x/tools/go/vcs v0.1.0-deprecated // indirect
460459
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
460+
gonum.org/v1/plot v0.14.0 // indirect
461461
google.golang.org/appengine v1.6.7 // indirect
462462
google.golang.org/genproto/googleapis/bytestream v0.0.0-20230525234009-2805bf891e89 // indirect
463463
google.golang.org/genproto/googleapis/rpc v0.0.0-20230525234030-28d5490b6b19 // indirect

pkg/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,7 @@ ALL_TESTS = [
819819
"//pkg/util/unique:unique_test",
820820
"//pkg/util/uuid:uuid_test",
821821
"//pkg/util/vector:vector_test",
822+
"//pkg/util/vfsutil:vfsutil_test",
822823
"//pkg/util:util_test",
823824
"//pkg/workload/bank:bank_test",
824825
"//pkg/workload/cli:cli_test",
@@ -2792,6 +2793,8 @@ GO_TARGETS = [
27922793
"//pkg/util/uuid:uuid_test",
27932794
"//pkg/util/vector:vector",
27942795
"//pkg/util/vector:vector_test",
2796+
"//pkg/util/vfsutil:vfsutil",
2797+
"//pkg/util/vfsutil:vfsutil_test",
27952798
"//pkg/util:util",
27962799
"//pkg/util:util_test",
27972800
"//pkg/workload/bank:bank",

pkg/cmd/dev/bench.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,10 @@ func (d *dev) bench(cmd *cobra.Command, commandLine []string) error {
140140
if runSepProcessTenant {
141141
args = append(args, "--test_arg", "-run-sep-process-tenant")
142142
}
143-
args = append(args, "--crdb_test_off")
143+
// The `crdb_test` flag enables metamorphic variables and various "extra" debug
144+
// checking code paths that can interfere with performance testing, hence
145+
// it should disabled for benchmarks.
146+
args = append(args, "--crdb_test_off", "--crdb_bench")
144147
if testArgs != "" {
145148
goTestArgs, err := d.getGoTestArgs(ctx, testArgs)
146149
if err != nil {

pkg/cmd/dev/testdata/datadriven/bench

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,35 @@ exec
22
dev bench pkg/spanconfig/...
33
----
44
echo $HOME/.cache
5-
bazel test pkg/spanconfig/...:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=. --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed
5+
bazel test pkg/spanconfig/...:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=. --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --crdb_bench --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed
66

77
exec
88
dev bench pkg/sql/parser --filter=BenchmarkParse
99
----
1010
echo $HOME/.cache
11-
bazel test pkg/sql/parser:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed
11+
bazel test pkg/sql/parser:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --crdb_bench --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed
1212

1313
exec
1414
dev bench pkg/sql/parser --filter=BenchmarkParse --stream-output=false
1515
----
1616
echo $HOME/.cache
17-
bazel test pkg/sql/parser:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors
17+
bazel test pkg/sql/parser:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --crdb_bench --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors
1818

1919
exec
2020
dev bench pkg/bench -f=BenchmarkTracing/1node/scan/trace=off --count=2 --bench-time=10x --bench-mem=false
2121
----
2222
echo $HOME/.cache
23-
bazel test pkg/bench:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.count=2 --test_arg -test.benchtime=10x --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed
23+
bazel test pkg/bench:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.count=2 --test_arg -test.benchtime=10x --crdb_test_off --crdb_bench --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed
2424

2525
exec
2626
dev bench pkg/spanconfig/spanconfigkvsubscriber -f=BenchmarkSpanConfigDecoder --cpus=10 --ignore-cache=false -v --timeout=50s
2727
----
2828
echo $HOME/.cache
29-
bazel test --local_cpu_resources=10 --test_timeout=50 pkg/spanconfig/spanconfigkvsubscriber:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkSpanConfigDecoder --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.v --test_arg -test.benchmem --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed
29+
bazel test --local_cpu_resources=10 --test_timeout=50 pkg/spanconfig/spanconfigkvsubscriber:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkSpanConfigDecoder --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.v --test_arg -test.benchmem --crdb_test_off --crdb_bench --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed
3030

3131
exec
3232
dev bench pkg/bench -f='BenchmarkTracing/1node/scan/trace=off' --test-args '-test.memprofile=mem.out -test.cpuprofile=cpu.out'
3333
----
3434
bazel info workspace --color=no
3535
echo $HOME/.cache
36-
bazel test pkg/bench:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --test_arg -test.outputdir=crdb-checkout --sandbox_writable_path=crdb-checkout --test_arg -test.memprofile=mem.out --test_arg -test.cpuprofile=cpu.out --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed
36+
bazel test pkg/bench:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --test_arg -test.cpu --test_arg 1 --test_arg -test.benchmem --crdb_test_off --crdb_bench --test_arg -test.outputdir=crdb-checkout --sandbox_writable_path=crdb-checkout --test_arg -test.memprofile=mem.out --test_arg -test.cpuprofile=cpu.out --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed

pkg/kv/kvserver/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,7 @@ go_test(
562562
"//pkg/util/tracing/tracingpb",
563563
"//pkg/util/uint128",
564564
"//pkg/util/uuid",
565+
"//pkg/util/vfsutil",
565566
"@com_github_cockroachdb_cockroach_go_v2//crdb",
566567
"@com_github_cockroachdb_crlib//crtime",
567568
"@com_github_cockroachdb_datadriven//:datadriven",

pkg/kv/kvserver/asim/mmaintegration/mma_store_rebalancer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func NewMMAStoreRebalancer(
8888
// Tick is called periodically to check for and apply rebalancing operations
8989
// using mmaprototype.Allocator.
9090
func (msr *MMAStoreRebalancer) Tick(ctx context.Context, tick time.Time, s state.State) {
91-
ctx = msr.ResetAndAnnotateCtx(ctx)
91+
ctx = msr.AnnotateCtx(ctx)
9292
ctx = logtags.AddTag(ctx, "t", tick.Sub(msr.settings.StartTime))
9393

9494
if !msr.currentlyRebalancing &&

0 commit comments

Comments
 (0)