Merge cockroachdb#85810 cockroachdb#86167 cockroachdb#86183

85810: build: enable required release justifications for stability period r=matthewtodd a=matthewtodd

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

86167: bazel: make dev bench disable crdb_test r=cucaroach a=cucaroach

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 the new flag
'--crdb-test-on' can be used.

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: cockroachdb#83599

Release note: None
Release justification: Non-production code


86183: server: propagate a testing knob to tenants r=ajwerner a=ajwerner

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 justification: testing only change

Release note: None

Co-authored-by: Matthew Todd <[email protected]>
Co-authored-by: Tommy Reilly <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>

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

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

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 {

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

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'",