From 7c8125cd17a3cb3396688d5e6f8c85a15d19da2f Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Mon, 15 Aug 2022 18:06:48 -0400 Subject: [PATCH 1/3] server: propagate a testing knob to tenants I don't really know if this is the source of problems. I see some non-determinism in ID generation, and I see that the testing knobs passed to the testserver don't make their way to the tenant. I'm hopeful this helps, and sure that this won't hurt. Release note: None --- pkg/server/testserver.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index 504d86af6669..0e2953efe622 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -519,6 +519,10 @@ func (ts *TestServer) maybeStartDefaultTestTenant(ctx context.Context) error { } tempStorageConfig := base.DefaultTestTempStorageConfig(cluster.MakeTestingClusterSettings()) + var useTransactionalDescIDGenerator bool + if knobs, ok := ts.params.Knobs.SQLExecutor.(*sql.ExecutorTestingKnobs); ok { + useTransactionalDescIDGenerator = knobs.UseTransactionalDescIDGenerator + } params := base.TestTenantArgs{ // Currently, all the servers leverage the same tenant ID. We may // want to change this down the road, for more elaborate testing. @@ -537,7 +541,8 @@ func (ts *TestServer) maybeStartDefaultTestTenant(ctx context.Context) error { // successfully. TestingKnobs: base.TestingKnobs{ SQLExecutor: &sql.ExecutorTestingKnobs{ - DeterministicExplain: true, + DeterministicExplain: true, + UseTransactionalDescIDGenerator: useTransactionalDescIDGenerator, }, SQLStatsKnobs: &sqlstats.TestingKnobs{ AOSTClause: "AS OF SYSTEM TIME '-1us'", From 2483872a6dfa027c55be3d50267d2d428690e8cb Mon Sep 17 00:00:00 2001 From: Matthew Todd Date: Tue, 9 Aug 2022 10:49:23 -0400 Subject: [PATCH 2/3] build: enable required release justifications for stability period Addresses RE-235. To begin stability period for the 22.2 release. Will merge Sunday night. Release justification: non-production code change Release note: None --- githooks/commit-msg | 2 +- githooks/prepare-commit-msg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/githooks/commit-msg b/githooks/commit-msg index 146aed08dfb9..dfb1b02da454 100755 --- a/githooks/commit-msg +++ b/githooks/commit-msg @@ -52,7 +52,7 @@ IFS=' notes=($($grep -iE '^release note' "$1")) # Set this to 1 to require a release justification note. -require_justification=0 +require_justification=1 justification=($($grep -iE '^release justification: \S+' "$1")) IFS=$saveIFS diff --git a/githooks/prepare-commit-msg b/githooks/prepare-commit-msg index 1a24e426bd2a..cb69ded5ce23 100755 --- a/githooks/prepare-commit-msg +++ b/githooks/prepare-commit-msg @@ -2,7 +2,7 @@ # # Prepare the commit message by adding a release note. -require_justification=0 +require_justification=1 set -euo pipefail if [[ "${2-}" = "message" ]]; then From 220d6703fd189a5dc5c5e01154fa34e6761dad36 Mon Sep 17 00:00:00 2001 From: Tommy Reilly Date: Mon, 15 Aug 2022 14:59:11 -0400 Subject: [PATCH 3/3] bazel: make dev bench disable crdb_test crdb_test enables metamorphic variables and various "extra" debug checking code paths that can interfere with performance testing. This is also consistent with how make bench behaved. If for whatever reason crdb_test is desirable just pass --crdb_test to the underlying bazel invocation. If you want crdb_test on but no metamorphic variables the bazel test_env argument can be used like so: --test_env=COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING=true Informs: #83599 Release note: None Release justification: non-production code change --- dev | 2 +- pkg/cmd/dev/bench.go | 1 + pkg/cmd/dev/testdata/datadriven/bench | 12 ++++++------ 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dev b/dev index c6c1755a5a03..0ecc418e1c40 100755 --- a/dev +++ b/dev @@ -8,7 +8,7 @@ fi set -euo pipefail # Bump this counter to force rebuilding `dev` on all machines. -DEV_VERSION=52 +DEV_VERSION=53 THIS_DIR=$(cd "$(dirname "$0")" && pwd) BINARY_DIR=$THIS_DIR/bin/dev-versions diff --git a/pkg/cmd/dev/bench.go b/pkg/cmd/dev/bench.go index 32a86a581e41..f158f02ae8e9 100644 --- a/pkg/cmd/dev/bench.go +++ b/pkg/cmd/dev/bench.go @@ -146,6 +146,7 @@ func (d *dev) bench(cmd *cobra.Command, commandLine []string) error { if benchMem { args = append(args, "--test_arg", "-test.benchmem") } + args = append(args, "--crdb_test_off") if testArgs != "" { goTestArgs, err := d.getGoTestArgs(ctx, testArgs) if err != nil { diff --git a/pkg/cmd/dev/testdata/datadriven/bench b/pkg/cmd/dev/testdata/datadriven/bench index 418ab21d9770..4e553b11d028 100644 --- a/pkg/cmd/dev/testdata/datadriven/bench +++ b/pkg/cmd/dev/testdata/datadriven/bench @@ -1,30 +1,30 @@ exec dev bench pkg/spanconfig/... ---- -bazel test pkg/spanconfig/...:all --test_arg -test.run=- --test_arg -test.bench=. --test_output errors +bazel test pkg/spanconfig/...:all --test_arg -test.run=- --test_arg -test.bench=. --crdb_test_off --test_output errors exec dev bench pkg/sql/parser --filter=BenchmarkParse ---- -bazel test pkg/sql/parser:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --test_output errors +bazel test pkg/sql/parser:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --crdb_test_off --test_output errors exec dev bench pkg/sql/parser --filter=BenchmarkParse --stream-output ---- -bazel test pkg/sql/parser:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --test_output streamed +bazel test pkg/sql/parser:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --crdb_test_off --test_output streamed exec dev bench pkg/bench -f=BenchmarkTracing/1node/scan/trace=off --count=2 --bench-time=10x --bench-mem ---- -bazel test pkg/bench:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --test_arg -test.count=2 --test_arg -test.benchtime=10x --test_arg -test.benchmem --test_output errors +bazel test pkg/bench:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --test_arg -test.count=2 --test_arg -test.benchtime=10x --test_arg -test.benchmem --crdb_test_off --test_output errors exec dev bench pkg/spanconfig/spanconfigkvsubscriber -f=BenchmarkSpanConfigDecoder --cpus=10 --ignore-cache -v --timeout=50s ---- -bazel test --local_cpu_resources=10 --test_timeout=55 --test_arg -test.timeout=50s pkg/spanconfig/spanconfigkvsubscriber:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkSpanConfigDecoder --test_sharding_strategy=disabled --test_arg -test.v --test_output all +bazel test --local_cpu_resources=10 --test_timeout=55 --test_arg -test.timeout=50s pkg/spanconfig/spanconfigkvsubscriber:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkSpanConfigDecoder --test_sharding_strategy=disabled --test_arg -test.v --crdb_test_off --test_output all exec dev bench pkg/bench -f='BenchmarkTracing/1node/scan/trace=off' --test-args '-test.memprofile=mem.out -test.cpuprofile=cpu.out' ---- bazel info workspace --color=no -bazel test pkg/bench:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --test_arg -test.outputdir=crdb-checkout --sandbox_writable_path=crdb-checkout --test_arg -test.memprofile=mem.out --test_arg -test.cpuprofile=cpu.out --test_output errors +bazel test pkg/bench:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --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_output errors