-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
roachtest: fix schemachange/mixed/tpcc for 19.1 #41079
roachtest: fix schemachange/mixed/tpcc for 19.1 #41079
Conversation
949980e
to
d4a96aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @lucy-zhang and @pbardea)
pkg/cmd/roachtest/schemachange.go, line 428 at r1 (raw file):
if err := runAndLogStmts(ctx, t, c, "mixed-schema-changes-19.1", []string{ `CREATE TABLE tpcc.orderpks (o_w_id, o_d_id, o_id, PRIMARY KEY(o_w_id, o_d_id, o_id));`, `INSERT INTO tpcc.orderpks SELECT o_w_id, o_d_id, o_id FROM tpcc.order LIMIT 1000;`,
What's the purpose of this LIMIT 1000
?
`schemachange/mixed/tpcc` uses `CREATE TABLE AS` in 19.2. This PR will have the test correctly create a similar table in 19.1 without using `CREATE TABLE AS`. Release justification: Fixes a test. Release note: None
d4a96aa
to
2c744d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @pbardea)
pkg/cmd/roachtest/schemachange.go, line 428 at r1 (raw file):
Previously, pbardea (Paul Bardea) wrote…
What's the purpose of this
LIMIT 1000
?
It's just to limit the number of rows to insert so we don't try to read and insert all 30 million rows of tpcc.order
in this transaction, since I figured that the tables don't have to be identical in 19.1 and 19.2. 1000 might be too conservative, though. Now that I think about it, the best option is probably to just use all the rows to get a similarly-sized table, but use AOST to reduce contention, so I changed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @pbardea)
TFTR bors r+ |
41079: roachtest: fix schemachange/mixed/tpcc for 19.1 r=lucy-zhang a=lucy-zhang `schemachange/mixed/tpcc` uses `CREATE TABLE AS` in 19.2. This PR will have the test correctly create a similar table in 19.1 without using `CREATE TABLE AS`. See #40935 (comment). Release justification: Fixes a test. Release note: None Co-authored-by: Lucy Zhang <lucy-zhang@users.noreply.github.com>
Build succeeded |
schemachange/mixed/tpcc
usesCREATE TABLE AS
in 19.2. This PR will have thetest correctly create a similar table in 19.1 without using
CREATE TABLE AS
.See #40935 (comment).
Release justification: Fixes a test.
Release note: None