-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
MoveTables: update vschema while moving tables with autoincrement from sharded to unsharded #9288
Merged
rohit-nayak-ps
merged 4 commits into
vitessio:main
from
planetscale:rn-movetables-vschema-sequence
Dec 15, 2021
Merged
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
3372287
Set sequence if defined for a table in the vschema when moving tables…
rohit-nayak-ps 9ac5266
Do not copy sequence info for a migrate workflow
rohit-nayak-ps cab2a7e
Address review comments
rohit-nayak-ps dd042cf
Address review comments
rohit-nayak-ps File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,8 @@ import ( | |
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/assert" | ||
|
||
"vitess.io/vitess/go/vt/log" | ||
|
||
"vitess.io/vitess/go/vt/wrangler" | ||
|
@@ -244,6 +246,101 @@ func TestBasicV2Workflows(t *testing.T) { | |
log.Flush() | ||
} | ||
|
||
func waitForWorkflowToStart(t *testing.T, ksWorkflow string) { | ||
done := false | ||
ticker := time.NewTicker(10 * time.Millisecond) | ||
log.Infof("Waiting for workflow %s to start", ksWorkflow) | ||
for { | ||
select { | ||
case <-ticker.C: | ||
if done { | ||
return | ||
} | ||
output, err := vc.VtctlClient.ExecuteCommandWithOutput("Workflow", ksWorkflow, "show") | ||
require.NoError(t, err) | ||
if strings.Contains(output, "\"State\": \"Running\"") { | ||
done = true | ||
log.Infof("Workflow %s has started", ksWorkflow) | ||
} | ||
case <-time.After(5 * time.Second): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor nit, any reason to have the timeout this low? We risk causing the entire action/test to fail. I'm not sure more time is really needed, just curious. We could also make the wait time a parameter. Feel free to ignore. |
||
require.FailNow(t, "workflow %s not yet started", ksWorkflow) | ||
} | ||
} | ||
} | ||
|
||
/* | ||
testVSchemaForSequenceAfterMoveTables checks that the related sequence tag is migrated correctly in the vschema | ||
while moving a table with an auto-increment from sharded to unsharded. | ||
*/ | ||
func testVSchemaForSequenceAfterMoveTables(t *testing.T) { | ||
// at this point the unsharded product and sharded customer keyspaces are created by previous tests | ||
|
||
// use MoveTables to move customer2 from product to customer using | ||
currentWorkflowType = wrangler.MoveTablesWorkflow | ||
err := tstWorkflowExec(t, defaultCellName, "wf2", sourceKs, targetKs, | ||
"customer2", workflowActionCreate, "", "", "") | ||
require.NoError(t, err) | ||
|
||
waitForWorkflowToStart(t, "customer.wf2") | ||
|
||
err = tstWorkflowExec(t, defaultCellName, "wf2", sourceKs, targetKs, | ||
"", workflowActionSwitchTraffic, "", "", "") | ||
require.NoError(t, err) | ||
err = tstWorkflowExec(t, defaultCellName, "wf2", sourceKs, targetKs, | ||
"", workflowActionComplete, "", "", "") | ||
require.NoError(t, err) | ||
|
||
// sanity check | ||
output, err := vc.VtctlClient.ExecuteCommandWithOutput("GetVSchema", "product") | ||
require.NoError(t, err) | ||
assert.NotContains(t, output, "customer2\"", "customer2 still found in keyspace product") | ||
validateCount(t, vtgateConn, "customer", "customer2", 3) | ||
|
||
// check that customer2 has the sequence tag | ||
output, err = vc.VtctlClient.ExecuteCommandWithOutput("GetVSchema", "customer") | ||
require.NoError(t, err) | ||
assert.Contains(t, output, "\"sequence\": \"customer_seq2\"", "customer2 sequence missing in keyspace customer") | ||
|
||
// ensure sequence is available to vtgate | ||
num := 5 | ||
for i := 0; i < num; i++ { | ||
execVtgateQuery(t, vtgateConn, "customer", "insert into customer2(name) values('a')") | ||
} | ||
validateCount(t, vtgateConn, "customer", "customer2", 3+num) | ||
want := fmt.Sprintf("[[INT32(%d)]]", 100+num-1) | ||
validateQuery(t, vtgateConn, "customer", "select max(cid) from customer2", want) | ||
|
||
// use MoveTables to move customer2 back to product. Note that now the table has an associated sequence | ||
err = tstWorkflowExec(t, defaultCellName, "wf3", targetKs, sourceKs, | ||
"customer2", workflowActionCreate, "", "", "") | ||
require.NoError(t, err) | ||
waitForWorkflowToStart(t, "product.wf3") | ||
err = tstWorkflowExec(t, defaultCellName, "wf3", targetKs, sourceKs, | ||
"", workflowActionSwitchTraffic, "", "", "") | ||
require.NoError(t, err) | ||
err = tstWorkflowExec(t, defaultCellName, "wf3", targetKs, sourceKs, | ||
"", workflowActionComplete, "", "", "") | ||
require.NoError(t, err) | ||
|
||
// sanity check | ||
output, err = vc.VtctlClient.ExecuteCommandWithOutput("GetVSchema", "product") | ||
require.NoError(t, err) | ||
assert.Contains(t, output, "customer2\"", "customer2 not found in keyspace product ") | ||
|
||
// check that customer2 still has the sequence tag | ||
output, err = vc.VtctlClient.ExecuteCommandWithOutput("GetVSchema", "product") | ||
require.NoError(t, err) | ||
assert.Contains(t, output, "\"sequence\": \"customer_seq2\"", "customer2 still found in keyspace product") | ||
|
||
// ensure sequence is available to vtgate | ||
for i := 0; i < num; i++ { | ||
execVtgateQuery(t, vtgateConn, "product", "insert into customer2(name) values('a')") | ||
} | ||
validateCount(t, vtgateConn, "product", "customer2", 3+num+num) | ||
rohit-nayak-ps marked this conversation as resolved.
Show resolved
Hide resolved
|
||
want = fmt.Sprintf("[[INT32(%d)]]", 100+num+num-1) | ||
validateQuery(t, vtgateConn, "product", "select max(cid) from customer2", want) | ||
} | ||
|
||
func testReshardV2Workflow(t *testing.T) { | ||
currentWorkflowType = wrangler.ReshardWorkflow | ||
|
||
|
@@ -276,7 +373,9 @@ func testMoveTablesV2Workflow(t *testing.T) { | |
output, _ := vc.VtctlClient.ExecuteCommandWithOutput(listAllArgs...) | ||
require.Contains(t, output, "No workflows found in keyspace customer") | ||
|
||
createMoveTablesWorkflow(t, "customer2") | ||
testVSchemaForSequenceAfterMoveTables(t) | ||
|
||
createMoveTablesWorkflow(t, "tenant") | ||
output, _ = vc.VtctlClient.ExecuteCommandWithOutput(listAllArgs...) | ||
require.Contains(t, output, "Following workflow(s) found in keyspace customer: wf1") | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Minor nit, maybe better to use a RegEx or JSON parsing so that we are checking for what we really care about while resilient to whitespace and other minor format changes? Full JSON parsing is probably overkill, maybe RegEx is too :-)
Feel free to ignore.