Skip to content

Commit

Permalink
VReplication: Do not delete sharded target vschema table entries on C…
Browse files Browse the repository at this point in the history
…ancel (#13146)

* Don't delete vschema table entries that VReplication does not create

Signed-off-by: Matt Lord <[email protected]>

* Update unit test accordingly

Signed-off-by: Matt Lord <[email protected]>

* Improve comment

Signed-off-by: Matt Lord <[email protected]>

* Adjust workflow complete unit test as well

Signed-off-by: Matt Lord <[email protected]>

* Limit this behavior change to Cancel

Signed-off-by: Matt Lord <[email protected]>

* Improve comment

Signed-off-by: Matt Lord <[email protected]>

---------

Signed-off-by: Matt Lord <[email protected]>
  • Loading branch information
mattlord authored May 25, 2023
1 parent 6ab9a67 commit f5a460c
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
9 changes: 8 additions & 1 deletion go/vt/wrangler/traffic_switcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,14 @@ func (ts *trafficSwitcher) dropParticipatingTablesFromKeyspace(ctx context.Conte
if err != nil {
return err
}
// VReplication does NOT create the vschema entries in SHARDED
// TARGET keyspaces -- as we cannot know the proper vindex
// definitions to use -- and we should not delete them either
// (on workflow Cancel) as the user must create them separately
// and they contain information about the vindex definitions, etc.
if vschema.Sharded && keyspace == ts.TargetKeyspaceName() {
return nil
}
for _, tableName := range ts.Tables() {
delete(vschema.Tables, tableName)
}
Expand Down Expand Up @@ -1774,7 +1782,6 @@ func (ts *trafficSwitcher) removeTargetTables(ctx context.Context) error {
}

return ts.dropParticipatingTablesFromKeyspace(ctx, ts.TargetKeyspaceName())

}

func (ts *trafficSwitcher) dropTargetShards(ctx context.Context) error {
Expand Down
16 changes: 14 additions & 2 deletions go/vt/wrangler/workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,20 @@ func TestMoveTablesV2Cancel(t *testing.T) {

require.True(t, checkIfTableExistInVSchema(ctx, t, wf.wr.ts, "ks1", "t1"))
require.True(t, checkIfTableExistInVSchema(ctx, t, wf.wr.ts, "ks1", "t2"))
require.False(t, checkIfTableExistInVSchema(ctx, t, wf.wr.ts, "ks2", "t1"))
require.False(t, checkIfTableExistInVSchema(ctx, t, wf.wr.ts, "ks2", "t2"))

// Should target vschema table entries be deleted upon Cancel. For unsharded
// keyspaces they should be as they are empty table entries that we also
// create when the workflow is Created.
targetVSchemaEntriesRemain := false
if len(tme.targetShards) > 1 {
// If the target keyspace is sharded -- which it is today in the test -- the
// vschema must be created by the user before the workflow is started. Thus
// we should also not delete the vschema table entries upon Cancel as the
// management of the sharded vschema is up to the user.
targetVSchemaEntriesRemain = true
}
require.Equal(t, targetVSchemaEntriesRemain, checkIfTableExistInVSchema(ctx, t, wf.wr.ts, "ks2", "t1"))
require.Equal(t, targetVSchemaEntriesRemain, checkIfTableExistInVSchema(ctx, t, wf.wr.ts, "ks2", "t2"))
}

func TestReshardV2(t *testing.T) {
Expand Down

0 comments on commit f5a460c

Please sign in to comment.