From a4aa8ef496c66e904407680a7a4b601fc64c256e Mon Sep 17 00:00:00 2001 From: Faizan Qazi Date: Fri, 17 Mar 2023 14:34:50 +0000 Subject: [PATCH 1/2] roachtest: enable schema changes in acceptance/version-upgrade Previously, due to flakes we disabled schema changes inside the version update test. This patch re-enables them, since we are confident that the workload itslef is now stable in a mixed version state. Fixes: #58489 Release note: None --- pkg/cmd/roachtest/tests/versionupgrade.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/roachtest/tests/versionupgrade.go b/pkg/cmd/roachtest/tests/versionupgrade.go index cf87d9c476c3..d4321e007aeb 100644 --- a/pkg/cmd/roachtest/tests/versionupgrade.go +++ b/pkg/cmd/roachtest/tests/versionupgrade.go @@ -21,6 +21,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/roachtestutil" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/clusterupgrade" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil/mixedversion" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test" @@ -102,8 +103,12 @@ func runVersionUpgrade(ctx context.Context, t test.Test, c cluster.Cluster) { if c.IsLocal() && runtime.GOARCH == "arm64" { t.Skip("Skip under ARM64. See https://github.com/cockroachdb/cockroach/issues/89268") } - + c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.All()) mvt := mixedversion.NewTest(ctx, t, t.L(), c, c.All()) + mvt.OnStartup("setup schema changer workload", func(ctx context.Context, l *logger.Logger, r *rand.Rand, helper *mixedversion.Helper) error { + // Execute the workload init. + return c.RunE(ctx, c.All(), "./workload init schemachange") + }) mvt.InMixedVersion("run backup", func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper) error { // Verify that backups can be created in various configurations. This is // important to test because changes in system tables might cause backups to @@ -126,6 +131,15 @@ func runVersionUpgrade(ctx context.Context, t test.Test, c cluster.Cluster) { return nil }, ) + mvt.InMixedVersion( + "test schema change step", + func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper) error { + l.Printf("running schema workload step") + runCmd := roachtestutil.NewCommand("./workload run schemachange").Flag("verbose", 1).Flag("max-ops", 10).Flag("concurrency", 2).Arg("{pgurl:1-%d}", len(c.All())) + randomNode := h.RandomNode(rng, c.All()) + return c.RunE(ctx, option.NodeListOption{randomNode}, runCmd.String()) + }, + ) mvt.AfterUpgradeFinalized( "check if GC TTL is pinned", func(ctx context.Context, l *logger.Logger, rng *rand.Rand, h *mixedversion.Helper) error { From 1e0366915d65cff27f535d7862d1bdb83cc2114d Mon Sep 17 00:00:00 2001 From: Faizan Qazi Date: Mon, 20 Mar 2023 15:32:36 +0000 Subject: [PATCH 2/2] workload/schemachanger: allow errors if the descID generator unavailable Previously, when the randomized declarative schema changer was run during version upgrade we the descriptor ID generator can be temporarily unavailable between 22.2 and 23.1 This patch, allows this error temporarily for this workload. Epic: none Release note: None --- .../schemachange/operation_generator.go | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/pkg/workload/schemachange/operation_generator.go b/pkg/workload/schemachange/operation_generator.go index bb89630ab114..50633fb12845 100644 --- a/pkg/workload/schemachange/operation_generator.go +++ b/pkg/workload/schemachange/operation_generator.go @@ -1135,6 +1135,15 @@ func (og *operationGenerator) createSequence(ctx context.Context, tx pgx.Tx) (*o {code: pgcode.UndefinedSchema, condition: !schemaExists}, {code: pgcode.DuplicateRelation, condition: sequenceExists && !ifNotExists}, }.add(stmt.expectedExecErrors) + // Descriptor ID generator may be temporarily unavailable, so + // allow this to be detected. + potentialDescIDGeneratorError, err := maybeExpectPotentialDescIDGenerationError(ctx, tx) + if err != nil { + return nil, err + } + codesWithConditions{ + {code: pgcode.Uncategorized, condition: potentialDescIDGeneratorError}, + }.add(stmt.potentialExecErrors) var seqOptions tree.SequenceOptions // Decide if the sequence should be owned by a column. If so, it can @@ -1296,6 +1305,15 @@ func (og *operationGenerator) createTable(ctx context.Context, tx pgx.Tx) (*opSt {code: pgcode.FeatureNotSupported, condition: hasUnsupportedTSQuery}, {code: pgcode.FeatureNotSupported, condition: hasUnsupportedForwardQueries}, }.add(opStmt.potentialExecErrors) + // Descriptor ID generator may be temporarily unavailable, so + // allow uncategorized errors temporarily. + potentialDescIDGeneratorError, err := maybeExpectPotentialDescIDGenerationError(ctx, tx) + if err != nil { + return nil, err + } + codesWithConditions{ + {code: pgcode.Uncategorized, condition: potentialDescIDGeneratorError}, + }.add(opStmt.potentialExecErrors) opStmt.sql = tree.Serialize(stmt) return opStmt, nil } @@ -1314,6 +1332,15 @@ func (og *operationGenerator) createEnum(ctx context.Context, tx pgx.Tx) (*opStm {code: pgcode.DuplicateObject, condition: typeExists}, {code: pgcode.InvalidSchemaName, condition: !schemaExists}, }.add(opStmt.expectedExecErrors) + // Descriptor ID generator may be temporarily unavailable, so + // allow uncategorized errors temporarily. + potentialDescIDGeneratorError, err := maybeExpectPotentialDescIDGenerationError(ctx, tx) + if err != nil { + return nil, err + } + codesWithConditions{ + {code: pgcode.Uncategorized, condition: potentialDescIDGeneratorError}, + }.add(opStmt.potentialExecErrors) stmt := randgen.RandCreateType(og.params.rng, typName.Object(), "asdf") stmt.(*tree.CreateType).TypeName = typName.ToUnresolvedObjectName() opStmt.sql = tree.Serialize(stmt) @@ -1436,7 +1463,15 @@ func (og *operationGenerator) createTableAs(ctx context.Context, tx pgx.Tx) (*op {code: pgcode.DuplicateAlias, condition: duplicateSourceTables}, {code: pgcode.DuplicateColumn, condition: duplicateColumns}, }.add(opStmt.expectedExecErrors) - + // Descriptor ID generator may be temporarily unavailable, so + // allow uncategorized errors temporarily. + potentialDescIDGeneratorError, err := maybeExpectPotentialDescIDGenerationError(ctx, tx) + if err != nil { + return nil, err + } + codesWithConditions{ + {code: pgcode.Uncategorized, condition: potentialDescIDGeneratorError}, + }.add(opStmt.potentialExecErrors) // Confirm the select itself doesn't run into any column generation errors, // by executing it independently first until we add validation when adding // generated columns. See issue: #81698?, which will allow us to remove this @@ -1566,7 +1601,15 @@ func (og *operationGenerator) createView(ctx context.Context, tx pgx.Tx) (*opStm {code: pgcode.DuplicateAlias, condition: duplicateSourceTables}, {code: pgcode.DuplicateColumn, condition: duplicateColumns}, }.add(opStmt.expectedExecErrors) - + // Descriptor ID generator may be temporarily unavailable, so + // allow uncategorized errors temporarily. + potentialDescIDGeneratorError, err := maybeExpectPotentialDescIDGenerationError(ctx, tx) + if err != nil { + return nil, err + } + codesWithConditions{ + {code: pgcode.Uncategorized, condition: potentialDescIDGeneratorError}, + }.add(opStmt.potentialExecErrors) opStmt.sql = fmt.Sprintf(`CREATE VIEW %s AS %s`, destViewName, selectStatement.String()) return opStmt, nil @@ -3665,3 +3708,10 @@ func isFkConstraintsEnabled(ctx context.Context, tx pgx.Tx) (bool, error) { clusterversion.ByKey(clusterversion.TODODelete_V22_2Start)) return !fkConstraintDisabledVersion, err } + +func maybeExpectPotentialDescIDGenerationError(ctx context.Context, tx pgx.Tx) (bool, error) { + descIDGenerationVersion := clusterversion.ByKey(clusterversion.V23_1DescIDSequenceForSystemTenant) + descIDGenerationErrorPossible, err := isClusterVersionLessThan(ctx, + tx, descIDGenerationVersion) + return descIDGenerationErrorPossible, err +}