From c2606a535210328ae8ba1a506434b17ef3e1b097 Mon Sep 17 00:00:00 2001 From: Stan Rosenberg Date: Wed, 31 May 2023 22:46:15 -0400 Subject: [PATCH] roachtest: require perf. tests to opt in via TestSpec.Benchmark Previously, roachtests which benchmark performance (cf. correctness) were indistinguishable from correctness tests. That is, a performance test is like any other test with the exception of _optionally_ writing stats.json under 'Test.PerfArtifactsDir'; these artifacts are automatically exported to a gcs bucket, used in conjunction with the roachperf dashboard. Having no direct way to distinguish a performance test from a correctness test has several challenges. E.g., performance tests may require a specific machine type or architecture; background workloads like incremental backup may cause a performance regression; new metamorphic configurations like arm64 and fips may require a "bake-in" time before performance tests can be enabled. In future, the test runner may make specialized decisions (e.g., don't reuse a cluster) when executing a performance test. Thus, we need a (standard) mechanism to enumerate all performance tests. Given their specific requirements, the test author must explicitly opt in, by setting TestSpec.Benchmark to 'true'. This PR applies the above change retroactively, i.e., setting 'TestSpec.Benchmark' for all _known_ performance tests, including those which _assert_ on performance instead of exporting stats.json. It also fixes `roachtest list --bench` and `roachtest bench`, which were out-of-date, albeit not actively used. Epic: none Release note: None --- pkg/cmd/roachtest/github_test.go | 7 +++- pkg/cmd/roachtest/main.go | 33 +++++++++---------- pkg/cmd/roachtest/registry/test_spec.go | 8 +++++ pkg/cmd/roachtest/test_registry.go | 21 ++++++++---- pkg/cmd/roachtest/test_registry_test.go | 4 +-- pkg/cmd/roachtest/test_runner.go | 4 ++- pkg/cmd/roachtest/test_test.go | 12 ++++--- pkg/cmd/roachtest/tests/allocator.go | 27 ++++++++------- pkg/cmd/roachtest/tests/backup.go | 1 + pkg/cmd/roachtest/tests/connection_latency.go | 19 ++++++----- pkg/cmd/roachtest/tests/copyfrom.go | 24 +++++++++----- pkg/cmd/roachtest/tests/decommissionbench.go | 4 ++- pkg/cmd/roachtest/tests/import.go | 2 ++ .../roachtest/tests/import_cancellation.go | 9 ++--- pkg/cmd/roachtest/tests/indexes.go | 7 ++-- pkg/cmd/roachtest/tests/kv.go | 17 ++++++---- pkg/cmd/roachtest/tests/ledger.go | 7 ++-- .../tests/loss_of_quorum_recovery.go | 2 ++ pkg/cmd/roachtest/tests/registry.go | 11 ------- pkg/cmd/roachtest/tests/restore.go | 10 +++--- pkg/cmd/roachtest/tests/schemachange.go | 20 ++++++----- .../tests/schemachange_random_load.go | 6 ++-- pkg/cmd/roachtest/tests/scrub.go | 8 +++-- pkg/cmd/roachtest/tests/tpcc.go | 10 +++--- pkg/cmd/roachtest/tests/tpch_concurrency.go | 10 ++++++ pkg/cmd/roachtest/tests/tpchbench.go | 7 ++-- pkg/cmd/roachtest/tests/ycsb.go | 21 +++++++----- 27 files changed, 188 insertions(+), 123 deletions(-) diff --git a/pkg/cmd/roachtest/github_test.go b/pkg/cmd/roachtest/github_test.go index 094aed9ba900..8b49dfe86c7a 100644 --- a/pkg/cmd/roachtest/github_test.go +++ b/pkg/cmd/roachtest/github_test.go @@ -74,7 +74,8 @@ func TestShouldPost(t *testing.T) { {false, 1, "token", "master", true}, } - reg, _ := makeTestRegistry(spec.GCE, "", "", false) + reg := makeTestRegistry(spec.GCE, "", "", false, false) + for _, c := range testCases { t.Setenv("GITHUB_API_TOKEN", c.envGithubAPIToken) t.Setenv("TC_BUILD_BRANCH", c.envTcBuildBranch) @@ -145,8 +146,12 @@ func TestCreatePostRequest(t *testing.T) { {true, false, true, false, otherErr, false, nil}, } +<<<<<<< HEAD reg, _ := makeTestRegistry(spec.GCE, "", "", false) +======= + reg := makeTestRegistry(spec.GCE, "", "", false, false) +>>>>>>> 0df3a03e781 (roachtest: require perf. tests to opt in via TestSpec.Benchmark) for _, c := range testCases { clusterSpec := reg.MakeClusterSpec(1) diff --git a/pkg/cmd/roachtest/main.go b/pkg/cmd/roachtest/main.go index 8694704fc7c3..9e03a453e81e 100644 --- a/pkg/cmd/roachtest/main.go +++ b/pkg/cmd/roachtest/main.go @@ -173,17 +173,13 @@ Examples: roachtest list tag:weekly `, RunE: func(_ *cobra.Command, args []string) error { - r, err := makeTestRegistry(cloud, instanceType, zonesF, localSSDArg) + r, err := makeTestRegistry(cloud, instanceType, zonesF, localSSDArg, listBench) if err != nil { return err } - if !listBench { - tests.RegisterTests(&r) - } else { - tests.RegisterBenchmarks(&r) - } + tests.RegisterTests(&r) - matchedTests := r.List(context.Background(), args) + matchedTests := r.List(args) for _, test := range matchedTests { var skip string if test.Skip != "" { @@ -228,7 +224,8 @@ runner itself. user: username, clusterID: clusterID, versionsBinaryOverride: versionsBinaryOverride, - }) + enableFIPS: enableFIPS, + }, false /* benchOnly */) }, } @@ -255,7 +252,7 @@ runner itself. if literalArtifacts == "" { literalArtifacts = artifacts } - return runTests(tests.RegisterBenchmarks, cliCfg{ + return runTests(tests.RegisterTests, cliCfg{ args: args, count: count, cpuQuota: cpuQuota, @@ -266,7 +263,8 @@ runner itself. user: username, clusterID: clusterID, versionsBinaryOverride: versionsBinaryOverride, - }) + enableFIPS: enableFIPS, + }, true /* benchOnly */) }, } @@ -360,14 +358,17 @@ type cliCfg struct { versionsBinaryOverride map[string]string } -func runTests(register func(registry.Registry), cfg cliCfg) error { +func runTests(register func(registry.Registry), cfg cliCfg, benchOnly bool) error { if cfg.count <= 0 { return fmt.Errorf("--count (%d) must by greater than 0", cfg.count) } - r, err := makeTestRegistry(cloud, instanceType, zonesF, localSSDArg) + r, err := makeTestRegistry(cloud, instanceType, zonesF, localSSDArg, benchOnly) if err != nil { return err } + + // actual registering of tests + // TODO: don't register if we can't run on the specified registry cloud register(&r) cr := newClusterRegistry() stopper := stop.NewStopper() @@ -403,7 +404,7 @@ func runTests(register func(registry.Registry), cfg cliCfg) error { return err } - tests := testsToRun(context.Background(), r, filter) + tests := testsToRun(r, filter) n := len(tests) if n*cfg.count < cfg.parallelism { // Don't spin up more workers than necessary. This has particular @@ -535,10 +536,8 @@ func testRunnerLogger( return l, teeOpt } -func testsToRun( - ctx context.Context, r testRegistryImpl, filter *registry.TestFilter, -) []registry.TestSpec { - tests := r.GetTests(ctx, filter) +func testsToRun(r testRegistryImpl, filter *registry.TestFilter) []registry.TestSpec { + tests := r.GetTests(filter) var notSkipped []registry.TestSpec for _, s := range tests { diff --git a/pkg/cmd/roachtest/registry/test_spec.go b/pkg/cmd/roachtest/registry/test_spec.go index 8b5aa6aef939..46e51da56a1c 100644 --- a/pkg/cmd/roachtest/registry/test_spec.go +++ b/pkg/cmd/roachtest/registry/test_spec.go @@ -40,6 +40,14 @@ type TestSpec struct { // associated cluster expires. The timeout is always truncated to 10m before // the test's cluster expires. Timeout time.Duration + // Denotes whether the test is a roachperf benchmark. If true, the test is expected, but not required, to produce + // artifacts in Test.PerfArtifactsDir(), which get exported to the roachperf dashboard + // (see getPerfArtifacts() in test_runner.go). + // N.B. performance tests may choose to _assert_ on latency, throughput, or some other perf. metric, _without_ + // exporting artifacts to the dashboard. E.g., see 'registerKVContention' and 'verifyTxnPerSecond'. + // N.B. performance tests may have different requirements than correctness tests, e.g., machine type/architecture. + // Thus, they must be opted into explicitly via this field. + Benchmark bool // Tags is a set of tags associated with the test that allow grouping // tests. If no tags are specified, the set ["default"] is automatically // given. diff --git a/pkg/cmd/roachtest/test_registry.go b/pkg/cmd/roachtest/test_registry.go index 495addedeaf7..0f6f2e305367 100644 --- a/pkg/cmd/roachtest/test_registry.go +++ b/pkg/cmd/roachtest/test_registry.go @@ -11,7 +11,6 @@ package main import ( - "context" "fmt" "os" "os/exec" @@ -43,11 +42,13 @@ type testRegistryImpl struct { preferSSD bool // buildVersion is the version of the Cockroach binary that tests will run against. buildVersion version.Version + + benchOnly bool } // makeTestRegistry constructs a testRegistryImpl and configures it with opts. func makeTestRegistry( - cloud string, instanceType string, zones string, preferSSD bool, + cloud string, instanceType string, zones string, preferSSD bool, benchOnly bool, ) (testRegistryImpl, error) { r := testRegistryImpl{ cloud: cloud, @@ -55,6 +56,7 @@ func makeTestRegistry( zones: zones, preferSSD: preferSSD, m: make(map[string]*registry.TestSpec), + benchOnly: benchOnly, } v := buildTag if v == "" { @@ -76,6 +78,11 @@ func (r *testRegistryImpl) Add(spec registry.TestSpec) { fmt.Fprintf(os.Stderr, "test %s already registered\n", spec.Name) os.Exit(1) } + if r.benchOnly && !spec.Benchmark { + // Skip non-benchmarks. + return + } + if err := r.prepareSpec(&spec); err != nil { fmt.Fprintf(os.Stderr, "%+v\n", err) os.Exit(1) @@ -158,9 +165,8 @@ func (r *testRegistryImpl) prepareSpec(spec *registry.TestSpec) error { // GetTests returns all the tests that match the given regexp. // Skipped tests are included, and tests that don't match their minVersion spec // are also included but marked as skipped. -func (r testRegistryImpl) GetTests( - ctx context.Context, filter *registry.TestFilter, -) []registry.TestSpec { +func (r testRegistryImpl) GetTests(filter *registry.TestFilter) []registry.TestSpec { + var tests []registry.TestSpec for _, t := range r.m { if !t.MatchOrSkip(filter) { @@ -175,9 +181,10 @@ func (r testRegistryImpl) GetTests( } // List lists tests that match one of the filters. -func (r testRegistryImpl) List(ctx context.Context, filters []string) []registry.TestSpec { +func (r testRegistryImpl) List(filters []string) []registry.TestSpec { filter := registry.NewTestFilter(filters) - tests := r.GetTests(ctx, filter) + tests := r.GetTests(filter) + sort.Slice(tests, func(i, j int) bool { return tests[i].Name < tests[j].Name }) return tests } diff --git a/pkg/cmd/roachtest/test_registry_test.go b/pkg/cmd/roachtest/test_registry_test.go index 01ea18cec466..9ab9c2780537 100644 --- a/pkg/cmd/roachtest/test_registry_test.go +++ b/pkg/cmd/roachtest/test_registry_test.go @@ -20,8 +20,9 @@ import ( func TestMakeTestRegistry(t *testing.T) { testutils.RunTrueAndFalse(t, "preferSSD", func(t *testing.T, preferSSD bool) { - r, err := makeTestRegistry(spec.AWS, "foo", "zone123", preferSSD) + r, err := makeTestRegistry(spec.AWS, "foo", "zone123", preferSSD, false) require.NoError(t, err) + require.Equal(t, preferSSD, r.preferSSD) require.Equal(t, "zone123", r.zones) require.Equal(t, "foo", r.instanceType) @@ -41,5 +42,4 @@ func TestMakeTestRegistry(t *testing.T) { require.EqualValues(t, 4, s.CPUs) require.True(t, s.TerminateOnMigration) }) - } diff --git a/pkg/cmd/roachtest/test_runner.go b/pkg/cmd/roachtest/test_runner.go index 9cfa0339a984..7160cd4b9ec3 100644 --- a/pkg/cmd/roachtest/test_runner.go +++ b/pkg/cmd/roachtest/test_runner.go @@ -704,7 +704,9 @@ func (r *testRunner) runWorker( } } else { // Upon success fetch the perf artifacts from the remote hosts. - getPerfArtifacts(ctx, l, c, t) + if t.spec.Benchmark { + getPerfArtifacts(ctx, l, c, t) + } } } } diff --git a/pkg/cmd/roachtest/test_test.go b/pkg/cmd/roachtest/test_test.go index b61659325479..b42236a7dbd8 100644 --- a/pkg/cmd/roachtest/test_test.go +++ b/pkg/cmd/roachtest/test_test.go @@ -45,7 +45,8 @@ const defaultParallelism = 10 func mkReg(t *testing.T) testRegistryImpl { t.Helper() - r, err := makeTestRegistry(spec.GCE, "", "", false /* preferSSD */) + + r, err := makeTestRegistry(spec.GCE, "", "", false /* preferSSD */, false /* benchOnly */) require.NoError(t, err) return r } @@ -244,8 +245,7 @@ type runnerTest struct { func setupRunnerTest(t *testing.T, r testRegistryImpl, testFilters []string) *runnerTest { ctx := context.Background() - - tests := testsToRun(ctx, r, registry.NewTestFilter(testFilters)) + tests := testsToRun(r, registry.NewTestFilter(testFilters)) cr := newClusterRegistry() stopper := stop.NewStopper() @@ -402,11 +402,12 @@ func TestRegistryPrepareSpec(t *testing.T) { } for _, c := range testCases { t.Run("", func(t *testing.T) { - r, err := makeTestRegistry(spec.GCE, "", "", false /* preferSSD */) + r, err := makeTestRegistry(spec.GCE, "", "", false /* preferSSD */, false /* benchOnly */) if err != nil { t.Fatal(err) } err = r.prepareSpec(&c.spec) + if !testutils.IsError(err, c.expectedErr) { t.Fatalf("expected %q, but found %q", c.expectedErr, err.Error()) } @@ -439,7 +440,8 @@ func runExitCodeTest(t *testing.T, injectedError error) error { } }, }) - tests := testsToRun(ctx, r, registry.NewTestFilter(nil)) + + tests := testsToRun(r, registry.NewTestFilter(nil)) lopt := loggingOpt{ l: nilLogger(), tee: logger.NoTee, diff --git a/pkg/cmd/roachtest/tests/allocator.go b/pkg/cmd/roachtest/tests/allocator.go index 5dc0fca8954d..1ee2972e3d12 100644 --- a/pkg/cmd/roachtest/tests/allocator.go +++ b/pkg/cmd/roachtest/tests/allocator.go @@ -145,27 +145,32 @@ func registerAllocator(r registry.Registry) { } r.Add(registry.TestSpec{ - Name: `replicate/up/1to3`, - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(4), + Name: `replicate/up/1to3`, + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec(4), + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runAllocator(ctx, t, c, 1, 10.0) }, }) r.Add(registry.TestSpec{ - Name: `replicate/rebalance/3to5`, - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(6), + Name: `replicate/rebalance/3to5`, + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec(6), + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runAllocator(ctx, t, c, 3, 42.0) }, }) r.Add(registry.TestSpec{ - Name: `replicate/wide`, - Owner: registry.OwnerKV, - Timeout: 10 * time.Minute, - Cluster: r.MakeClusterSpec(9, spec.CPU(1)), - Run: runWideReplication, + Name: `replicate/wide`, + Owner: registry.OwnerKV, + Benchmark: true, + Timeout: 10 * time.Minute, + Cluster: r.MakeClusterSpec(9, spec.CPU(1)), + Run: runWideReplication, }) } diff --git a/pkg/cmd/roachtest/tests/backup.go b/pkg/cmd/roachtest/tests/backup.go index 504e87127708..ea5eeb5e7b68 100644 --- a/pkg/cmd/roachtest/tests/backup.go +++ b/pkg/cmd/roachtest/tests/backup.go @@ -617,6 +617,7 @@ func registerBackup(r registry.Registry) { r.Add(registry.TestSpec{ Name: fmt.Sprintf("backup/2TB/%s", backup2TBSpec), Owner: registry.OwnerDisasterRecovery, + Benchmark: true, Cluster: backup2TBSpec, EncryptionSupport: registry.EncryptionMetamorphic, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { diff --git a/pkg/cmd/roachtest/tests/connection_latency.go b/pkg/cmd/roachtest/tests/connection_latency.go index 44e64ff8bab6..558a494c3397 100644 --- a/pkg/cmd/roachtest/tests/connection_latency.go +++ b/pkg/cmd/roachtest/tests/connection_latency.go @@ -117,8 +117,9 @@ func registerConnectionLatencyTest(r registry.Registry) { // Single region test. numNodes := 3 r.Add(registry.TestSpec{ - Name: fmt.Sprintf("connection_latency/nodes=%d/certs", numNodes), - Owner: registry.OwnerSQLFoundations, + Name: fmt.Sprintf("connection_latency/nodes=%d/certs", numNodes), + Owner: registry.OwnerSQLFoundations, + Benchmark: true, // Add one more node for load node. Cluster: r.MakeClusterSpec(numNodes+1, spec.Zones(regionUsCentral)), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { @@ -133,18 +134,20 @@ func registerConnectionLatencyTest(r registry.Registry) { loadNodes := numZones r.Add(registry.TestSpec{ - Name: fmt.Sprintf("connection_latency/nodes=%d/multiregion/certs", numMultiRegionNodes), - Owner: registry.OwnerSQLFoundations, - Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.Zones(geoZonesStr)), + Name: fmt.Sprintf("connection_latency/nodes=%d/multiregion/certs", numMultiRegionNodes), + Owner: registry.OwnerSQLFoundations, + Benchmark: true, + Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.Zones(geoZonesStr)), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runConnectionLatencyTest(ctx, t, c, numMultiRegionNodes, numZones, false /*password*/) }, }) r.Add(registry.TestSpec{ - Name: fmt.Sprintf("connection_latency/nodes=%d/multiregion/password", numMultiRegionNodes), - Owner: registry.OwnerSQLFoundations, - Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.Zones(geoZonesStr)), + Name: fmt.Sprintf("connection_latency/nodes=%d/multiregion/password", numMultiRegionNodes), + Owner: registry.OwnerSQLFoundations, + Benchmark: true, + Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.Zones(geoZonesStr)), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runConnectionLatencyTest(ctx, t, c, numMultiRegionNodes, numZones, true /*password*/) }, diff --git a/pkg/cmd/roachtest/tests/copyfrom.go b/pkg/cmd/roachtest/tests/copyfrom.go index 64e1ad0608fe..d68806b995a6 100644 --- a/pkg/cmd/roachtest/tests/copyfrom.go +++ b/pkg/cmd/roachtest/tests/copyfrom.go @@ -155,25 +155,31 @@ func registerCopyFrom(r registry.Registry) { for _, tc := range testcases { tc := tc r.Add(registry.TestSpec{ - Name: fmt.Sprintf("copyfrom/crdb-atomic/sf=%d/nodes=%d", tc.sf, tc.nodes), - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(tc.nodes), + Name: fmt.Sprintf("copyfrom/crdb-atomic/sf=%d/nodes=%d", tc.sf, tc.nodes), + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec(tc.nodes), + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runCopyFromCRDB(ctx, t, c, tc.sf, true /*atomic*/) }, }) r.Add(registry.TestSpec{ - Name: fmt.Sprintf("copyfrom/crdb-nonatomic/sf=%d/nodes=%d", tc.sf, tc.nodes), - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(tc.nodes), + Name: fmt.Sprintf("copyfrom/crdb-nonatomic/sf=%d/nodes=%d", tc.sf, tc.nodes), + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec(tc.nodes), + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runCopyFromCRDB(ctx, t, c, tc.sf, false /*atomic*/) }, }) r.Add(registry.TestSpec{ - Name: fmt.Sprintf("copyfrom/pg/sf=%d/nodes=%d", tc.sf, tc.nodes), - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(tc.nodes), + Name: fmt.Sprintf("copyfrom/pg/sf=%d/nodes=%d", tc.sf, tc.nodes), + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec(tc.nodes), + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runCopyFromPG(ctx, t, c, tc.sf) }, diff --git a/pkg/cmd/roachtest/tests/decommissionbench.go b/pkg/cmd/roachtest/tests/decommissionbench.go index 839f045528c5..58b5d5b5400b 100644 --- a/pkg/cmd/roachtest/tests/decommissionbench.go +++ b/pkg/cmd/roachtest/tests/decommissionbench.go @@ -288,7 +288,9 @@ func registerDecommissionBenchSpec(r registry.Registry, benchSpec decommissionBe r.Add(registry.TestSpec{ Name: fmt.Sprintf("decommissionBench/nodes=%d/cpu=%d/warehouses=%d%s", benchSpec.nodes, benchSpec.cpus, benchSpec.warehouses, extraName), - Owner: registry.OwnerKV, + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec( benchSpec.nodes+addlNodeCount+1, specOptions..., diff --git a/pkg/cmd/roachtest/tests/import.go b/pkg/cmd/roachtest/tests/import.go index 45f4b219d43b..89e006e30589 100644 --- a/pkg/cmd/roachtest/tests/import.go +++ b/pkg/cmd/roachtest/tests/import.go @@ -162,6 +162,7 @@ func registerImportTPCC(r registry.Registry) { r.Add(registry.TestSpec{ Name: testName, Owner: registry.OwnerDisasterRecovery, + Benchmark: true, Cluster: r.MakeClusterSpec(numNodes), Timeout: timeout, EncryptionSupport: registry.EncryptionMetamorphic, @@ -207,6 +208,7 @@ func registerImportTPCH(r registry.Registry) { r.Add(registry.TestSpec{ Name: fmt.Sprintf(`import/tpch/nodes=%d`, item.nodes), Owner: registry.OwnerDisasterRecovery, + Benchmark: true, Cluster: r.MakeClusterSpec(item.nodes), Timeout: item.timeout, EncryptionSupport: registry.EncryptionMetamorphic, diff --git a/pkg/cmd/roachtest/tests/import_cancellation.go b/pkg/cmd/roachtest/tests/import_cancellation.go index f854b144a83b..fa2d8813a1e3 100644 --- a/pkg/cmd/roachtest/tests/import_cancellation.go +++ b/pkg/cmd/roachtest/tests/import_cancellation.go @@ -31,10 +31,11 @@ import ( func registerImportCancellation(r registry.Registry) { for _, rangeTombstones := range []bool{true, false} { r.Add(registry.TestSpec{ - Name: fmt.Sprintf(`import-cancellation/rangeTs=%t`, rangeTombstones), - Owner: registry.OwnerDisasterRecovery, - Timeout: 4 * time.Hour, - Cluster: r.MakeClusterSpec(6, spec.CPU(32)), + Name: fmt.Sprintf(`import-cancellation/rangeTs=%t`, rangeTombstones), + Owner: registry.OwnerDisasterRecovery, + Benchmark: true, + Timeout: 4 * time.Hour, + Cluster: r.MakeClusterSpec(6, spec.CPU(32)), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runImportCancellation(ctx, t, c, rangeTombstones) }, diff --git a/pkg/cmd/roachtest/tests/indexes.go b/pkg/cmd/roachtest/tests/indexes.go index 3e46b1033aa8..cb2cdff6124b 100644 --- a/pkg/cmd/roachtest/tests/indexes.go +++ b/pkg/cmd/roachtest/tests/indexes.go @@ -34,9 +34,10 @@ func registerNIndexes(r registry.Registry, secondaryIndexes int) { } geoZonesStr := strings.Join(geoZones, ",") r.Add(registry.TestSpec{ - Name: fmt.Sprintf("indexes/%d/nodes=%d/multi-region", secondaryIndexes, nodes), - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(nodes+1, spec.CPU(16), spec.Geo(), spec.Zones(geoZonesStr)), + Name: fmt.Sprintf("indexes/%d/nodes=%d/multi-region", secondaryIndexes, nodes), + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec(nodes+1, spec.CPU(16), spec.Geo(), spec.Zones(geoZonesStr)), // Uses CONFIGURE ZONE USING ... COPY FROM PARENT syntax. Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { firstAZ := geoZones[0] diff --git a/pkg/cmd/roachtest/tests/kv.go b/pkg/cmd/roachtest/tests/kv.go index 22c00b266400..6998639bc223 100644 --- a/pkg/cmd/roachtest/tests/kv.go +++ b/pkg/cmd/roachtest/tests/kv.go @@ -297,10 +297,11 @@ func registerKV(r registry.Registry) { skip = fmt.Sprintf("multi-store tests are not supported on cloud %s", cSpec.Cloud) } r.Add(registry.TestSpec{ - Skip: skip, - Name: strings.Join(nameParts, "/"), - Owner: owner, - Cluster: cSpec, + Skip: skip, + Name: strings.Join(nameParts, "/"), + Owner: owner, + Benchmark: true, + Cluster: cSpec, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runKV(ctx, t, c, opts) }, @@ -313,9 +314,11 @@ func registerKV(r registry.Registry) { func registerKVContention(r registry.Registry) { const nodes = 4 r.Add(registry.TestSpec{ - Name: fmt.Sprintf("kv/contention/nodes=%d", nodes), - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(nodes + 1), + Name: fmt.Sprintf("kv/contention/nodes=%d", nodes), + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec(nodes + 1), + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { c.Put(ctx, t.Cockroach(), "./cockroach", c.Range(1, nodes)) c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.Node(nodes+1)) diff --git a/pkg/cmd/roachtest/tests/ledger.go b/pkg/cmd/roachtest/tests/ledger.go index b9a7840dbc9a..f001126eb07b 100644 --- a/pkg/cmd/roachtest/tests/ledger.go +++ b/pkg/cmd/roachtest/tests/ledger.go @@ -28,9 +28,10 @@ func registerLedger(r registry.Registry) { // https://github.com/cockroachdb/cockroach/issues/66184 const azs = "us-central1-f,us-central1-b,us-central1-c" r.Add(registry.TestSpec{ - Name: fmt.Sprintf("ledger/nodes=%d/multi-az", nodes), - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(nodes+1, spec.CPU(16), spec.Geo(), spec.Zones(azs)), + Name: fmt.Sprintf("ledger/nodes=%d/multi-az", nodes), + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec(nodes+1, spec.CPU(16), spec.Geo(), spec.Zones(azs)), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { roachNodes := c.Range(1, nodes) gatewayNodes := c.Range(1, nodes/3) diff --git a/pkg/cmd/roachtest/tests/loss_of_quorum_recovery.go b/pkg/cmd/roachtest/tests/loss_of_quorum_recovery.go index 75571a1fdd30..9748c3bddfc4 100644 --- a/pkg/cmd/roachtest/tests/loss_of_quorum_recovery.go +++ b/pkg/cmd/roachtest/tests/loss_of_quorum_recovery.go @@ -63,9 +63,11 @@ func registerLOQRecovery(r registry.Registry) { r.Add(registry.TestSpec{ Name: s.String(), Owner: registry.OwnerReplication, + Benchmark: true, Tags: []string{`default`}, Cluster: spec, NonReleaseBlocker: true, + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runRecoverLossOfQuorum(ctx, t, c, testSpec) }, diff --git a/pkg/cmd/roachtest/tests/registry.go b/pkg/cmd/roachtest/tests/registry.go index b006104ff3d5..80cc4171cc6b 100644 --- a/pkg/cmd/roachtest/tests/registry.go +++ b/pkg/cmd/roachtest/tests/registry.go @@ -137,14 +137,3 @@ func RegisterTests(r registry.Registry) { registerYCSB(r) registerDeclarativeSchemaChangerJobCompatibilityInMixedVersion(r) } - -// RegisterBenchmarks registers all benchmarks to the registry. This powers `roachtest bench`. -// -// TODO(tbg): it's unclear that `roachtest bench` is that useful, perhaps we make everything -// a roachtest but use a `bench` tag to determine what tests to understand as benchmarks. -func RegisterBenchmarks(r registry.Registry) { - registerIndexesBench(r) - registerTPCCBench(r) - registerKVBench(r) - registerTPCHBench(r) -} diff --git a/pkg/cmd/roachtest/tests/restore.go b/pkg/cmd/roachtest/tests/restore.go index 2bf7d4cae86e..471902cf433b 100644 --- a/pkg/cmd/roachtest/tests/restore.go +++ b/pkg/cmd/roachtest/tests/restore.go @@ -646,10 +646,12 @@ func registerRestore(r registry.Registry) { withPauseTestName := fmt.Sprintf("restore%s/nodes=%d/with-pause", withPauseDataset.name(), 10) withPauseTimeout := 3 * time.Hour r.Add(registry.TestSpec{ - Name: withPauseTestName, - Owner: registry.OwnerDisasterRecovery, - Cluster: r.MakeClusterSpec(10), - Timeout: withPauseTimeout, + Name: withPauseTestName, + Owner: registry.OwnerDisasterRecovery, + Cluster: r.MakeClusterSpec(10), + Benchmark: true, + Timeout: withPauseTimeout, + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { c.Put(ctx, t.Cockroach(), "./cockroach") c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings()) diff --git a/pkg/cmd/roachtest/tests/schemachange.go b/pkg/cmd/roachtest/tests/schemachange.go index 2667f915b664..1043ab294e8d 100644 --- a/pkg/cmd/roachtest/tests/schemachange.go +++ b/pkg/cmd/roachtest/tests/schemachange.go @@ -305,10 +305,12 @@ func makeIndexAddTpccTest( spec spec.ClusterSpec, warehouses int, length time.Duration, ) registry.TestSpec { return registry.TestSpec{ - Name: fmt.Sprintf("schemachange/index/tpcc/w=%d", warehouses), - Owner: registry.OwnerSQLFoundations, - Cluster: spec, - Timeout: length * 3, + Name: fmt.Sprintf("schemachange/index/tpcc/w=%d", warehouses), + Owner: registry.OwnerSQLFoundations, + Benchmark: true, + Cluster: spec, + Timeout: length * 3, + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runTPCC(ctx, t, c, tpccOptions{ Warehouses: warehouses, @@ -423,10 +425,12 @@ func makeSchemaChangeDuringTPCC( spec spec.ClusterSpec, warehouses int, length time.Duration, ) registry.TestSpec { return registry.TestSpec{ - Name: "schemachange/during/tpcc", - Owner: registry.OwnerSQLFoundations, - Cluster: spec, - Timeout: length * 3, + Name: "schemachange/during/tpcc", + Owner: registry.OwnerSQLFoundations, + Benchmark: true, + Cluster: spec, + Timeout: length * 3, + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runTPCC(ctx, t, c, tpccOptions{ Warehouses: warehouses, diff --git a/pkg/cmd/roachtest/tests/schemachange_random_load.go b/pkg/cmd/roachtest/tests/schemachange_random_load.go index c4fc4fa6a0da..e7614616463b 100644 --- a/pkg/cmd/roachtest/tests/schemachange_random_load.go +++ b/pkg/cmd/roachtest/tests/schemachange_random_load.go @@ -38,8 +38,9 @@ func registerSchemaChangeRandomLoad(r registry.Registry) { } geoZonesStr := strings.Join(geoZones, ",") r.Add(registry.TestSpec{ - Name: "schemachange/random-load", - Owner: registry.OwnerSQLFoundations, + Name: "schemachange/random-load", + Owner: registry.OwnerSQLFoundations, + Benchmark: true, Cluster: r.MakeClusterSpec( 3, spec.Geo(), @@ -84,6 +85,7 @@ func registerRandomLoadBenchSpec(r registry.Registry, b randomLoadBenchSpec) { r.Add(registry.TestSpec{ Name: name, Owner: registry.OwnerSQLFoundations, + Benchmark: true, Cluster: r.MakeClusterSpec(b.Nodes), NativeLibs: registry.LibGEOS, Skip: "https://github.com/cockroachdb/cockroach/issues/56230", diff --git a/pkg/cmd/roachtest/tests/scrub.go b/pkg/cmd/roachtest/tests/scrub.go index 8def4d759283..a2524090a05a 100644 --- a/pkg/cmd/roachtest/tests/scrub.go +++ b/pkg/cmd/roachtest/tests/scrub.go @@ -54,9 +54,11 @@ func makeScrubTPCCTest( } return registry.TestSpec{ - Name: fmt.Sprintf("scrub/%s/tpcc/w=%d", optionName, warehouses), - Owner: registry.OwnerSQLQueries, - Cluster: r.MakeClusterSpec(numNodes), + Name: fmt.Sprintf("scrub/%s/tpcc/w=%d", optionName, warehouses), + Owner: registry.OwnerSQLQueries, + Benchmark: true, + Cluster: r.MakeClusterSpec(numNodes), + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runTPCC(ctx, t, c, tpccOptions{ Warehouses: warehouses, diff --git a/pkg/cmd/roachtest/tests/tpcc.go b/pkg/cmd/roachtest/tests/tpcc.go index 3be014117110..c22666379bcc 100644 --- a/pkg/cmd/roachtest/tests/tpcc.go +++ b/pkg/cmd/roachtest/tests/tpcc.go @@ -1022,13 +1022,15 @@ func registerTPCCBenchSpec(r registry.Registry, b tpccBenchSpec) { } r.Add(registry.TestSpec{ - Name: name, - Owner: owner, - Cluster: nodes, - Tags: b.Tags, + Name: name, + Owner: owner, + Benchmark: true, + Cluster: nodes, + Tags: b.Tags, // NB: intentionally not enabling encryption-at-rest to produce // consistent results. EncryptionSupport: registry.EncryptionAlwaysDisabled, + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runTPCCBench(ctx, t, c, b) }, diff --git a/pkg/cmd/roachtest/tests/tpch_concurrency.go b/pkg/cmd/roachtest/tests/tpch_concurrency.go index bab42ed9ef81..974cf9b9c55e 100644 --- a/pkg/cmd/roachtest/tests/tpch_concurrency.go +++ b/pkg/cmd/roachtest/tests/tpch_concurrency.go @@ -200,7 +200,9 @@ func registerTPCHConcurrency(r registry.Registry) { r.Add(registry.TestSpec{ Name: "tpch_concurrency", Owner: registry.OwnerSQLQueries, + Benchmark: true, Cluster: r.MakeClusterSpec(numNodes), + Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runTPCHConcurrency(ctx, t, c, true /* lowerRefreshSpansBytes */, false /* disableStreamer */) }, @@ -230,9 +232,17 @@ func registerTPCHConcurrency(r registry.Registry) { // TODO(yuzefovich): remove this once the streamer is stabilized. r.Add(registry.TestSpec{ +<<<<<<< HEAD Name: "tpch_concurrency/no_streamer", Owner: registry.OwnerSQLQueries, Cluster: r.MakeClusterSpec(numNodes), +======= + Name: "tpch_concurrency/no_streamer", + Owner: registry.OwnerSQLQueries, + Benchmark: true, + Timeout: timeout, + Cluster: r.MakeClusterSpec(numNodes), +>>>>>>> 0df3a03e781 (roachtest: require perf. tests to opt in via TestSpec.Benchmark) Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runTPCHConcurrency(ctx, t, c, true /* lowerRefreshSpansBytes */, true /* disableStreamer */) }, diff --git a/pkg/cmd/roachtest/tests/tpchbench.go b/pkg/cmd/roachtest/tests/tpchbench.go index 001169c8e4d8..db8e502a541b 100644 --- a/pkg/cmd/roachtest/tests/tpchbench.go +++ b/pkg/cmd/roachtest/tests/tpchbench.go @@ -172,9 +172,10 @@ func registerTPCHBenchSpec(r registry.Registry, b tpchBenchSpec) { } r.Add(registry.TestSpec{ - Name: strings.Join(nameParts, "/"), - Owner: registry.OwnerSQLQueries, - Cluster: r.MakeClusterSpec(numNodes), + Name: strings.Join(nameParts, "/"), + Owner: registry.OwnerSQLQueries, + Benchmark: true, + Cluster: r.MakeClusterSpec(numNodes), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runTPCHBench(ctx, t, c, b) }, diff --git a/pkg/cmd/roachtest/tests/ycsb.go b/pkg/cmd/roachtest/tests/ycsb.go index 5bc1fa4c8b0e..96f9f4b563b6 100644 --- a/pkg/cmd/roachtest/tests/ycsb.go +++ b/pkg/cmd/roachtest/tests/ycsb.go @@ -102,9 +102,10 @@ func registerYCSB(r registry.Registry) { } wl, cpus := wl, cpus r.Add(registry.TestSpec{ - Name: name, - Owner: registry.OwnerTestEng, - Cluster: r.MakeClusterSpec(4, spec.CPU(cpus)), + Name: name, + Owner: registry.OwnerTestEng, + Benchmark: true, + Cluster: r.MakeClusterSpec(4, spec.CPU(cpus)), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runYCSB(ctx, t, c, wl, cpus, false /* rangeTombstone */) }, @@ -112,9 +113,10 @@ func registerYCSB(r registry.Registry) { if wl == "A" { r.Add(registry.TestSpec{ - Name: fmt.Sprintf("zfs/ycsb/%s/nodes=3/cpu=%d", wl, cpus), - Owner: registry.OwnerStorage, - Cluster: r.MakeClusterSpec(4, spec.CPU(cpus), spec.SetFileSystem(spec.Zfs)), + Name: fmt.Sprintf("zfs/ycsb/%s/nodes=3/cpu=%d", wl, cpus), + Owner: registry.OwnerStorage, + Benchmark: true, + Cluster: r.MakeClusterSpec(4, spec.CPU(cpus), spec.SetFileSystem(spec.Zfs)), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runYCSB(ctx, t, c, wl, cpus, false /* rangeTombstone */) }, @@ -123,9 +125,10 @@ func registerYCSB(r registry.Registry) { if cpus == cpusWithGlobalMVCCRangeTombstone { r.Add(registry.TestSpec{ - Name: fmt.Sprintf("%s/mvcc-range-keys=global", name), - Owner: registry.OwnerTestEng, - Cluster: r.MakeClusterSpec(4, spec.CPU(cpus)), + Name: fmt.Sprintf("%s/mvcc-range-keys=global", name), + Owner: registry.OwnerTestEng, + Benchmark: true, + Cluster: r.MakeClusterSpec(4, spec.CPU(cpus)), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runYCSB(ctx, t, c, wl, cpus, true /* rangeTombstone */) },