From d8695ae28f305e67ac42804c08ebca6595f6ce0a Mon Sep 17 00:00:00 2001 From: DarrylWong <darryl@cockroachlabs.com> Date: Mon, 1 Jul 2024 10:31:46 -0400 Subject: [PATCH 1/2] roachtest: add RequiresDeprecatedWorkload testSpec Previously we required the workload binary to be present for all tests to run even if the test did not use it. Now that we have removed all uncessary usage of the deprecated workload, we can switch the validation of the workload binary to be opt in. Co-authored-by: dt <david@cockroachlabs.com> Fixes: none Epic: none Release note: none --- pkg/cmd/roachtest/cluster.go | 3 +- pkg/cmd/roachtest/registry/test_spec.go | 9 +++ pkg/cmd/roachtest/test_impl.go | 6 ++ pkg/cmd/roachtest/test_runner.go | 5 ++ pkg/cmd/roachtest/tests/acceptance.go | 61 ++++++++++--------- .../tests/admission_control_index_overload.go | 2 +- .../tests/admission_control_tpcc_overload.go | 21 ++++--- .../tests/backup_restore_roundtrip.go | 11 ++-- pkg/cmd/roachtest/tests/cdc.go | 13 ++-- pkg/cmd/roachtest/tests/connection_latency.go | 33 +++++----- pkg/cmd/roachtest/tests/indexes.go | 5 +- pkg/cmd/roachtest/tests/inverted_index.go | 13 ++-- pkg/cmd/roachtest/tests/ledger.go | 13 ++-- .../tests/mixed_version_schemachange.go | 15 ++--- pkg/cmd/roachtest/tests/queue.go | 15 ++--- pkg/cmd/roachtest/tests/roachmart.go | 13 ++-- .../tests/schemachange_random_load.go | 9 +-- pkg/cmd/roachtest/tests/tpchbench.go | 7 ++- 18 files changed, 146 insertions(+), 108 deletions(-) diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go index d53d99e56d20..dfcf879918f9 100644 --- a/pkg/cmd/roachtest/cluster.go +++ b/pkg/cmd/roachtest/cluster.go @@ -324,7 +324,8 @@ func initBinariesAndLibraries() { cockroachEAPath := roachtestflags.CockroachEAPath workloadPath := roachtestflags.WorkloadPath cockroach[defaultArch], _ = resolveBinary("cockroach", cockroachPath, defaultArch, true, false) - workload[defaultArch], _ = resolveBinary("workload", workloadPath, defaultArch, true, false) + // Let the test runner verify the workload binary exists if TestSpec.RequiresDeprecatedWorkload is true. + workload[defaultArch], _ = resolveBinary("workload", workloadPath, defaultArch, false, false) cockroachEA[defaultArch], err = resolveBinary("cockroach-ea", cockroachEAPath, defaultArch, false, true) if err != nil { fmt.Fprintf(os.Stderr, "WARN: unable to find %q for %q: %s\n", "cockroach-ea", defaultArch, err) diff --git a/pkg/cmd/roachtest/registry/test_spec.go b/pkg/cmd/roachtest/registry/test_spec.go index 90ca753ae957..f4fa151043b5 100644 --- a/pkg/cmd/roachtest/registry/test_spec.go +++ b/pkg/cmd/roachtest/registry/test_spec.go @@ -105,6 +105,15 @@ type TestSpec struct { // in the environment. RequiresLicense bool + // RequiresDeprecatedWorkload indicates that the test requires + // the 'workload' binary to be present for the test to run. Use + // this to ensure tests will fail-early if a 'workload' binary + // does not exist. + // + // N.B. The workload binary is deprecated, use 'cockroach workload' + // instead if the desired workload exists there. + RequiresDeprecatedWorkload bool + // EncryptionSupport encodes to what extent tests supports // encryption-at-rest. See the EncryptionSupport type for details. // Encryption support is opt-in -- i.e., if the TestSpec does not diff --git a/pkg/cmd/roachtest/test_impl.go b/pkg/cmd/roachtest/test_impl.go index bbe98fcf744e..ddbea779737b 100644 --- a/pkg/cmd/roachtest/test_impl.go +++ b/pkg/cmd/roachtest/test_impl.go @@ -195,6 +195,12 @@ func (t *testImpl) StandardCockroach() string { } func (t *testImpl) DeprecatedWorkload() string { + // Discourage usage of the deprecated workload by gating it behind the + // 'RequiresDeprecatedWorkload' test spec. Tests should use 'cockroach + // workload' instead when possible. + if !t.spec.RequiresDeprecatedWorkload { + t.Fatal("Using deprecated workload but `RequiresDeprecatedWorkload` is not set in test spec.") + } return t.deprecatedWorkload } diff --git a/pkg/cmd/roachtest/test_runner.go b/pkg/cmd/roachtest/test_runner.go index acc949a26212..c4d47d1be22f 100644 --- a/pkg/cmd/roachtest/test_runner.go +++ b/pkg/cmd/roachtest/test_runner.go @@ -730,6 +730,11 @@ func (r *testRunner) runWorker( return err } + // Verify that the deprecated workload is available if needed. + if testToRun.spec.RequiresDeprecatedWorkload && workload[arch] == "" { + return errors.Errorf("%s requires deprecated workload binary but one was not found", testToRun.spec.Name) + } + var clusterCreateErr error var vmCreateOpts *vm.CreateOpts diff --git a/pkg/cmd/roachtest/tests/acceptance.go b/pkg/cmd/roachtest/tests/acceptance.go index b71f7a43795e..77457da75f5b 100644 --- a/pkg/cmd/roachtest/tests/acceptance.go +++ b/pkg/cmd/roachtest/tests/acceptance.go @@ -26,19 +26,20 @@ func registerAcceptance(r registry.Registry) { cloudsWithoutServiceRegistration := registry.AllClouds.Remove(registry.CloudsWithServiceRegistration) testCases := map[registry.Owner][]struct { - name string - fn func(ctx context.Context, t test.Test, c cluster.Cluster) - skip string - numNodes int - nodeRegions []string - timeout time.Duration - encryptionSupport registry.EncryptionSupport - defaultLeases bool - requiresLicense bool - randomized bool - nativeLibs []string - workloadNode bool - incompatibleClouds registry.CloudSet + name string + fn func(ctx context.Context, t test.Test, c cluster.Cluster) + skip string + numNodes int + nodeRegions []string + timeout time.Duration + encryptionSupport registry.EncryptionSupport + defaultLeases bool + requiresLicense bool + randomized bool + nativeLibs []string + workloadNode bool + incompatibleClouds registry.CloudSet + requiresDeprecatedWorkload bool }{ // NOTE: acceptance tests are lightweight tests that run as part // of CI. As such, they must: @@ -77,12 +78,13 @@ func registerAcceptance(r registry.Registry) { }, registry.OwnerTestEng: { { - name: "version-upgrade", - fn: runVersionUpgrade, - timeout: 2 * time.Hour, // actually lower in local runs; see `runVersionUpgrade` - defaultLeases: true, - randomized: true, - nativeLibs: registry.LibGEOS, + name: "version-upgrade", + fn: runVersionUpgrade, + timeout: 2 * time.Hour, // actually lower in local runs; see `runVersionUpgrade` + defaultLeases: true, + randomized: true, + nativeLibs: registry.LibGEOS, + requiresDeprecatedWorkload: true, }, }, registry.OwnerDisasterRecovery: { @@ -146,16 +148,17 @@ func registerAcceptance(r registry.Registry) { } testSpec := registry.TestSpec{ - Name: "acceptance/" + tc.name, - Owner: owner, - Cluster: r.MakeClusterSpec(numNodes, extraOptions...), - Skip: tc.skip, - EncryptionSupport: tc.encryptionSupport, - Timeout: 10 * time.Minute, - CompatibleClouds: registry.AllClouds.Remove(tc.incompatibleClouds), - Suites: registry.Suites(registry.Nightly, registry.Quick, registry.Acceptance), - Randomized: tc.randomized, - RequiresLicense: tc.requiresLicense, + Name: "acceptance/" + tc.name, + Owner: owner, + Cluster: r.MakeClusterSpec(numNodes, extraOptions...), + Skip: tc.skip, + EncryptionSupport: tc.encryptionSupport, + Timeout: 10 * time.Minute, + CompatibleClouds: registry.AllClouds.Remove(tc.incompatibleClouds), + Suites: registry.Suites(registry.Nightly, registry.Quick, registry.Acceptance), + Randomized: tc.randomized, + RequiresLicense: tc.requiresLicense, + RequiresDeprecatedWorkload: tc.requiresDeprecatedWorkload, } if tc.timeout != 0 { diff --git a/pkg/cmd/roachtest/tests/admission_control_index_overload.go b/pkg/cmd/roachtest/tests/admission_control_index_overload.go index 6c5873e81f45..ca2967af831b 100644 --- a/pkg/cmd/roachtest/tests/admission_control_index_overload.go +++ b/pkg/cmd/roachtest/tests/admission_control_index_overload.go @@ -96,7 +96,7 @@ func registerIndexOverload(r registry.Registry) { m.Go(func(ctx context.Context) error { testDurationStr := " --duration=" + testDuration.String() concurrency := ifLocal(c, " --concurrency=8", " --concurrency=2048") - c.Run(ctx, option.WithNodes(c.CRDBNodes()), + c.Run(ctx, option.WithNodes(c.WorkloadNode()), "./cockroach workload run kv --read-percent=50 --max-rate=1000 --max-block-bytes=4096"+ testDurationStr+concurrency+fmt.Sprintf(" {pgurl%s}", c.CRDBNodes()), ) diff --git a/pkg/cmd/roachtest/tests/admission_control_tpcc_overload.go b/pkg/cmd/roachtest/tests/admission_control_tpcc_overload.go index c69100710730..385479c83bbe 100644 --- a/pkg/cmd/roachtest/tests/admission_control_tpcc_overload.go +++ b/pkg/cmd/roachtest/tests/admission_control_tpcc_overload.go @@ -163,16 +163,17 @@ func registerTPCCOverload(r registry.Registry) { name := fmt.Sprintf("admission-control/tpcc-olap/nodes=%d/cpu=%d/w=%d/c=%d", s.Nodes, s.CPUs, s.Warehouses, s.Concurrency) r.Add(registry.TestSpec{ - Name: name, - Owner: registry.OwnerAdmissionControl, - Benchmark: true, - CompatibleClouds: registry.AllExceptAWS, - Suites: registry.Suites(registry.Weekly), - Cluster: r.MakeClusterSpec(s.Nodes+1, spec.CPU(s.CPUs), spec.WorkloadNode()), - Run: s.run, - EncryptionSupport: registry.EncryptionMetamorphic, - Leases: registry.MetamorphicLeases, - Timeout: 20 * time.Minute, + Name: name, + Owner: registry.OwnerAdmissionControl, + Benchmark: true, + CompatibleClouds: registry.AllExceptAWS, + Suites: registry.Suites(registry.Weekly), + Cluster: r.MakeClusterSpec(s.Nodes+1, spec.CPU(s.CPUs), spec.WorkloadNode()), + Run: s.run, + EncryptionSupport: registry.EncryptionMetamorphic, + Leases: registry.MetamorphicLeases, + Timeout: 20 * time.Minute, + RequiresDeprecatedWorkload: true, }) } } diff --git a/pkg/cmd/roachtest/tests/backup_restore_roundtrip.go b/pkg/cmd/roachtest/tests/backup_restore_roundtrip.go index 8d6563428040..67de78d4ef89 100644 --- a/pkg/cmd/roachtest/tests/backup_restore_roundtrip.go +++ b/pkg/cmd/roachtest/tests/backup_restore_roundtrip.go @@ -86,11 +86,12 @@ func registerBackupRestoreRoundTrip(r registry.Registry) { RequiresLicense: true, NativeLibs: registry.LibGEOS, // See https://github.com/cockroachdb/cockroach/issues/105968 - CompatibleClouds: registry.Clouds(spec.GCE, spec.Local), - Suites: registry.Suites(registry.Nightly), - TestSelectionOptOutSuites: registry.Suites(registry.Nightly), - Randomized: true, - Skip: sp.skip, + CompatibleClouds: registry.Clouds(spec.GCE, spec.Local), + Suites: registry.Suites(registry.Nightly), + TestSelectionOptOutSuites: registry.Suites(registry.Nightly), + Randomized: true, + Skip: sp.skip, + RequiresDeprecatedWorkload: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { backupRestoreRoundTrip(ctx, t, c, sp) }, diff --git a/pkg/cmd/roachtest/tests/cdc.go b/pkg/cmd/roachtest/tests/cdc.go index 0289b0459c22..79ce299930a0 100644 --- a/pkg/cmd/roachtest/tests/cdc.go +++ b/pkg/cmd/roachtest/tests/cdc.go @@ -1725,12 +1725,13 @@ func registerCDC(r registry.Registry) { // TODO(mrtracy): This workload is designed to be running on a 20CPU nodes, // but this cannot be allocated without some sort of configuration outside // of this test. Look into it. - Benchmark: true, - Cluster: r.MakeClusterSpec(4, spec.CPU(16), spec.WorkloadNode()), - Leases: registry.MetamorphicLeases, - CompatibleClouds: registry.AllExceptAWS, - Suites: registry.Suites(registry.Nightly), - RequiresLicense: true, + Benchmark: true, + Cluster: r.MakeClusterSpec(4, spec.CPU(16), spec.WorkloadNode()), + Leases: registry.MetamorphicLeases, + CompatibleClouds: registry.AllExceptAWS, + Suites: registry.Suites(registry.Nightly), + RequiresLicense: true, + RequiresDeprecatedWorkload: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { ct := newCDCTester(ctx, t, c) defer ct.Close() diff --git a/pkg/cmd/roachtest/tests/connection_latency.go b/pkg/cmd/roachtest/tests/connection_latency.go index 890c27648217..b455d8675a21 100644 --- a/pkg/cmd/roachtest/tests/connection_latency.go +++ b/pkg/cmd/roachtest/tests/connection_latency.go @@ -109,9 +109,10 @@ func registerConnectionLatencyTest(r registry.Registry) { Owner: registry.OwnerSQLFoundations, Benchmark: true, // Add one more node for load node. - Cluster: r.MakeClusterSpec(numNodes+1, spec.WorkloadNode(), spec.GCEZones(regionUsCentral)), - CompatibleClouds: registry.OnlyGCE, - Suites: registry.Suites(registry.Nightly), + Cluster: r.MakeClusterSpec(numNodes+1, spec.WorkloadNode(), spec.GCEZones(regionUsCentral)), + CompatibleClouds: registry.OnlyGCE, + Suites: registry.Suites(registry.Nightly), + RequiresDeprecatedWorkload: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runConnectionLatencyTest(ctx, t, c, numNodes, 1, false /*password*/) }, @@ -124,24 +125,26 @@ func registerConnectionLatencyTest(r registry.Registry) { loadNodes := numZones r.Add(registry.TestSpec{ - Name: fmt.Sprintf("connection_latency/nodes=%d/multiregion/certs", numMultiRegionNodes), - Owner: registry.OwnerSQLFoundations, - Benchmark: true, - Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.GCEZones(geoZonesStr)), - CompatibleClouds: registry.OnlyGCE, - Suites: registry.Suites(registry.Nightly), + Name: fmt.Sprintf("connection_latency/nodes=%d/multiregion/certs", numMultiRegionNodes), + Owner: registry.OwnerSQLFoundations, + Benchmark: true, + Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.GCEZones(geoZonesStr)), + CompatibleClouds: registry.OnlyGCE, + Suites: registry.Suites(registry.Nightly), + RequiresDeprecatedWorkload: true, 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, - Benchmark: true, - Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.GCEZones(geoZonesStr)), - CompatibleClouds: registry.OnlyGCE, - Suites: registry.Suites(registry.Nightly), + Name: fmt.Sprintf("connection_latency/nodes=%d/multiregion/password", numMultiRegionNodes), + Owner: registry.OwnerSQLFoundations, + Benchmark: true, + Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.GCEZones(geoZonesStr)), + CompatibleClouds: registry.OnlyGCE, + Suites: registry.Suites(registry.Nightly), + RequiresDeprecatedWorkload: true, 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/indexes.go b/pkg/cmd/roachtest/tests/indexes.go index f2a86cbe9c47..14b9317141e2 100644 --- a/pkg/cmd/roachtest/tests/indexes.go +++ b/pkg/cmd/roachtest/tests/indexes.go @@ -44,8 +44,9 @@ func registerNIndexes(r registry.Registry, secondaryIndexes int) { spec.AWSZones(strings.Join(awsGeoZones, ",")), ), // TODO(radu): enable this test on AWS. - CompatibleClouds: registry.OnlyGCE, - Suites: registry.Suites(registry.Nightly), + CompatibleClouds: registry.OnlyGCE, + Suites: registry.Suites(registry.Nightly), + RequiresDeprecatedWorkload: true, // Uses CONFIGURE ZONE USING ... COPY FROM PARENT syntax. Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { firstAZ := gceGeoZones[0] diff --git a/pkg/cmd/roachtest/tests/inverted_index.go b/pkg/cmd/roachtest/tests/inverted_index.go index dfb67c7b04cf..d45359a38e42 100644 --- a/pkg/cmd/roachtest/tests/inverted_index.go +++ b/pkg/cmd/roachtest/tests/inverted_index.go @@ -26,12 +26,13 @@ import ( func registerSchemaChangeInvertedIndex(r registry.Registry) { r.Add(registry.TestSpec{ - Name: "schemachange/invertedindex", - Owner: registry.OwnerSQLFoundations, - Cluster: r.MakeClusterSpec(5, spec.WorkloadNode()), - CompatibleClouds: registry.AllExceptAWS, - Suites: registry.Suites(registry.Nightly), - Leases: registry.MetamorphicLeases, + Name: "schemachange/invertedindex", + Owner: registry.OwnerSQLFoundations, + Cluster: r.MakeClusterSpec(5, spec.WorkloadNode()), + CompatibleClouds: registry.AllExceptAWS, + Suites: registry.Suites(registry.Nightly), + Leases: registry.MetamorphicLeases, + RequiresDeprecatedWorkload: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runSchemaChangeInvertedIndex(ctx, t, c) }, diff --git a/pkg/cmd/roachtest/tests/ledger.go b/pkg/cmd/roachtest/tests/ledger.go index fdf5c30411e8..2d02e8ddfad0 100644 --- a/pkg/cmd/roachtest/tests/ledger.go +++ b/pkg/cmd/roachtest/tests/ledger.go @@ -28,12 +28,13 @@ 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, - Benchmark: true, - Cluster: r.MakeClusterSpec(nodes+1, spec.CPU(16), spec.WorkloadNode(), spec.WorkloadNodeCPU(16), spec.Geo(), spec.GCEZones(azs)), - CompatibleClouds: registry.OnlyGCE, - Suites: registry.Suites(registry.Nightly), + Name: fmt.Sprintf("ledger/nodes=%d/multi-az", nodes), + Owner: registry.OwnerKV, + Benchmark: true, + Cluster: r.MakeClusterSpec(nodes+1, spec.CPU(16), spec.WorkloadNode(), spec.WorkloadNodeCPU(16), spec.Geo(), spec.GCEZones(azs)), + CompatibleClouds: registry.OnlyGCE, + Suites: registry.Suites(registry.Nightly), + RequiresDeprecatedWorkload: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { gatewayNodes := c.Range(1, nodes/3) diff --git a/pkg/cmd/roachtest/tests/mixed_version_schemachange.go b/pkg/cmd/roachtest/tests/mixed_version_schemachange.go index e86ab8489f0f..71b08474b160 100644 --- a/pkg/cmd/roachtest/tests/mixed_version_schemachange.go +++ b/pkg/cmd/roachtest/tests/mixed_version_schemachange.go @@ -29,13 +29,14 @@ func registerSchemaChangeMixedVersions(r registry.Registry) { r.Add(registry.TestSpec{ // schemachange/mixed-versions tests random schema changes (via the schemachange workload) // in a mixed version state, validating that the cluster is still healthy (via debug doctor examine). - Name: "schemachange/mixed-versions", - Owner: registry.OwnerSQLFoundations, - Cluster: r.MakeClusterSpec(4, spec.WorkloadNode()), - CompatibleClouds: registry.AllExceptAWS, - Suites: registry.Suites(registry.Nightly), - Randomized: true, - NativeLibs: registry.LibGEOS, + Name: "schemachange/mixed-versions", + Owner: registry.OwnerSQLFoundations, + Cluster: r.MakeClusterSpec(4, spec.WorkloadNode()), + CompatibleClouds: registry.AllExceptAWS, + Suites: registry.Suites(registry.Nightly), + Randomized: true, + NativeLibs: registry.LibGEOS, + RequiresDeprecatedWorkload: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { maxOps := 1000 concurrency := 5 diff --git a/pkg/cmd/roachtest/tests/queue.go b/pkg/cmd/roachtest/tests/queue.go index 9e199ba4d28f..29d41f04cdf8 100644 --- a/pkg/cmd/roachtest/tests/queue.go +++ b/pkg/cmd/roachtest/tests/queue.go @@ -29,13 +29,14 @@ func registerQueue(r registry.Registry) { // One node runs the workload generator, all other nodes host CockroachDB. const numNodes = 2 r.Add(registry.TestSpec{ - Skip: "https://github.com/cockroachdb/cockroach/issues/17229", - Name: fmt.Sprintf("queue/nodes=%d", numNodes-1), - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(numNodes, spec.WorkloadNode()), - CompatibleClouds: registry.AllExceptAWS, - Suites: registry.Suites(registry.Nightly), - Leases: registry.MetamorphicLeases, + Skip: "https://github.com/cockroachdb/cockroach/issues/17229", + Name: fmt.Sprintf("queue/nodes=%d", numNodes-1), + Owner: registry.OwnerKV, + Cluster: r.MakeClusterSpec(numNodes, spec.WorkloadNode()), + CompatibleClouds: registry.AllExceptAWS, + Suites: registry.Suites(registry.Nightly), + Leases: registry.MetamorphicLeases, + RequiresDeprecatedWorkload: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runQueue(ctx, t, c) }, diff --git a/pkg/cmd/roachtest/tests/roachmart.go b/pkg/cmd/roachtest/tests/roachmart.go index 8ad4f4f35e12..13608b2a63e3 100644 --- a/pkg/cmd/roachtest/tests/roachmart.go +++ b/pkg/cmd/roachtest/tests/roachmart.go @@ -74,12 +74,13 @@ func registerRoachmart(r registry.Registry) { for _, v := range []bool{true, false} { v := v r.Add(registry.TestSpec{ - Name: fmt.Sprintf("roachmart/partition=%v", v), - Owner: registry.OwnerKV, - Cluster: r.MakeClusterSpec(9, spec.Geo(), spec.GCEZones("us-central1-b,us-west1-b,europe-west2-b")), - CompatibleClouds: registry.OnlyGCE, - Suites: registry.Suites(registry.Nightly), - Leases: registry.MetamorphicLeases, + Name: fmt.Sprintf("roachmart/partition=%v", v), + Owner: registry.OwnerKV, + Cluster: r.MakeClusterSpec(9, spec.Geo(), spec.GCEZones("us-central1-b,us-west1-b,europe-west2-b")), + CompatibleClouds: registry.OnlyGCE, + Suites: registry.Suites(registry.Nightly), + Leases: registry.MetamorphicLeases, + RequiresDeprecatedWorkload: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runRoachmart(ctx, t, c, v) }, diff --git a/pkg/cmd/roachtest/tests/schemachange_random_load.go b/pkg/cmd/roachtest/tests/schemachange_random_load.go index 814f7d62fd46..85435c51ae9c 100644 --- a/pkg/cmd/roachtest/tests/schemachange_random_load.go +++ b/pkg/cmd/roachtest/tests/schemachange_random_load.go @@ -40,10 +40,11 @@ func registerSchemaChangeRandomLoad(r registry.Registry) { spec.AWSZones("us-east-2b,us-west-1a,eu-west-1a"), ), // TODO(radu): enable this test on AWS. - CompatibleClouds: registry.AllExceptAWS, - Suites: registry.Suites(registry.Nightly), - Leases: registry.MetamorphicLeases, - NativeLibs: registry.LibGEOS, + CompatibleClouds: registry.AllExceptAWS, + Suites: registry.Suites(registry.Nightly), + Leases: registry.MetamorphicLeases, + NativeLibs: registry.LibGEOS, + RequiresDeprecatedWorkload: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { maxOps := 5000 concurrency := 20 diff --git a/pkg/cmd/roachtest/tests/tpchbench.go b/pkg/cmd/roachtest/tests/tpchbench.go index 0ce7938248fd..b043b094eac1 100644 --- a/pkg/cmd/roachtest/tests/tpchbench.go +++ b/pkg/cmd/roachtest/tests/tpchbench.go @@ -85,7 +85,7 @@ func runTPCHBench(ctx context.Context, t test.Test, c cluster.Cluster, b tpchBen // Run with only one worker to get best-case single-query performance. cmd := fmt.Sprintf( - "./cockroach workload run querybench --db=tpch --concurrency=1 --query-file=%s "+ + "./workload run querybench --db=tpch --concurrency=1 --query-file=%s "+ "--num-runs=%d --max-ops=%d {pgurl%s} "+ "--histograms="+t.PerfArtifactsDir()+"/stats.json --histograms-max-latency=%s", filename, @@ -165,8 +165,9 @@ func registerTPCHBenchSpec(r registry.Registry, b tpchBenchSpec) { Cluster: r.MakeClusterSpec(numNodes, spec.WorkloadNode()), // Uses gs://cockroach-fixtures-us-east1. See: // https://github.com/cockroachdb/cockroach/issues/105968 - CompatibleClouds: registry.Clouds(spec.GCE, spec.Local), - Suites: registry.Suites(registry.Nightly), + CompatibleClouds: registry.Clouds(spec.GCE, spec.Local), + Suites: registry.Suites(registry.Nightly), + RequiresDeprecatedWorkload: true, // uses querybench Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runTPCHBench(ctx, t, c, b) }, From 6cdc33dd013095ba8d87de873644288bdb4a4604 Mon Sep 17 00:00:00 2001 From: DarrylWong <darryl@cockroachlabs.com> Date: Thu, 1 Aug 2024 15:33:24 -0400 Subject: [PATCH 2/2] roachtest: auto upload workload binary if required by workload node Currently, if a test wishes to use the deprecated workload, they must both set the RequiresDeprecatedWorkload test spec and manually upload it in the test itself. However, if the test provisions a workload node via the WorkloadNode cluster spec, we know that the test most likely wants the workload there. This change checks that the above two conditions are true, and now automatically uploads the workload binary for the test. This leaves just 3 occurences of tests manually uploading the workload binary themselves. --- pkg/cmd/roachtest/cluster.go | 14 ++++++++++++++ pkg/cmd/roachtest/test/test_interface.go | 5 +++++ pkg/cmd/roachtest/test_runner.go | 3 +++ pkg/cmd/roachtest/tests/acceptance.go | 2 +- .../tests/admission_control_tpcc_overload.go | 6 +----- .../roachtest/tests/backup_restore_roundtrip.go | 2 +- pkg/cmd/roachtest/tests/cdc.go | 5 +---- pkg/cmd/roachtest/tests/connection_latency.go | 6 +++--- pkg/cmd/roachtest/tests/indexes.go | 5 +---- pkg/cmd/roachtest/tests/inverted_index.go | 5 +---- pkg/cmd/roachtest/tests/ledger.go | 6 +----- pkg/cmd/roachtest/tests/mixed_version_backup.go | 1 - .../roachtest/tests/mixed_version_schemachange.go | 4 +--- pkg/cmd/roachtest/tests/queue.go | 5 +---- pkg/cmd/roachtest/tests/roachmart.go | 5 ++--- .../roachtest/tests/schemachange_random_load.go | 2 +- 16 files changed, 37 insertions(+), 39 deletions(-) diff --git a/pkg/cmd/roachtest/cluster.go b/pkg/cmd/roachtest/cluster.go index dfcf879918f9..f87f9fcb2438 100644 --- a/pkg/cmd/roachtest/cluster.go +++ b/pkg/cmd/roachtest/cluster.go @@ -2007,6 +2007,20 @@ func (c *clusterImpl) PutLibraries( return nil } +// PutDeprecatedWorkload checks if the test requires the deprecated +// workload and that it has a workload node provisioned. If it does, +// then it auto-uploads the workload binary to the workload node. +// If the test requires the binary but doesn't have a workload node, +// it should handle uploading it in the test itself. +func (c *clusterImpl) PutDeprecatedWorkload( + ctx context.Context, l *logger.Logger, t *testImpl, +) error { + if t.spec.RequiresDeprecatedWorkload && t.spec.Cluster.WorkloadNode { + return c.PutE(ctx, l, t.DeprecatedWorkload(), test.DefaultDeprecatedWorkloadPath, c.WorkloadNode()) + } + return nil +} + // Stage stages a binary to the cluster. func (c *clusterImpl) Stage( ctx context.Context, diff --git a/pkg/cmd/roachtest/test/test_interface.go b/pkg/cmd/roachtest/test/test_interface.go index cdf93721f1cd..6b8bc92e03e2 100644 --- a/pkg/cmd/roachtest/test/test_interface.go +++ b/pkg/cmd/roachtest/test/test_interface.go @@ -20,6 +20,11 @@ import ( // cluster. const DefaultCockroachPath = "./cockroach" +// DefaultDeprecatedWorkloadPath is the path where the binary passed +// to the `--workload` flag will be made available in the workload +// node if one is provisioned. +const DefaultDeprecatedWorkloadPath = "./workload" + // EnvAssertionsEnabledSeed is the name of the environment variable // that, when set, causes roachtest to use a binary with runtime // assertions enabled (if available), using the random seed contained diff --git a/pkg/cmd/roachtest/test_runner.go b/pkg/cmd/roachtest/test_runner.go index c4d47d1be22f..ac86e1776f23 100644 --- a/pkg/cmd/roachtest/test_runner.go +++ b/pkg/cmd/roachtest/test_runner.go @@ -861,6 +861,9 @@ func (r *testRunner) runWorker( if setupErr == nil { setupErr = c.PutLibraries(ctx, "./lib", t.spec.NativeLibs) } + if setupErr == nil { + setupErr = c.PutDeprecatedWorkload(ctx, l, t) + } if setupErr != nil { // If there was an error setting up the cluster (uploading diff --git a/pkg/cmd/roachtest/tests/acceptance.go b/pkg/cmd/roachtest/tests/acceptance.go index 77457da75f5b..01448f0af532 100644 --- a/pkg/cmd/roachtest/tests/acceptance.go +++ b/pkg/cmd/roachtest/tests/acceptance.go @@ -84,7 +84,7 @@ func registerAcceptance(r registry.Registry) { defaultLeases: true, randomized: true, nativeLibs: registry.LibGEOS, - requiresDeprecatedWorkload: true, + requiresDeprecatedWorkload: true, // uses schemachange }, }, registry.OwnerDisasterRecovery: { diff --git a/pkg/cmd/roachtest/tests/admission_control_tpcc_overload.go b/pkg/cmd/roachtest/tests/admission_control_tpcc_overload.go index 385479c83bbe..e0c63bae5127 100644 --- a/pkg/cmd/roachtest/tests/admission_control_tpcc_overload.go +++ b/pkg/cmd/roachtest/tests/admission_control_tpcc_overload.go @@ -54,10 +54,6 @@ func (s tpccOLAPSpec) run(ctx context.Context, t test.Test, c cluster.Cluster) { ctx, t, t.L(), c, tpccOptions{ Warehouses: s.Warehouses, SetupType: usingImport, }) - // We make use of querybench below, only available through the `workload` - // binary. - c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.WorkloadNode()) - const queryFileName = "queries.sql" // querybench expects the entire query to be on a single line. queryLine := `"` + strings.Replace(tpccOlapQuery, "\n", " ", -1) + `"` @@ -173,7 +169,7 @@ func registerTPCCOverload(r registry.Registry) { EncryptionSupport: registry.EncryptionMetamorphic, Leases: registry.MetamorphicLeases, Timeout: 20 * time.Minute, - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses querybench }) } } diff --git a/pkg/cmd/roachtest/tests/backup_restore_roundtrip.go b/pkg/cmd/roachtest/tests/backup_restore_roundtrip.go index 67de78d4ef89..c2ed8a816afd 100644 --- a/pkg/cmd/roachtest/tests/backup_restore_roundtrip.go +++ b/pkg/cmd/roachtest/tests/backup_restore_roundtrip.go @@ -91,7 +91,7 @@ func registerBackupRestoreRoundTrip(r registry.Registry) { TestSelectionOptOutSuites: registry.Suites(registry.Nightly), Randomized: true, Skip: sp.skip, - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses schemachange Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { backupRestoreRoundTrip(ctx, t, c, sp) }, diff --git a/pkg/cmd/roachtest/tests/cdc.go b/pkg/cmd/roachtest/tests/cdc.go index 79ce299930a0..e8fb5fd4cfd5 100644 --- a/pkg/cmd/roachtest/tests/cdc.go +++ b/pkg/cmd/roachtest/tests/cdc.go @@ -1731,15 +1731,12 @@ func registerCDC(r registry.Registry) { CompatibleClouds: registry.AllExceptAWS, Suites: registry.Suites(registry.Nightly), RequiresLicense: true, - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses ledger Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { ct := newCDCTester(ctx, t, c) defer ct.Close() workloadStart := timeutil.Now() - // The ledger workload is not available in the cockroach binary so we - // must use the deprecated workload. - c.Put(ctx, t.DeprecatedWorkload(), "./workload", ct.workloadNode) ct.runLedgerWorkload(ledgerArgs{duration: "28m"}) alterStmt := "ALTER DATABASE ledger CONFIGURE ZONE USING range_max_bytes = 805306368, range_min_bytes = 134217728" diff --git a/pkg/cmd/roachtest/tests/connection_latency.go b/pkg/cmd/roachtest/tests/connection_latency.go index b455d8675a21..04154280f7a9 100644 --- a/pkg/cmd/roachtest/tests/connection_latency.go +++ b/pkg/cmd/roachtest/tests/connection_latency.go @@ -112,7 +112,7 @@ func registerConnectionLatencyTest(r registry.Registry) { Cluster: r.MakeClusterSpec(numNodes+1, spec.WorkloadNode(), spec.GCEZones(regionUsCentral)), CompatibleClouds: registry.OnlyGCE, Suites: registry.Suites(registry.Nightly), - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses connectionlatency Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runConnectionLatencyTest(ctx, t, c, numNodes, 1, false /*password*/) }, @@ -131,7 +131,7 @@ func registerConnectionLatencyTest(r registry.Registry) { Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.GCEZones(geoZonesStr)), CompatibleClouds: registry.OnlyGCE, Suites: registry.Suites(registry.Nightly), - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses connectionlatency Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runConnectionLatencyTest(ctx, t, c, numMultiRegionNodes, numZones, false /*password*/) }, @@ -144,7 +144,7 @@ func registerConnectionLatencyTest(r registry.Registry) { Cluster: r.MakeClusterSpec(numMultiRegionNodes+loadNodes, spec.Geo(), spec.GCEZones(geoZonesStr)), CompatibleClouds: registry.OnlyGCE, Suites: registry.Suites(registry.Nightly), - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses connectionlatency 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/indexes.go b/pkg/cmd/roachtest/tests/indexes.go index 14b9317141e2..480fc6674b55 100644 --- a/pkg/cmd/roachtest/tests/indexes.go +++ b/pkg/cmd/roachtest/tests/indexes.go @@ -46,7 +46,7 @@ func registerNIndexes(r registry.Registry, secondaryIndexes int) { // TODO(radu): enable this test on AWS. CompatibleClouds: registry.OnlyGCE, Suites: registry.Suites(registry.Nightly), - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses indexes // Uses CONFIGURE ZONE USING ... COPY FROM PARENT syntax. Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { firstAZ := gceGeoZones[0] @@ -55,9 +55,6 @@ func registerNIndexes(r registry.Registry, secondaryIndexes int) { } gatewayNodes := c.Range(1, nodes/3) - // The indexes workload is not available in the cockroach binary, - // so we must use the deprecated workload. - c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.WorkloadNode()) c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.CRDBNodes()) conn := c.Conn(ctx, t.L(), 1) diff --git a/pkg/cmd/roachtest/tests/inverted_index.go b/pkg/cmd/roachtest/tests/inverted_index.go index d45359a38e42..416dd666a8b3 100644 --- a/pkg/cmd/roachtest/tests/inverted_index.go +++ b/pkg/cmd/roachtest/tests/inverted_index.go @@ -32,7 +32,7 @@ func registerSchemaChangeInvertedIndex(r registry.Registry) { CompatibleClouds: registry.AllExceptAWS, Suites: registry.Suites(registry.Nightly), Leases: registry.MetamorphicLeases, - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses json Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runSchemaChangeInvertedIndex(ctx, t, c) }, @@ -42,9 +42,6 @@ func registerSchemaChangeInvertedIndex(r registry.Registry) { // runInvertedIndex tests the correctness and performance of building an // inverted index on randomly generated JSON data (from the JSON workload). func runSchemaChangeInvertedIndex(ctx context.Context, t test.Test, c cluster.Cluster) { - // The json workload is not available in the cockroach binary, - // so we must use the deprecated workload. - c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.WorkloadNode()) c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.CRDBNodes()) cmdInit := "./workload init json {pgurl:1}" diff --git a/pkg/cmd/roachtest/tests/ledger.go b/pkg/cmd/roachtest/tests/ledger.go index 2d02e8ddfad0..08eedcd0e15a 100644 --- a/pkg/cmd/roachtest/tests/ledger.go +++ b/pkg/cmd/roachtest/tests/ledger.go @@ -34,14 +34,10 @@ func registerLedger(r registry.Registry) { Cluster: r.MakeClusterSpec(nodes+1, spec.CPU(16), spec.WorkloadNode(), spec.WorkloadNodeCPU(16), spec.Geo(), spec.GCEZones(azs)), CompatibleClouds: registry.OnlyGCE, Suites: registry.Suites(registry.Nightly), - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses ledger Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { gatewayNodes := c.Range(1, nodes/3) - // The ledger workload is not available in the cockroach binary, - // so we must use the deprecated workload. - c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.WorkloadNode()) - // Don't start a scheduled backup on this perf sensitive roachtest that reports to roachperf. c.Start(ctx, t.L(), option.NewStartOpts(option.NoBackupSchedule), install.MakeClusterSettings(), c.CRDBNodes()) diff --git a/pkg/cmd/roachtest/tests/mixed_version_backup.go b/pkg/cmd/roachtest/tests/mixed_version_backup.go index 2f1422e06990..d19455db1302 100644 --- a/pkg/cmd/roachtest/tests/mixed_version_backup.go +++ b/pkg/cmd/roachtest/tests/mixed_version_backup.go @@ -2743,7 +2743,6 @@ func prepSchemaChangeWorkload( testUtils *CommonTestUtils, testRNG *rand.Rand, ) error { - testUtils.cluster.Put(ctx, testUtils.t.DeprecatedWorkload(), "./workload", workloadNode) if err := testUtils.Exec(ctx, testRNG, fmt.Sprintf("CREATE DATABASE %s", schemaChangeDB)); err != nil { return err } diff --git a/pkg/cmd/roachtest/tests/mixed_version_schemachange.go b/pkg/cmd/roachtest/tests/mixed_version_schemachange.go index 71b08474b160..265cf5c3a395 100644 --- a/pkg/cmd/roachtest/tests/mixed_version_schemachange.go +++ b/pkg/cmd/roachtest/tests/mixed_version_schemachange.go @@ -36,7 +36,7 @@ func registerSchemaChangeMixedVersions(r registry.Registry) { Suites: registry.Suites(registry.Nightly), Randomized: true, NativeLibs: registry.LibGEOS, - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses schemachange Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { maxOps := 1000 concurrency := 5 @@ -65,8 +65,6 @@ func runSchemaChangeMixedVersions( mixedversion.AlwaysUseLatestPredecessors, ) - c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.WorkloadNode()) - // Run the schemachange workload on a random node, along with validating the schema changes for the cluster on a random node. schemaChangeAndValidationStep := func( ctx context.Context, l *logger.Logger, r *rand.Rand, h *mixedversion.Helper, diff --git a/pkg/cmd/roachtest/tests/queue.go b/pkg/cmd/roachtest/tests/queue.go index 29d41f04cdf8..810271b75e01 100644 --- a/pkg/cmd/roachtest/tests/queue.go +++ b/pkg/cmd/roachtest/tests/queue.go @@ -36,7 +36,7 @@ func registerQueue(r registry.Registry) { CompatibleClouds: registry.AllExceptAWS, Suites: registry.Suites(registry.Nightly), Leases: registry.MetamorphicLeases, - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses queue Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runQueue(ctx, t, c) }, @@ -46,9 +46,6 @@ func registerQueue(r registry.Registry) { func runQueue(ctx context.Context, t test.Test, c cluster.Cluster) { dbNodeCount := c.Spec().NodeCount - 1 // Distribute programs to the correct nodes and start CockroachDB. - // The queue workload is not available in the cockroach binary, - // so we must use the deprecated workload. - c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.WorkloadNode()) c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.CRDBNodes()) runQueueWorkload := func(duration time.Duration, initTables bool) { diff --git a/pkg/cmd/roachtest/tests/roachmart.go b/pkg/cmd/roachtest/tests/roachmart.go index 13608b2a63e3..b0129a57279f 100644 --- a/pkg/cmd/roachtest/tests/roachmart.go +++ b/pkg/cmd/roachtest/tests/roachmart.go @@ -24,8 +24,7 @@ import ( func registerRoachmart(r registry.Registry) { runRoachmart := func(ctx context.Context, t test.Test, c cluster.Cluster, partition bool) { - // The roachmart workload is not available in the cockroach binary, - // so we must use the deprecated workload. + // This test expects the workload binary on all nodes. c.Put(ctx, t.DeprecatedWorkload(), "./workload") c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings()) @@ -80,7 +79,7 @@ func registerRoachmart(r registry.Registry) { CompatibleClouds: registry.OnlyGCE, Suites: registry.Suites(registry.Nightly), Leases: registry.MetamorphicLeases, - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses roachmart Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { runRoachmart(ctx, t, c, v) }, diff --git a/pkg/cmd/roachtest/tests/schemachange_random_load.go b/pkg/cmd/roachtest/tests/schemachange_random_load.go index 85435c51ae9c..4723f52b7565 100644 --- a/pkg/cmd/roachtest/tests/schemachange_random_load.go +++ b/pkg/cmd/roachtest/tests/schemachange_random_load.go @@ -44,7 +44,7 @@ func registerSchemaChangeRandomLoad(r registry.Registry) { Suites: registry.Suites(registry.Nightly), Leases: registry.MetamorphicLeases, NativeLibs: registry.LibGEOS, - RequiresDeprecatedWorkload: true, + RequiresDeprecatedWorkload: true, // uses schemachange Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { maxOps := 5000 concurrency := 20