Vtctldclient MoveTables by mattlord · Pull Request #13015 · vitessio/vitess

mattlord · 2023-05-02T23:00:55Z

Description

This work migrates the vtctl MoveTables client command to vtctldclient. In the process we also do some refactoring to eliminate at least some of the generation of SQL statements in vtctld which are sent to vttablets using the VReplicationExec tabletmanager RPC and instead call purpose-built tabletmanager RPCs so that the tablet controls the implementation and we eliminate upgrade/downgrade issues.

This PR also moves the MoveTables related steps in the local examples to vtctldclient. Finally, it also moves all MoveTables related steps in the endtoend tests to use vtctldclient except for the vreplication_v2 workflow (we will have the vtctlclient implementation around still for quite some time and need to test that as well).

Example Output

❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" Create --source-keyspace "commerce" --tables "customer,corder"
The following vreplication streams exist for workflow customer.commerce2customer:

id=1 on customer/zone1-201: Status: Copying. VStream Lag: 0s.

❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" --format text Cancel
Successfully cancelled the commerce2customer workflow in the customer keyspace


❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" --format json Create --source-keyspace "commerce" --tables "customer,corder"
{
  "table_copy_state": {},
  "shard_streams": {
    "customer/0": {
      "streams": [
        {
          "id": 2,
          "tablet": {
            "cell": "zone1",
            "uid": 201
          },
          "source_shard": "commerce/0",
          "position": "",
          "status": "Copying",
          "info": "VStream has not started"
        }
      ]
    }
  }
}

❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" --format json Cancel
{
  "summary": "Successfully cancelled the commerce2customer workflow in the customer keyspace",
  "details": [
    {
      "tablet": {
        "cell": "zone1",
        "uid": 201
      },
      "deleted": true
    }
  ]
}

❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" SwitchTraffic --dry-run
SwitchTraffic dry run results for workflow customer.commerce2customer at 10 Jul 23 16:54 UTC

Lock keyspace commerce
Switch reads for tables [corder,customer] to keyspace customer for tablet types [PRIMARY,REPLICA,RDONLY]
Routing rules for tables [corder,customer] will be updated
Unlock keyspace commerce
Lock keyspace commerce
Lock keyspace customer
Stop writes on keyspace commerce for tables [corder,customer]: [keyspace:commerce;shard:0;position:MySQL56/1f5a0d8c-1f42-11ee-8b4e-b10505092187:1-51]
Wait for vreplication on stopped streams to catchup for up to 30s
Create reverse vreplication workflow commerce2customer_reverse
Create journal entries on source databases
Enable writes on keyspace customer for tables [corder,customer]
Switch routing from keyspace commerce to keyspace customer
Routing rules for tables [corder,customer] will be updated
Switch writes completed, freeze and delete vreplication streams on: [tablet:201]
Start reverse vreplication streams on: [tablet:101]
Mark vreplication streams frozen on: [keyspace:customer;shard:0;tablet:201;workflow:commerce2customer;dbname:vt_customer]
Unlock keyspace customer
Unlock keyspace commerce


❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" --format json SwitchTraffic --dry-run
{
  "summary": "SwitchTraffic dry run results for workflow customer.commerce2customer at 10 Jul 23 16:57 UTC",
  "dry_run_results": [
    "Lock keyspace commerce",
    "Switch reads for tables [corder,customer] to keyspace customer for tablet types [PRIMARY,REPLICA,RDONLY]",
    "Routing rules for tables [corder,customer] will be updated",
    "Unlock keyspace commerce",
    "Lock keyspace commerce",
    "Lock keyspace customer",
    "Stop writes on keyspace commerce for tables [corder,customer]: [keyspace:commerce;shard:0;position:MySQL56/adfe5ffc-1f42-11ee-8b41-4c6d40f922ba:1-47]",
    "Wait for vreplication on stopped streams to catchup for up to 30s",
    "Create reverse vreplication workflow commerce2customer_reverse",
    "Create journal entries on source databases",
    "Enable writes on keyspace customer for tables [corder,customer]",
    "Switch routing from keyspace commerce to keyspace customer",
    "Routing rules for tables [corder,customer] will be updated",
    "Switch writes completed, freeze and delete vreplication streams on: [tablet:201]",
    "Start reverse vreplication streams on: [tablet:100]",
    "Mark vreplication streams frozen on: [keyspace:customer;shard:0;tablet:201;workflow:commerce2customer;dbname:vt_customer]",
    "Unlock keyspace customer",
    "Unlock keyspace commerce"
  ]
}

❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" SwitchTraffic --tablet-types "rdonly,replica"
SwitchTraffic was successful for workflow customer.commerce2customer

Start State: Reads Not Switched. Writes Not Switched
Current State: All Reads Switched. Writes Not Switched


❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" --format json SwitchTraffic --tablet-types primary
{
  "summary": "SwitchTraffic was successful for workflow customer.commerce2customer",
  "start_state": "All Reads Switched. Writes Not Switched",
  "current_state": "All Reads Switched. Writes Switched"
}

❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" Complete --dry-run
Complete dry run results for workflow customer.commerce2customer at 10 Jul 23 17:07 UTC

Lock keyspace commerce
Lock keyspace customer
Dropping these tables from the database and removing them from the vschema for keyspace commerce: [keyspace:commerce;shard:0;dbname:vt_commerce;tablet:100;table:corder,keyspace:commerce;shard:0;dbname:vt_commerce;tablet:100;table:customer]
Denied tables records on [corder,customer] will be removed from: [keyspace:commerce;shard:0;tablet:100]
Delete reverse vreplication streams on sources: [keyspace:commerce;shard:0;workflow:commerce2customer_reverse;dbname:vt_commerce;tablet:100]
Delete vreplication streams on targets: [keyspace:customer;shard:0;workflow:commerce2customer;dbname:vt_customer;tablet:201]
Routing rules for participating tables will be deleted
Unlock keyspace customer
Unlock keyspace commerce


❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" --format json Complete --dry-run
{
  "summary": "Complete dry run results for workflow customer.commerce2customer at 10 Jul 23 17:07 UTC",
  "dry_run_results": [
    "Lock keyspace commerce",
    "Lock keyspace customer",
    "Dropping these tables from the database and removing them from the vschema for keyspace commerce: [keyspace:commerce;shard:0;dbname:vt_commerce;tablet:100;table:corder,keyspace:commerce;shard:0;dbname:vt_commerce;tablet:100;table:customer]",
    "Denied tables records on [corder,customer] will be removed from: [keyspace:commerce;shard:0;tablet:100]",
    "Delete reverse vreplication streams on sources: [keyspace:commerce;shard:0;workflow:commerce2customer_reverse;dbname:vt_commerce;tablet:100]",
    "Delete vreplication streams on targets: [keyspace:customer;shard:0;workflow:commerce2customer;dbname:vt_customer;tablet:201]",
    "Routing rules for participating tables will be deleted",
    "Unlock keyspace customer",
    "Unlock keyspace commerce"
  ]
}

❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" Complete
Successfully completed the commerce2customer workflow in the customer keyspace


❯ vtctldclient MoveTables --workflow "commerce2customer" --target-keyspace "customer" --format json Complete
{
  "summary": "Successfully completed the commerce2customer workflow in the customer keyspace"
}

Related Issue(s)

Fixes: Migrate MoveTables Command to Vtctldclient #12153
Fixes: Migrate Workflow Command to Vtctldclient #12157
Related to: Tracking Issue: Replace VReplicationExec Command + RPC #12086
Related to: Compatibility issues between v11 and v12 for getting workflows #11535

Checklist

"Backport to:" labels have been added if this change should be back-ported
Tests were added or are not required
Did the new or modified tests pass consistently locally and on the CI
Documentation: Document https://github.com/vitessio/vitess/pull/13015 website#1540

Signed-off-by: Matt Lord <mattalord@gmail.com>

vitess-bot · 2023-05-02T23:01:00Z

Signed-off-by: Matt Lord <mattalord@gmail.com>

Next step is to try and remove wrangler.TrafficSwitcher and have the wrangler use the workflow implementation if possible. Signed-off-by: Matt Lord <mattalord@gmail.com>

Signed-off-by: Matt Lord <mattalord@gmail.com>

go/vt/topo/topoproto/tablet.go

rohit-nayak-ps · 2023-07-24T20:24:39Z

go/vt/vtctl/workflow/materializer.go

+	})
+}
+
+func Materialize(ctx context.Context, ts *topo.Server, tmc tmclient.TabletManagerClient, ms *vtctldatapb.MaterializeSettings) error {


We are not using this method now as part of this PR, right? And the related functions that use VReplicationExec etc? We will need to write updated functions for Materialize when we port over that method, right?

Correct. I moved it over with the other materializer functions -- many of which are used for various other workflow types. MoveTables uses most of them.

Yes, I do think we'll want to update the Materialize specific functions when we migrate that command.

rohit-nayak-ps · 2023-07-24T21:00:02Z

go/vt/vtctl/workflow/materializer_test.go

+}
+
+/*
+func TestMoveTablesV2(t *testing.T) {


Do we still need to port these commented tests?

I had left them in as a reminder for myself. I think that we have the main ones covered now and I added a ticket to the tracking issue to do a final pass to make sure we have good/equivalent coverage for things: #12152

I'll remove those commented tests from the file for now.

rohit-nayak-ps · 2023-07-24T21:31:56Z

go/vt/vtctl/workflow/server.go

+	var dryRunResults *[]string
+
+	if state.WorkflowType == TypeMigrate {
+		dryRunResults, err = s.finalizeMigrateWorkflow(ctx, req.TargetKeyspace, req.Workflow, strings.Join(ts.tables, ","),


The Migrate command is not yet ported over to vtctld, right? This code path will be exercised once we do that?

Correct. I started by copying existing wrangler code and then modifying it -- so I didn't see any point in removing this type of code as I expected that we'll be using it again shortly as we migrate the other commands over.

rohit-nayak-ps

Great work on this.

A huge step towards deprecating vtctlclient! ❤️

Signed-off-by: Matt Lord <mattalord@gmail.com>

ajm188

looking awesome!! everything i have is non-blocking, either because we'll ignore it (😂) or because it's minor changes

ajm188 · 2023-07-25T14:15:51Z

go/cmd/vtctldclient/command/movetables.go

+	MoveTablesShow = &cobra.Command{
+		Use:                   "show",
+		Short:                 "Show the details for a MoveTables VReplication workflow.",
+		Example:               `vtctldclient --server localhost:15999 movetables --workflow commerce2customer --target-keyspace customer show`,


these examples are still using the lowercase version of MoveTables

also ... is it going to be strange the parent command is BigCamelCase while the subcommand is alllowercase?

(i see they have aliases to their lowercase versions, but now my second question is "is it weird that the parent command is uppercase with alias to lowercase while the child is lowercase with alias to upper?")

I was using lowercase for the new commands but then when building the docs realized that ALL of the existing commands started with an uppercase letter. Otherwise I would have stuck with everything being lowercase. I thought that the current mix made it uniform while also supporting / showing all lowercase.

go/cmd/vtctldclient/command/workflows.go

go/cmd/vtctldclient/command/movetables.go

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord added 6 commits May 2, 2023 10:33

Add MoveTables RPC

dd6cac7

Signed-off-by: Matt Lord <mattalord@gmail.com>

More testing

baa26cf

Signed-off-by: Matt Lord <mattalord@gmail.com>

You guessed it

0f54fc8

Signed-off-by: Matt Lord <mattalord@gmail.com>

Get vtctldclient in sync with a dumb copy first

c3185cb

Signed-off-by: Matt Lord <mattalord@gmail.com>

Move to tabletmanager MoveTablesCreate RPC

ea8d4d6

Signed-off-by: Matt Lord <mattalord@gmail.com>

Move to tmc.CreateVRWorkflow

7542015

Signed-off-by: Matt Lord <mattalord@gmail.com>

vitess-bot bot added NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsWebsiteDocsUpdate What it says labels May 2, 2023

github-actions bot added this to the v17.0.0 milestone May 2, 2023

Add DeleteVRWorkflow RPC

16fe219

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the vtctldclient_movetables branch from fb7c442 to f31ff17 Compare May 2, 2023 23:53

Response improvements

0763cf1

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the vtctldclient_movetables branch from f31ff17 to 0763cf1 Compare May 3, 2023 00:26

Update vtctldclient --help output

c0df28b

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the vtctldclient_movetables branch 9 times, most recently from 0735046 to d7543f1 Compare May 3, 2023 14:38

Cleanup artifacts on workflow Cancel/Delete

28c1c84

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the vtctldclient_movetables branch from d7543f1 to 28c1c84 Compare May 3, 2023 14:39

mattlord added 3 commits May 4, 2023 14:38

Migrate TrafficSwitcher to workflow package.

6ddd57e

Next step is to try and remove wrangler.TrafficSwitcher and have the wrangler use the workflow implementation if possible. Signed-off-by: Matt Lord <mattalord@gmail.com>

Merge remote-tracking branch 'origin/main' into vtctldclient_movetables

f0dd0b2

Signed-off-by: Matt Lord <mattalord@gmail.com>

Getting Cancel/Delete artifact cleanup working

afe099d

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord force-pushed the vtctldclient_movetables branch from dfd2782 to 89b6b1f Compare May 6, 2023 17:02

mattlord removed the NeedsWebsiteDocsUpdate What it says label Jul 14, 2023

mattlord marked this pull request as ready for review July 14, 2023 20:14

mattlord requested a review from deepthi as a code owner July 14, 2023 20:14

Minor tweaks

c15cca2

Signed-off-by: Matt Lord <mattalord@gmail.com>

ajm188 mentioned this pull request Jul 17, 2023

fix docgen for subcommands #13518

Merged

4 tasks

mattlord added 14 commits July 17, 2023 12:33

Merge remote-tracking branch 'origin/main' into vtctldclient_movetables

cc1e925

Signed-off-by: Matt Lord <mattalord@gmail.com>

Migrate fixes from vitessio#13515

de4884e

Signed-off-by: Matt Lord <mattalord@gmail.com>

Merge remote-tracking branch 'origin/main' into vtctldclient_movetables

16559a9

Signed-off-by: Matt Lord <mattalord@gmail.com>

Align and unify command capitalization and examples

8f0bdfb

Signed-off-by: Matt Lord <mattalord@gmail.com>

Fix docgen back links

e1e19a9

Signed-off-by: Matt Lord <mattalord@gmail.com>

Use standard sentence punctuation for command Short output

f332d35

Signed-off-by: Matt Lord <mattalord@gmail.com>

Properly fix doxgen generation (after testing)

5ec944e

Signed-off-by: Matt Lord <mattalord@gmail.com>

Update vtctldclient help output

0aec5c7

Signed-off-by: Matt Lord <mattalord@gmail.com>

Merge remote-tracking branch 'origin/main' into vtctldclient_movetables

6a05959

Signed-off-by: Matt Lord <mattalord@gmail.com>

Merge remote-tracking branch 'origin/main' into vtctldclient_movetables

b12aa14

Signed-off-by: Matt Lord <mattalord@gmail.com>

Merge remote-tracking branch 'origin/main' into vtctldclient_movetables

6aba116

Signed-off-by: Matt Lord <mattalord@gmail.com>

Merge remote-tracking branch 'origin/main' into vtctldclient_movetables

7ff6ba4

Signed-off-by: Matt Lord <mattalord@gmail.com>

Revert incompatible existing proto msg change

84c7590

Signed-off-by: Matt Lord <mattalord@gmail.com>

Undo errant default value change

eeaf270

Signed-off-by: Matt Lord <mattalord@gmail.com>

rohit-nayak-ps reviewed Jul 25, 2023

View reviewed changes

rohit-nayak-ps approved these changes Jul 25, 2023

View reviewed changes

mattlord added 2 commits July 25, 2023 08:01

Address review comments

fe3db85

Signed-off-by: Matt Lord <mattalord@gmail.com>

Merge remote-tracking branch 'origin/main' into vtctldclient_movetables

4ddf930

Signed-off-by: Matt Lord <mattalord@gmail.com>

ajm188 approved these changes Jul 25, 2023

View reviewed changes

mattlord added 3 commits July 25, 2023 12:16

Minor changes after self review

b602cef

Signed-off-by: Matt Lord <mattalord@gmail.com>

Move format check before cli.FinishedParsing

ea3910a

Signed-off-by: Matt Lord <mattalord@gmail.com>

Add nil check and handling for issue seen

659f1cb

Signed-off-by: Matt Lord <mattalord@gmail.com>

mattlord mentioned this pull request Jul 25, 2023

Enhancing VTGate buffering for MoveTables and Shard by Shard Migration #13507

Merged

4 tasks

rohit-nayak-ps mentioned this pull request Sep 8, 2023

Implement Reshard in vtctldclient #13792

Merged

4 tasks

GrahamCampbell mentioned this pull request Aug 19, 2024

VReplication: Gather source positions once we know all writes are done during traffic switch #16572

Merged

5 tasks

Conversation

mattlord commented May 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

Checklist

Uh oh!

vitess-bot bot commented May 2, 2023

Review Checklist

General

If a new flag is being introduced:

If a workflow is added or modified:

Bug fixes

Non-trivial changes

New/Existing features

Backward compatibility

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattlord Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rohit-nayak-ps left a comment

Choose a reason for hiding this comment

Uh oh!

ajm188 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattlord Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mattlord commented May 2, 2023 •

edited

Loading

mattlord Jul 25, 2023 •

edited

Loading

mattlord Jul 25, 2023 •

edited

Loading