workload/mixed-version/schemachanger: re-enable mixed version workload #87142
Conversation
|
This change is |
| @@ -2488,7 +2498,6 @@ func (og *operationGenerator) insertRow(ctx context.Context, tx pgx.Tx) (stmt *o | |||
| for _, row := range rows { | |||
| formattedRows = append(formattedRows, fmt.Sprintf("(%s)", strings.Join(row, ","))) | |||
| } | |||
|
|
|||
| tableExists = false | ||
| tableName.SchemaName = "InvalidObjectName" |
There was a problem hiding this comment.
How does these two lines disable selects? In particular, why is the second line (tableName.SchemaName = "InvalidObjectName") needed?
| @@ -64,7 +64,7 @@ func runSchemaChangeWorkloadStep(loadNode, maxOps, concurrency int) versionStep | |||
| // crashes, deadlocks, etc. | |||
| // TODO(spaskob): remove when https://github.com/cockroachdb/cockroach/issues/47430 | |||
| // is closed. | |||
| "--tolerate-errors=true", | |||
| //"--tolerate-errors=true", | |||
There was a problem hiding this comment.
why do we want to comment out this line? It seems necessary.
fqazi
force-pushed
the
mixedVersion
branch
2 times, most recently
from
305ff0c
to
d991a87
Compare
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @Xiang-Gu)
pkg/cmd/roachtest/tests/mixed_version_schemachange.go line 67 at r3 (raw file):
Previously, Xiang-Gu (Xiang Gu) wrote…
why do we want to comment out this line? It seems necessary.
Done.
pkg/workload/schemachange/operation_generator.go line 2384 at r3 (raw file):
Previously, Xiang-Gu (Xiang Gu) wrote…
How does these two lines disable selects? In particular, why is the second line (
tableName.SchemaName = "InvalidObjectName") needed?
Done.
So, the second line basically makes the inserts always target a non-existent table to skip all the logic.
|
@Xiang-Gu Can you take another look at this, if you have the cycles? |
|
Build failed (retrying...): |
|
bors r- @fqazi we recently changed the native library API for roachtests -- see #86451. Roachtests shouldn't call Cancelling the build here to let you rebase against master and fix the compilation issue. |
|
Canceled. |
…ployed Previously, the schemachanger mixed version workload did not deploy the libGEOS libraries causing operations to fail. To address, this patch will update the frontend to upload these binaries. Release justification: no real risk improves test coverage Release note: None merge back a<pkg>: <short description - lowercase, no final period> <what was there before: Previously, ...> <why it needed to change: This was inadequate because ...> <what you did about it: To address this, this patch ...>
… state Previously, the schema changer workload in a mixed version state attempted to use trigram indexes against 22.1, which is unsupported. This patch adds code to detect a mixed version state and expects the appropriate error when this occurs. Release justification: no risk improves test coverage Release note: None
Previously, if we ran in a mixed version state with the schema changer workload we could run into an optimizer bug (cockroachdb#80820). To address this, this patch in a mixed version workload disables the insert portion of the workload. Release justification: improves test coverage by enabling the mixed version test Release note: None
|
@renatolabs Thanks. Missed that change, I'll confirm that CI is happy then bors this again. |
|
bors r+ |
|
Build succeeded: |
Fixes: #58489 #87477
Previously the mixed version schema changer workload was disabled because of the lack of version gates. These changes will do the following:
Release justification: low risk only extends test coverage