From 5d7d5c1904aaa1b82a8227dd23a2b3f417c74277 Mon Sep 17 00:00:00 2001 From: Tobias Schottdorf Date: Mon, 11 Mar 2019 09:01:01 +0100 Subject: [PATCH] build: run logic tests in nightly stress 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 --- pkg/cmd/teamcity-trigger/main.go | 47 ++++++++++++++++++++------- pkg/cmd/teamcity-trigger/main_test.go | 42 ++++++++++++++++++++++++ pkg/sql/logictest/logic.go | 7 ++-- 3 files changed, 80 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/teamcity-trigger/main.go b/pkg/cmd/teamcity-trigger/main.go index 6b1ec3d0f411..40510014ad9a 100644 --- a/pkg/cmd/teamcity-trigger/main.go +++ b/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) } } diff --git a/pkg/cmd/teamcity-trigger/main_test.go b/pkg/cmd/teamcity-trigger/main_test.go index e69c8c043b55..06b1fe85f43c 100644 --- a/pkg/cmd/teamcity-trigger/main_test.go +++ b/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 +} diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go index 43e23f5c8e01..dc0faa8ca6e7 100644 --- a/pkg/sql/logictest/logic.go +++ b/pkg/sql/logictest/logic.go @@ -2056,10 +2056,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")