Merge #79667 #79669 #79689 #79690

79667: logictest: Ignore test flake in disjunction_in_join r=msirek a=msirek

This commit comments out a flaky test which will be fixed by #74554.

Release note: none

79669: dev: address paper cuts and improve `dev <cmd> -h` consistency r=irfansharif a=irfansharif

- Disable test sharding under --rewrite; sharded package test runners
  can trample over one another when updating data files
- Generate cgo files with naked `dev gen`
- Remove the `dev gen go+docs` target but retain the fast path when both
  the `go` and `docs` targets are specified (`dev gen go docs` for e.g.)
- Remove the `all` and `all_tests` moniker for `dev test`, and retain
  the sanitized path when testing everything using `dev test pkg/...`.
  The latter syntax feels closer to the `go test` expansion.
- Edit some help text padding to wrap more cleanly in terminals

Release note: None

79689: ci: build `cockroach-sql` binary r=jlinder a=rail

Previously, the `cockroach-sql` binary was added to the source code, but
has never been added as a compilation target in our CI.

This patch enables building `cockroach-sql` as a part of CI.

Release note: None

79690: changefeedccl: Make scan request size configurable. r=miretskiy a=miretskiy

Add a `changefeed.backfill.scan_request_size` setting to control
scan request size during backfill.  The default is maintained
at 16MB.  However, some latency sensitive environments may choose
to lower this setting, while increasing scan parallelism to
ensure that the latches are held for shorter periods of time.

Release Notes: Add a `changefeed.backfill.scan_request_size` setting
to control scan request size during backfill.

Relese Justification: Low danger stability and performance improvement.

Co-authored-by: Mark Sirek <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Yevgeniy Miretskiy <[email protected]>

build/teamcity/cockroach/ci/builds/build_impl.sh

@@ -24,4 +24,5 @@ bazel build //pkg/cmd/bazci --config=ci
 $(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci --compilation_mode opt \
 		       --config "$CONFIG" --config ci --config with_ui \
 		       build //pkg/cmd/cockroach-short //pkg/cmd/cockroach \
+               //pkg/cmd/cockroach-sql \
 		       //pkg/cmd/cockroach-oss //c-deps:libgeos $EXTRA_TARGETS

pkg/ccl/changefeedccl/changefeedbase/settings.go

@@ -108,6 +108,14 @@ var ScanRequestLimit = settings.RegisterIntSetting(
 	0,
 )
 
+// ScanRequestSize is the target size of the scan request response.
+var ScanRequestSize = settings.RegisterIntSetting(
+	settings.TenantWritable,
+	"changefeed.backfill.scan_request_size",
+	"the maximum number of bytes returned by each scan request",
+	16<<20,
+)
+
 // SinkThrottleConfig describes throttling configuration for the sink.
 // 0 values for any of the settings disable that setting.
 type SinkThrottleConfig struct {

pkg/ccl/changefeedccl/kvfeed/scanner.go

@@ -128,7 +128,7 @@ func (p *scanRequestScanner) exportSpan(
 	}
 	stopwatchStart := timeutil.Now()
 	var scanDuration, bufferDuration time.Duration
-	const targetBytesPerScan = 16 << 20 // 16 MiB
+	targetBytesPerScan := changefeedbase.ScanRequestSize.Get(&p.settings.SV)
 	for remaining := &span; remaining != nil; {
 		start := timeutil.Now()
 		b := txn.NewBatch()

pkg/cmd/dev/build.go

@@ -43,7 +43,7 @@ func makeBuildCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Com
 		Short: "Build the specified binaries",
 		Long: fmt.Sprintf(
 			"Build the specified binaries either using their bazel targets or one "+
-				"of the following shorthands:\n\t%s",
+				"of the following shorthands:\n\n\t%s",
 			strings.Join(allBuildTargets, "\n\t"),
 		),
 		// TODO(irfansharif): Flesh out the example usage patterns.
@@ -55,11 +55,7 @@ func makeBuildCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Com
 		RunE: runE,
 	}
 	buildCmd.Flags().String(volumeFlag, "bzlhome", "the Docker volume to use as the container home directory (only used for cross builds)")
-	buildCmd.Flags().String(crossFlag, "", `
-        Turns on cross-compilation. Builds the binary using the builder image w/ Docker.
-        You can optionally set a config, as in --cross=windows.
-        Defaults to linux if not specified. The config should be the name of a
-        build configuration specified in .bazelrc, minus the "cross" prefix.`)
+	buildCmd.Flags().String(crossFlag, "", "cross-compiles using the builder image (options: linux, linuxarm, macos, macosarm, windows)")
 	buildCmd.Flags().Lookup(crossFlag).NoOptDefVal = "linux"
 	addCommonBuildFlags(buildCmd)
 	return buildCmd

pkg/cmd/dev/generate.go

@@ -31,30 +31,22 @@ func makeGenerateCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.
 		Use:     "generate [target..]",
 		Aliases: []string{"gen"},
 		Short:   `Generate the specified files`,
-		Long: `Generate the specified files.
-` + "`dev generate bazel`" + ` updates BUILD.bazel files and other Bazel files like DEPS.bzl.
-Use the --mirror option if vendoring a new dependency that needs to be mirrored.
-` + "`dev generate go`" + ` populates the workspace with generated code.
-` + "`dev generate docs`" + ` updates generated documentation.
-` + "`dev generate go+docs`" + ` does the same as ` + "`dev generate go docs`" + `, but slightly faster.
-` + "`dev generate cgo`" + ` populates the workspace with a few zcgo_flags.go files that can prepare non-Bazel build systems to link in our C dependencies.
-` + "`dev generate protobuf`" + ` generates a subset of the code that ` + "`dev generate go`" + ` does, specifically the .pb.go files.`,
+		Long:    `Generate the specified files.`,
 		Example: `
         dev generate
-        dev generate bazel
-        dev generate docs
-        dev generate go
-        dev generate protobuf
-        dev generate cgo
-        dev generate go+docs
+        dev generate bazel     # DEPS.bzl and BUILD.bazel files
+        dev generate cgo       # files that help non-Bazel systems (IDEs, go) link to our C dependencies
+        dev generate docs      # generates documentation
+        dev generate go        # generates go code (execgen, stringer, protobufs, etc.)
+        dev generate protobuf  # *.pb.go files (subset of 'dev generate go')
 `,
 		Args: cobra.MinimumNArgs(0),
 		// TODO(irfansharif): Errors but default just eaten up. Let's wrap these
 		// invocations in something that prints out the appropriate error log
 		// (especially considering we've SilenceErrors-ed things away).
 		RunE: runE,
 	}
-	generateCmd.Flags().Bool(mirrorFlag, false, "mirror new dependencies to cloud storage")
+	generateCmd.Flags().Bool(mirrorFlag, false, "mirror new dependencies to cloud storage (use if vendoring)")
 	generateCmd.Flags().Bool(forceFlag, false, "force regeneration even if relevant files are unchanged from upstream")
 	return generateCmd
 }
@@ -66,14 +58,27 @@ func (d *dev) generate(cmd *cobra.Command, targets []string) error {
 		"docs":     d.generateDocs,
 		"go":       d.generateGo,
 		"protobuf": d.generateProtobuf,
-		"go+docs":  d.generateGoAndDocs,
 	}
 
 	if len(targets) == 0 {
-		targets = append(targets, "bazel", "go+docs")
+		targets = append(targets, "bazel", "go", "docs", "cgo")
 	}
 
+	targetsMap := make(map[string]struct{})
 	for _, target := range targets {
+		targetsMap[target] = struct{}{}
+	}
+	_, includesGo := targetsMap["go"]
+	_, includesDocs := targetsMap["docs"]
+	if includesGo && includesDocs {
+		delete(targetsMap, "go")
+		delete(targetsMap, "docs")
+		if err := d.generateGoAndDocs(cmd); err != nil {
+			return err
+		}
+	}
+
+	for target := range targetsMap {
 		generator, ok := generatorTargetMapping[target]
 		if !ok {
 			return fmt.Errorf("unrecognized target: %s", target)

pkg/cmd/dev/test.go

@@ -45,16 +45,15 @@ func makeTestCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Comm
 		Short: `Run the specified tests`,
 		Long: `Run the specified tests.
 
-This command takes a list of packages or build targets and tests all of them.
-The build target can be a normal bazel build target (like
-pkg/kv/kvserver:kvserver_test), or it can be a plain directory (like
-pkg/kv/kvserver). Furthermore, we accept the following shorthands: all
-and all_tests.`,
+This command takes a list of packages and tests all of them. It's also
+permissive enough to accept bazel build targets (like
+pkg/kv/kvserver:kvserver_test) instead.`,
 		Example: `
 	dev test
 	dev test pkg/kv/kvserver --filter=TestReplicaGC* -v --timeout=1m
 	dev test pkg/server -f=TestSpanStatsResponse -v --count=5 --vmodule='raft=1'
-        dev test all_tests -v
+	dev test pkg/spanconfig/... pkg/ccl/spanconfigccl/...
+	dev test pkg/... -v
 	dev test --stress --race ...`,
 		Args: cobra.MinimumNArgs(0),
 		RunE: runE,
@@ -104,14 +103,17 @@ func (d *dev) test(cmd *cobra.Command, commandLine []string) error {
 		count         = mustGetFlagInt(cmd, countFlag)
 		vModule       = mustGetFlagString(cmd, vModuleFlag)
 	)
+
+	var disableTestSharding bool
 	if rewrite {
 		ignoreCache = true
+		disableTestSharding = true
 	}
 
 	// Enumerate all tests to run.
 	if len(pkgs) == 0 {
-		// Empty `dev test` does the same thing as `dev test all_tests`.
-		pkgs = append(pkgs, "all_tests")
+		// Empty `dev test` does the same thing as `dev test pkg/...`
+		pkgs = append(pkgs, "pkg/...")
 	}
 
 	var args []string
@@ -122,12 +124,14 @@ func (d *dev) test(cmd *cobra.Command, commandLine []string) error {
 	if race {
 		args = append(args, "--config=race")
 	} else if stress {
-		args = append(args, "--test_sharding_strategy=disabled")
+		disableTestSharding = true
 	}
 
 	var testTargets []string
 	for _, pkg := range pkgs {
-		if pkg == "all" || pkg == "all_tests" {
+		if pkg == "pkg/..." {
+			// Special-cased target to skip known broken-under-bazel and
+			// integration tests.
 			testTargets = append(testTargets, "//pkg:all_tests")
 			continue
 		}
@@ -191,7 +195,7 @@ func (d *dev) test(cmd *cobra.Command, commandLine []string) error {
 		// For sharded test packages, it doesn't make much sense to spawn multiple
 		// test processes that don't end up running anything. Default to running
 		// things in a single process if a filter is specified.
-		args = append(args, "--test_sharding_strategy=disabled")
+		disableTestSharding = true
 	}
 	if short {
 		args = append(args, "--test_arg", "-test.short")
@@ -215,6 +219,9 @@ func (d *dev) test(cmd *cobra.Command, commandLine []string) error {
 		}
 		args = append(args, goTestArgs...)
 	}
+	if disableTestSharding {
+		args = append(args, "--test_sharding_strategy=disabled")
+	}
 
 	args = append(args, d.getTestOutputArgs(stress, verbose, showLogs, streamOutput)...)
 	args = append(args, additionalBazelArgs...)

pkg/cmd/dev/testdata/datadriven/generate

@@ -16,3 +16,24 @@ bazel info workspace --color=no
 export COCKROACH_BAZEL_CAN_MIRROR=1
 export COCKROACH_BAZEL_FORCE_GENERATE=1
 crdb-checkout/build/bazelutil/bazel-generate.sh
+
+exec
+dev generate go
+----
+bazel run //pkg/gen:code
+
+exec
+dev generate docs
+----
+bazel run //pkg/gen:docs
+bazel info workspace --color=no
+crdb-checkout/build/bazelutil/generate_redact_safe.sh
+echo "" > crdb-checkout/docs/generated/redact_safe.md
+
+exec
+dev gen go docs
+----
+bazel run //pkg/gen
+bazel info workspace --color=no
+crdb-checkout/build/bazelutil/generate_redact_safe.sh
+echo "" > crdb-checkout/docs/generated/redact_safe.md

pkg/cmd/dev/testdata/datadriven/test

@@ -16,7 +16,7 @@ bazel test pkg/util/tracing:all --test_env=GOTRACEBACK=all '--test_filter=TestSt
 exec
 dev test pkg/util/tracing -f TestStartChild* -v --show-logs
 ----
-bazel test pkg/util/tracing:all --test_env=GOTRACEBACK=all '--test_filter=TestStartChild*' --test_sharding_strategy=disabled --test_arg -test.v --test_arg -show-logs --test_output all
+bazel test pkg/util/tracing:all --test_env=GOTRACEBACK=all '--test_filter=TestStartChild*' --test_arg -test.v --test_arg -show-logs --test_sharding_strategy=disabled --test_output all
 
 exec
 dev test pkg/util/tracing -f TestStartChild* --ignore-cache
@@ -26,7 +26,7 @@ bazel test pkg/util/tracing:all --nocache_test_results --test_env=GOTRACEBACK=al
 exec
 dev test --stress pkg/util/tracing --filter TestStartChild* --cpus=12 --timeout=25s
 ----
-bazel test --local_cpu_resources=12 --test_sharding_strategy=disabled pkg/util/tracing:all --test_env=GOTRACEBACK=all --test_timeout=85 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=25s -p=12' '--test_filter=TestStartChild*' --test_sharding_strategy=disabled --test_output streamed
+bazel test --local_cpu_resources=12 pkg/util/tracing:all --test_env=GOTRACEBACK=all --test_timeout=85 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=25s -p=12' '--test_filter=TestStartChild*' --test_sharding_strategy=disabled --test_output streamed
 
 exec
 dev test //pkg/testutils --timeout=10s
@@ -67,12 +67,12 @@ exec
 dev test pkg/cmd/dev -f TestDataDriven/test --rewrite -v
 ----
 bazel info workspace --color=no
-bazel test pkg/cmd/dev:all --nocache_test_results --test_env=GOTRACEBACK=all --test_env=COCKROACH_WORKSPACE=crdb-checkout --test_arg -rewrite --sandbox_writable_path=crdb-checkout/pkg/cmd/dev --test_filter=TestDataDriven/test --test_sharding_strategy=disabled --test_arg -test.v --test_output all
+bazel test pkg/cmd/dev:all --nocache_test_results --test_env=GOTRACEBACK=all --test_env=COCKROACH_WORKSPACE=crdb-checkout --test_arg -rewrite --sandbox_writable_path=crdb-checkout/pkg/cmd/dev --test_filter=TestDataDriven/test --test_arg -test.v --test_sharding_strategy=disabled --test_output all
 
 exec
 dev test pkg/server -f=TestSpanStatsResponse -v --count=5 --vmodule=raft=1
 ----
-bazel test pkg/server:all --test_env=GOTRACEBACK=all --test_filter=TestSpanStatsResponse --test_sharding_strategy=disabled --test_arg -test.v --test_arg -test.count=5 --test_arg -vmodule=raft=1 --test_output all
+bazel test pkg/server:all --test_env=GOTRACEBACK=all --test_filter=TestSpanStatsResponse --test_arg -test.v --test_arg -test.count=5 --test_arg -vmodule=raft=1 --test_sharding_strategy=disabled --test_output all
 
 exec
 dev test --short
@@ -83,20 +83,41 @@ exec
 dev test pkg/kv/kvserver -f TestStoreRangeSplitMergeGeneration --test-args '-test.memprofile=mem.out'
 ----
 bazel info workspace --color=no
-bazel test pkg/kv/kvserver:all --test_env=GOTRACEBACK=all --test_filter=TestStoreRangeSplitMergeGeneration --test_sharding_strategy=disabled --test_arg -test.outputdir=crdb-checkout --sandbox_writable_path=crdb-checkout --test_arg -test.memprofile=mem.out --test_output errors
+bazel test pkg/kv/kvserver:all --test_env=GOTRACEBACK=all --test_filter=TestStoreRangeSplitMergeGeneration --test_arg -test.outputdir=crdb-checkout --sandbox_writable_path=crdb-checkout --test_arg -test.memprofile=mem.out --test_sharding_strategy=disabled --test_output errors
 
 exec
 dev test pkg/ccl/logictestccl -f=TestTenantLogic/3node-tenant/system -v --rewrite
 ----
 bazel info workspace --color=no
-bazel test pkg/ccl/logictestccl:all --nocache_test_results --test_env=GOTRACEBACK=all --test_env=COCKROACH_WORKSPACE=crdb-checkout --test_arg -rewrite --sandbox_writable_path=crdb-checkout/pkg/ccl/logictestccl --sandbox_writable_path=crdb-checkout/pkg/sql/logictest --test_filter=TestTenantLogic/3node-tenant/system --test_sharding_strategy=disabled --test_arg -test.v --test_output all
+bazel test pkg/ccl/logictestccl:all --nocache_test_results --test_env=GOTRACEBACK=all --test_env=COCKROACH_WORKSPACE=crdb-checkout --test_arg -rewrite --sandbox_writable_path=crdb-checkout/pkg/ccl/logictestccl --sandbox_writable_path=crdb-checkout/pkg/sql/logictest --test_filter=TestTenantLogic/3node-tenant/system --test_arg -test.v --test_sharding_strategy=disabled --test_output all
 
 exec
 dev test pkg/spanconfig/spanconfigkvsubscriber -f=TestDecodeSpanTargets -v --stream-output
 ----
-bazel test pkg/spanconfig/spanconfigkvsubscriber:all --test_env=GOTRACEBACK=all --test_filter=TestDecodeSpanTargets --test_sharding_strategy=disabled --test_arg -test.v --test_output streamed
+bazel test pkg/spanconfig/spanconfigkvsubscriber:all --test_env=GOTRACEBACK=all --test_filter=TestDecodeSpanTargets --test_arg -test.v --test_sharding_strategy=disabled --test_output streamed
 
 exec
 dev test pkg/sql/schemachanger pkg/sql/schemachanger/scplan -- --test_sharding_strategy=disabled
 ----
 bazel test pkg/sql/schemachanger:all pkg/sql/schemachanger/scplan:all --test_env=GOTRACEBACK=all --test_output errors --test_sharding_strategy=disabled
+
+exec
+dev test
+----
+bazel test //pkg:all_tests --test_env=GOTRACEBACK=all --test_output errors
+
+exec
+dev test pkg/...
+----
+bazel test //pkg:all_tests --test_env=GOTRACEBACK=all --test_output errors
+
+exec
+dev test pkg/spanconfig/... pkg/ccl/spanconfigccl/...
+----
+bazel test pkg/spanconfig/...:all pkg/ccl/spanconfigccl/...:all --test_env=GOTRACEBACK=all --test_output errors
+
+exec
+dev test pkg/sql/schemachanger --rewrite -v
+----
+bazel info workspace --color=no
+bazel test pkg/sql/schemachanger:all --nocache_test_results --test_env=GOTRACEBACK=all --test_env=COCKROACH_WORKSPACE=crdb-checkout --test_arg -rewrite --sandbox_writable_path=crdb-checkout/pkg/sql/schemachanger --test_arg -test.v --test_sharding_strategy=disabled --test_output all

pkg/sql/logictest/testdata/logic_test/disjunction_in_join

@@ -474,10 +474,11 @@ SELECT * FROM a WHERE (a1, a2) IN (SELECT c1, d1 FROM c, d WHERE c3 = d3 or c3 =
 ----
 0  0  0  0
 
-query IIII rowsort
-SELECT * FROM a WHERE (a1, a2) IN (SELECT c1, d1 FROM c, d WHERE c3 = d3 or c2 = d2 EXCEPT ALL
-                                   SELECT c1, d1 FROM c, d WHERE c3 = d3 or c2 = d2)
-----
+# TODO: Enable this test once issue #74554 is fixed.
+# query IIII rowsort
+# SELECT * FROM a WHERE (a1, a2) IN (SELECT c1, d1 FROM c, d WHERE c3 = d3 or c2 = d2 EXCEPT ALL
+#                                    SELECT c1, d1 FROM c, d WHERE c3 = d3 or c2 = d2)
+# ----
 
 query IIII rowsort
 SELECT * FROM a WHERE (a1, a2) IN (SELECT c1, d1 FROM c, d WHERE c1 IS NULL OR c1 = d1)