Skip to content

Commit

Permalink
Merge #105969
Browse files Browse the repository at this point in the history
105969: roachtest: load AWS fixtures from us-east-2 to avoid egress r=herkolategan a=srosenberg

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] #94143

Epic: none
Informs: #105968

Release note: None

Co-authored-by: Stan Rosenberg <[email protected]>
  • Loading branch information
craig[bot] and srosenberg committed Jul 3, 2023
2 parents c5a52d9 + a2ba442 commit 40aec52
Show file tree
Hide file tree
Showing 12 changed files with 51 additions and 4 deletions.
3 changes: 3 additions & 0 deletions pkg/cmd/roachtest/tests/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
},
})
Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/roachtest/tests/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
},
})
Expand Down
9 changes: 9 additions & 0 deletions pkg/cmd/roachtest/tests/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/roachtest/tests/import_cancellation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/mixed_version_backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
},
Expand Down
14 changes: 12 additions & 2 deletions pkg/cmd/roachtest/tests/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/roachtest/tests/schemachange.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions pkg/cmd/roachtest/tests/sqlsmith.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
})
Expand Down
5 changes: 5 additions & 0 deletions pkg/cmd/roachtest/tests/tpc_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/roachtest/tests/tpcdsvec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
},
})
Expand Down
3 changes: 3 additions & 0 deletions pkg/roachprod/vm/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 40aec52

Please sign in to comment.