Merge #36333

36333: build: run logic tests in nightly stress r=RaduBerinde a=RaduBerinde

We previously skipped it since it had in the past had a tendency to
overload the machine, but this time try instead to run it with lower
parallelism.

It's particularly important to run logic tests under the nightly
stressrace because they aren't run anywhere else in race builds. Note
that race builds also enable additional internal checks in the
optimizer.

Release note: None

Co-authored-by: Tobias Schottdorf <[email protected]>

pkg/cmd/teamcity-trigger/main.go

@@ -54,21 +54,44 @@ func main() {
 	})
 }
 
-func runTC(queueBuild func(string, map[string]string)) {
-	importPaths := gotool.ImportPaths([]string{"github.com/cockroachdb/cockroach/pkg/..."})
+const baseImportPath = "github.com/cockroachdb/cockroach/pkg/"
+
+var importPaths = gotool.ImportPaths([]string{baseImportPath + "..."})
 
+func runTC(queueBuild func(string, map[string]string)) {
 	// Queue stress builds. One per configuration per package.
-	for _, opts := range []map[string]string{
-		{}, // uninstrumented
-		// The race detector is CPU intensive, so we want to run less processes in
-		// parallel. (Stress, by default, will run one process per CPU.)
+	for _, importPath := range importPaths {
+		// The stress program by default runs as many instances in parallel as there
+		// are CPUs. Each instance itself can run tests in parallel. The amount of
+		// parallelism needs to be reduced, or we can run into OOM issues,
+		// especially for race builds and/or logic tests (see
+		// https://github.com/cockroachdb/cockroach/pull/10966).
 		//
-		// TODO(benesch): avoid assuming that TeamCity agents have eight CPUs.
-		{"env.GOFLAGS": "-race", "env.STRESSFLAGS": "-p 4"},
-	} {
-		for _, importPath := range importPaths {
-			opts["env.PKG"] = importPath
-			queueBuild("Cockroach_Nightlies_Stress", opts)
+		// We limit both the stress program parallelism and the go test parallelism
+		// to 4 for non-race builds and 2 for race builds. For logic tests, we
+		// halve these values.
+		parallelism := 4
+
+		// Stress logic tests with reduced parallelism (to avoid overloading the
+		// machine, see https://github.com/cockroachdb/cockroach/pull/10966).
+		if importPath == baseImportPath+"sql/logictest" {
+			parallelism /= 2
 		}
+
+		opts := map[string]string{
+			"env.PKG": importPath,
+		}
+
+		// Run non-race build.
+		opts["env.GOFLAGS"] = fmt.Sprintf("-parallel=%d", parallelism)
+		opts["env.STRESSFLAGS"] = fmt.Sprintf("-p %d", parallelism)
+		queueBuild("Cockroach_Nightlies_Stress", opts)
+
+		// Run race build. Reduce the parallelism to avoid overloading the machine.
+		parallelism /= 2
+		opts["env.GOFLAGS"] = fmt.Sprintf("-race -parallel=%d", parallelism)
+		opts["env.STRESSFLAGS"] = fmt.Sprintf("-p %d", parallelism)
+
+		queueBuild("Cockroach_Nightlies_Stress", opts)
 	}
 }

pkg/cmd/teamcity-trigger/main_test.go

@@ -15,6 +15,8 @@
 package main
 
 import (
+	"fmt"
+	"sort"
 	"strings"
 	"testing"
 )
@@ -35,3 +37,43 @@ func TestRunTC(t *testing.T) {
 		t.Fatal("no builds were created")
 	}
 }
+
+func Example_runTC() {
+	// Shows sample output for two packages, one of which runs with reduced
+	// parallelism.
+	runTC(func(buildID string, opts map[string]string) {
+		pkg := opts["env.PKG"]
+		if !strings.HasSuffix(pkg, "pkg/sql/logictest") && !strings.HasSuffix(pkg, "pkg/storage") {
+			return
+		}
+		var keys []string
+		for k := range opts {
+			if k != "env.PKG" {
+				keys = append(keys, k)
+			}
+		}
+		sort.Strings(keys)
+		fmt.Println(pkg)
+		for _, k := range keys {
+			fmt.Printf("  %-16s %s\n", k+":", opts[k])
+		}
+		fmt.Println()
+	})
+
+	// Output:
+	// github.com/cockroachdb/cockroach/pkg/sql/logictest
+	//   env.GOFLAGS:     -parallel=2
+	//   env.STRESSFLAGS: -p 2
+	//
+	// github.com/cockroachdb/cockroach/pkg/sql/logictest
+	//   env.GOFLAGS:     -race -parallel=1
+	//   env.STRESSFLAGS: -p 1
+	//
+	// github.com/cockroachdb/cockroach/pkg/storage
+	//   env.GOFLAGS:     -parallel=4
+	//   env.STRESSFLAGS: -p 4
+	//
+	// github.com/cockroachdb/cockroach/pkg/storage
+	//   env.GOFLAGS:     -race -parallel=2
+	//   env.STRESSFLAGS: -p 2
+}

pkg/sql/logictest/logic.go

@@ -2094,10 +2094,9 @@ var logicTestsConfigFilter = envutil.EnvOrDefaultString("COCKROACH_LOGIC_TESTS_C
 // RunLogicTest is the main entry point for the logic test. The globs parameter
 // specifies the default sets of files to run.
 func RunLogicTest(t *testing.T, globs ...string) {
-	if testutils.NightlyStress() {
-		// See https://github.com/cockroachdb/cockroach/pull/10966.
-		t.Skip()
-	}
+	// Note: there is special code in teamcity-trigger/main.go to run this package
+	// with less concurrency in the nightly stress runs. If you see problems
+	// please make adjustments there.
 
 	if skipLogicTests {
 		t.Skip("COCKROACH_LOGIC_TESTS_SKIP")