Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workload/mixed-version/schemachanger: re-enable mixed version workload #87142

Merged
merged 3 commits into from
Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions pkg/cmd/roachtest/tests/mixed_version_schemachange.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ func registerSchemaChangeMixedVersions(r registry.Registry) {
// This tests the work done for 20.1 that made schema changes jobs and in
// addition prevented making any new schema changes on a mixed cluster in
// order to prevent bugs during upgrades.
Cluster: r.MakeClusterSpec(4),
Cluster: r.MakeClusterSpec(4),
NativeLibs: registry.LibGEOS,
Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
maxOps := 100
concurrency := 5
Expand Down Expand Up @@ -56,12 +57,6 @@ func runSchemaChangeWorkloadStep(loadNode, maxOps, concurrency int) versionStep
t.L().Printf("Workload step run: %d", numFeatureRuns)
runCmd := []string{
"./workload run schemachange --verbose=1",
// The workload is still in development and occasionally discovers schema
// change errors so for now we don't fail on them but only on panics, server
// crashes, deadlocks, etc.
// TODO(spaskob): remove when https://github.com/cockroachdb/cockroach/issues/47430
// is closed.
"--tolerate-errors=true",
fmt.Sprintf("--max-ops %d", maxOps),
fmt.Sprintf("--concurrency %d", concurrency),
fmt.Sprintf("{pgurl:1-%d}", u.c.Spec().NodeCount),
Expand Down
40 changes: 40 additions & 0 deletions pkg/workload/schemachange/operation_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,29 @@ func (og *operationGenerator) createTable(ctx context.Context, tx pgx.Tx) (*opSt
stmt := randgen.RandCreateTableWithColumnIndexNumberGenerator(og.params.rng, "table", tableIdx, databaseHasMultiRegion, og.newUniqueSeqNum)
stmt.Table = *tableName
stmt.IfNotExists = og.randIntn(2) == 0
trigramIsNotSupported, err := isClusterVersionLessThan(
ctx,
tx,
clusterversion.ByKey(clusterversion.TrigramInvertedIndexes))
if err != nil {
return nil, err
}
hasTrigramIdxUnsupported := func() bool {
if !trigramIsNotSupported {
return false
}
// Check if any of the indexes have trigrams involved.
for _, def := range stmt.Defs {
if idx, ok := def.(*tree.IndexTableDef); ok && idx.Inverted {
lastColumn := idx.Columns[len(idx.Columns)-1]
switch lastColumn.OpClass {
case "gin_trgm_ops", "gist_trgm_ops":
return true
}
}
}
return false
}()

tableExists, err := og.tableExists(ctx, tx, tableName)
if err != nil {
Expand All @@ -1197,6 +1220,11 @@ func (og *operationGenerator) createTable(ctx context.Context, tx pgx.Tx) (*opSt
{code: pgcode.DuplicateRelation, condition: tableExists && !stmt.IfNotExists},
{code: pgcode.UndefinedSchema, condition: !schemaExists},
}.add(opStmt.expectedExecErrors)
// Compatibility errors aren't guaranteed since the cluster version update is not
// fully transaction aware.
codesWithConditions{
{code: pgcode.FeatureNotSupported, condition: hasTrigramIdxUnsupported},
}.add(opStmt.potentialExecErrors)
opStmt.sql = tree.Serialize(stmt)
return opStmt, nil
}
Expand Down Expand Up @@ -2345,6 +2373,18 @@ func (og *operationGenerator) insertRow(ctx context.Context, tx pgx.Tx) (stmt *o
if err != nil {
return nil, err
}
// If we aren't on 22.2 then disable the insert plugin, since 21.X
// can have schema instrospection queries fail due to an optimizer bug.
skipInserts, err := isClusterVersionLessThan(ctx, tx, clusterversion.ByKey(clusterversion.Start22_2))
if err != nil {
return nil, err
}
// If inserts are to be skipped, we will intentionally, target the insert towards
// a non-existent table, so that they become no-ops.
if skipInserts {
tableExists = false
tableName.SchemaName = "InvalidObjectName"
Comment on lines +2385 to +2386
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does these two lines disable selects? In particular, why is the second line (tableName.SchemaName = "InvalidObjectName") needed?

}
if !tableExists {
return makeOpStmtForSingleError(OpStmtDML,
fmt.Sprintf(
Expand Down