From a2ba442bd1021b17b295be732a19b84de1370de7 Mon Sep 17 00:00:00 2001 From: Stan Rosenberg Date: Sat, 1 Jul 2023 21:25:39 -0400 Subject: [PATCH] roachtest: load AWS fixtures from us-east-2 to avoid egress In [1], new restore tests have been added to load fixtures from `s3://cockroach-fixtures`. Since the regional bucket is located in us-east-1, and roachprod provisions in us-east-2, egress during nightly and weekly runs amounted to non-trivial cost. This change replaces the bucket with the regional `s3://cockroach-fixtures-us-east-2`, which was replicated from us-east-1. As a precaution, other roachtest which depend on `s3://cockroach-fixtures` but do not currently run in AWS, are now guarded by a conditional skip. A new issue (#105968) has been created to address the problem of localizing fixtures under a given cloud provider. [1] https://github.com/cockroachdb/cockroach/pull/94143 Epic: none Informs: https://github.com/cockroachdb/cockroach/issues/105968 Release note: None --- pkg/cmd/roachtest/tests/backup.go | 3 +++ pkg/cmd/roachtest/tests/copy.go | 4 ++++ pkg/cmd/roachtest/tests/import.go | 9 +++++++++ pkg/cmd/roachtest/tests/import_cancellation.go | 3 +++ pkg/cmd/roachtest/tests/mixed_version_backup.go | 2 +- .../mixed_version_decl_schemachange_compat.go | 2 +- pkg/cmd/roachtest/tests/restore.go | 14 ++++++++++++-- pkg/cmd/roachtest/tests/schemachange.go | 3 +++ pkg/cmd/roachtest/tests/sqlsmith.go | 3 +++ pkg/cmd/roachtest/tests/tpc_utils.go | 5 +++++ pkg/cmd/roachtest/tests/tpcdsvec.go | 4 ++++ pkg/roachprod/vm/aws/aws.go | 3 +++ 12 files changed, 51 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/roachtest/tests/backup.go b/pkg/cmd/roachtest/tests/backup.go index 7badd881f534..5dff03c089fb 100644 --- a/pkg/cmd/roachtest/tests/backup.go +++ b/pkg/cmd/roachtest/tests/backup.go @@ -893,6 +893,9 @@ func registerBackup(r registry.Registry) { Leases: registry.MetamorphicLeases, EncryptionSupport: registry.EncryptionMetamorphic, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } runBackupMVCCRangeTombstones(ctx, t, c, mvccRangeTombstoneConfig{}) }, }) diff --git a/pkg/cmd/roachtest/tests/copy.go b/pkg/cmd/roachtest/tests/copy.go index 6031f6c8d1f8..06cedc1e48f9 100644 --- a/pkg/cmd/roachtest/tests/copy.go +++ b/pkg/cmd/roachtest/tests/copy.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry" + "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" "github.com/cockroachdb/cockroach/pkg/roachprod/install" "github.com/cockroachdb/errors" @@ -184,6 +185,9 @@ func registerCopy(r registry.Registry) { Cluster: r.MakeClusterSpec(tc.nodes), Leases: registry.MetamorphicLeases, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } runCopy(ctx, t, c, tc.rows, tc.txn) }, }) diff --git a/pkg/cmd/roachtest/tests/import.go b/pkg/cmd/roachtest/tests/import.go index 0976869b77fe..ed549f6eabc6 100644 --- a/pkg/cmd/roachtest/tests/import.go +++ b/pkg/cmd/roachtest/tests/import.go @@ -90,6 +90,9 @@ func registerImportNodeShutdown(r registry.Registry) { Cluster: r.MakeClusterSpec(4), Leases: registry.MetamorphicLeases, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } c.Put(ctx, t.Cockroach(), "./cockroach") c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings()) gatewayNode := 2 @@ -105,6 +108,9 @@ func registerImportNodeShutdown(r registry.Registry) { Cluster: r.MakeClusterSpec(4), Leases: registry.MetamorphicLeases, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } c.Put(ctx, t.Cockroach(), "./cockroach") c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings()) gatewayNode := 2 @@ -219,6 +225,9 @@ func registerImportTPCH(r registry.Registry) { EncryptionSupport: registry.EncryptionMetamorphic, Leases: registry.MetamorphicLeases, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } tick, perfBuf := initBulkJobPerfArtifacts(t.Name(), item.timeout) c.Put(ctx, t.Cockroach(), "./cockroach") diff --git a/pkg/cmd/roachtest/tests/import_cancellation.go b/pkg/cmd/roachtest/tests/import_cancellation.go index 19f2606609c3..6537468c8421 100644 --- a/pkg/cmd/roachtest/tests/import_cancellation.go +++ b/pkg/cmd/roachtest/tests/import_cancellation.go @@ -38,6 +38,9 @@ func registerImportCancellation(r registry.Registry) { Cluster: r.MakeClusterSpec(6, spec.CPU(32)), Leases: registry.MetamorphicLeases, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } runImportCancellation(ctx, t, c, rangeTombstones) }, }) diff --git a/pkg/cmd/roachtest/tests/mixed_version_backup.go b/pkg/cmd/roachtest/tests/mixed_version_backup.go index 2c6a54514418..5eb36d7c5dbc 100644 --- a/pkg/cmd/roachtest/tests/mixed_version_backup.go +++ b/pkg/cmd/roachtest/tests/mixed_version_backup.go @@ -2086,7 +2086,7 @@ func registerBackupMixedVersion(r registry.Registry) { RequiresLicense: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { if c.Spec().Cloud != spec.GCE { - t.Skip("uses gs://cockroachdb-backup-testing, available only in GCE") + t.Skip("uses gs://cockroachdb-backup-testing; see https://github.com/cockroachdb/cockroach/issues/105968") } roachNodes := c.Range(1, c.Spec().NodeCount-1) diff --git a/pkg/cmd/roachtest/tests/mixed_version_decl_schemachange_compat.go b/pkg/cmd/roachtest/tests/mixed_version_decl_schemachange_compat.go index 9d791b11d53f..70c9004dd4cb 100644 --- a/pkg/cmd/roachtest/tests/mixed_version_decl_schemachange_compat.go +++ b/pkg/cmd/roachtest/tests/mixed_version_decl_schemachange_compat.go @@ -32,7 +32,7 @@ func registerDeclSchemaChangeCompatMixedVersions(r registry.Registry) { Cluster: r.MakeClusterSpec(1), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { if c.Spec().Cloud != spec.GCE { - t.Skip("uses gsutil with gs://cockroach-corpus, available only in GCE") + t.Skip("uses gs://cockroach-corpus; see https://github.com/cockroachdb/cockroach/issues/105968") } runDeclSchemaChangeCompatMixedVersions(ctx, t, c, *t.BuildVersion()) }, diff --git a/pkg/cmd/roachtest/tests/restore.go b/pkg/cmd/roachtest/tests/restore.go index f1cb8e93d476..a7623d273468 100644 --- a/pkg/cmd/roachtest/tests/restore.go +++ b/pkg/cmd/roachtest/tests/restore.go @@ -620,8 +620,18 @@ func (bs backupSpecs) storagePrefix() string { } func (bs backupSpecs) backupCollection() string { - return fmt.Sprintf(`'%s://cockroach-fixtures/backups/%s/%s/%s?AUTH=implicit'`, - bs.storagePrefix(), bs.workload.fixtureDir(), bs.version, bs.backupProperties) + // N.B. AWS buckets are _regional_ whereas GCS buckets are _multi-regional_. Thus, in order to avoid egress (cost), + // we use us-east-2 for AWS, which is the default region for all roachprod clusters. (See roachprod/vm/aws/aws.go) + switch bs.storagePrefix() { + case "s3": + return fmt.Sprintf(`'s3://cockroach-fixtures-us-east-2/backups/%s/%s/%s?AUTH=implicit'`, + bs.workload.fixtureDir(), bs.version, bs.backupProperties) + case "gs": + return fmt.Sprintf(`'gs://cockroach-fixtures/backups/%s/%s/%s?AUTH=implicit'`, + bs.workload.fixtureDir(), bs.version, bs.backupProperties) + default: + panic(fmt.Sprintf("unknown storage prefix: %s", bs.storagePrefix())) + } } // getAOSTCmd returns a sql cmd that will return a system time that is equal to the end time of diff --git a/pkg/cmd/roachtest/tests/schemachange.go b/pkg/cmd/roachtest/tests/schemachange.go index 5df1fe678953..6069c5cb223f 100644 --- a/pkg/cmd/roachtest/tests/schemachange.go +++ b/pkg/cmd/roachtest/tests/schemachange.go @@ -34,6 +34,9 @@ func registerSchemaChangeDuringKV(r registry.Registry) { Cluster: r.MakeClusterSpec(5), Leases: registry.MetamorphicLeases, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } const fixturePath = `gs://cockroach-fixtures/workload/tpch/scalefactor=10/backup?AUTH=implicit` c.Put(ctx, t.Cockroach(), "./cockroach") diff --git a/pkg/cmd/roachtest/tests/sqlsmith.go b/pkg/cmd/roachtest/tests/sqlsmith.go index 8052c64b2750..515216228ce8 100644 --- a/pkg/cmd/roachtest/tests/sqlsmith.go +++ b/pkg/cmd/roachtest/tests/sqlsmith.go @@ -317,6 +317,9 @@ WITH into_db = 'defaultdb', unsafe_restore_incompatible_version; // NB: sqlsmith failures should never block a release. NonReleaseBlocker: true, Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } runSQLSmith(ctx, t, c, setup, setting) }, }) diff --git a/pkg/cmd/roachtest/tests/tpc_utils.go b/pkg/cmd/roachtest/tests/tpc_utils.go index 9d12197ba5dc..99b8a7dd2ab8 100644 --- a/pkg/cmd/roachtest/tests/tpc_utils.go +++ b/pkg/cmd/roachtest/tests/tpc_utils.go @@ -17,6 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option" + "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" "github.com/cockroachdb/cockroach/pkg/roachprod/install" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -42,6 +43,10 @@ func loadTPCHDataset( roachNodes option.NodeListOption, disableMergeQueue bool, ) (retErr error) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } + _, err := db.Exec("SET CLUSTER SETTING sql.stats.automatic_collection.enabled = false;") if retErr != nil { return err diff --git a/pkg/cmd/roachtest/tests/tpcdsvec.go b/pkg/cmd/roachtest/tests/tpcdsvec.go index 892c0d56bfb0..960b43388211 100644 --- a/pkg/cmd/roachtest/tests/tpcdsvec.go +++ b/pkg/cmd/roachtest/tests/tpcdsvec.go @@ -19,6 +19,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry" + "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" "github.com/cockroachdb/cockroach/pkg/roachprod/install" "github.com/cockroachdb/cockroach/pkg/util/timeutil" @@ -190,6 +191,9 @@ WITH unsafe_restore_incompatible_version; Benchmark: true, Cluster: r.MakeClusterSpec(3), Run: func(ctx context.Context, t test.Test, c cluster.Cluster) { + if c.Spec().Cloud != spec.GCE { + t.Skip("uses gs://cockroach-fixtures; see https://github.com/cockroachdb/cockroach/issues/105968") + } runTPCDSVec(ctx, t, c) }, }) diff --git a/pkg/roachprod/vm/aws/aws.go b/pkg/roachprod/vm/aws/aws.go index e910a9b28210..93867e74e1de 100644 --- a/pkg/roachprod/vm/aws/aws.go +++ b/pkg/roachprod/vm/aws/aws.go @@ -264,6 +264,9 @@ var defaultConfig = func() (cfg *awsConfig) { // defaultCreateZones is the list of availability zones used by default for // cluster creation. If the geo flag is specified, nodes are distributed between // zones. +// NOTE: a number of AWS roachtests are dependent on us-east-2 for loading fixtures, +// out of s3://cockroach-fixtures-us-east-2. AWS doesn't support multi-regional buckets, thus resulting in material +// egress cost if the test loads from a different region. See https://github.com/cockroachdb/cockroach/issues/105968. var defaultCreateZones = []string{ // N.B. us-east-2a is the default zone for non-geo distributed clusters. It appears to have a higher on-demand // capacity of c7g.8xlarge (graviton3) than us-east-2b.