[vtctld] Add v0 GetWorkflows rpc and workflow/vexec packages #7575

ajm188 · 2021-03-01T19:41:40Z

Description

This adds an initial attempt at a GetWorkflows rpc to VtctldServer, by way of introducing new vtctl/workflow and vtctl/workflow/vexec packages to eventually share more code between the legacy and new vtctl(d) servers. I also made a more committed effort on that front by reimplementing wrangler's ListWorkflows function using the new vexec.

`package go/vt/vtctl/workflow`

This provides the actual implementation of GetWorkflows. I'm debating of separating the GetWorkflows rpc into a separate "service" in the protobuf file, because it seems weird to me that workflow.Server provides the actual implementation. I think this would look something like:

service Workflow {
  rpc GetWorkflows(vtctldata.GetWorkflowsRequest) returns (vtctldata.GetWorkflowsResponse) {};
}

// vtctl/grpcvtctldserver/server.go

func StartServer(s *grpc.Server, ts *topo.Server) {
	vtctld := NewVtctldServer(ts)
	vtctlservicepb.RegisterWorkflowServer(s, vtctld)
	vtctlservicepb.RegisterVtctldServer(s, vtctld)
}

// vtctl/workflow/server.go

type Server struct { ... }

var _ vtctlservicepb.WorkflowServer = (*Server)(nil)

This would allow us to assert that workflow.Server correctly implements the entirety of the Workflow rpcs, without also needing to implement the rest of VtctldServer by separating out the concerns. But then registering is maybe weird, so idk. All perspectives welcome!!

`package go/vt/vtctl/workflow/vexec`

This attempts to create a more ... encapsulated vexec API by more cleanly separating the query parsing/planning/rewriting phase from the execution phase. Eventually this should include all the functionality of wrangler's runVexec, but not yet, because that was a lot.

Usage

I got a Reshard running by running up through 301_reshard.sh of the local example, and then:

New CLI / code

❯ vtctldclient --server "localhost:15999" GetWorkflows customer
{
  "workflows": [
    {
      "name": "cust2cust",
      "source": {
        "keyspace": "customer",
        "shards": [
          "0"
        ]
      },
      "target": {
        "keyspace": "customer",
        "shards": [
          "-80",
          "80-"
        ]
      },
      "max_v_replication_lag": "1",
      "shard_statuses": {
        "-80/zone1-0000000300": {
          "primary_replication_statuses": [
            {
              "id": "1",
              "shard": "-80",
              "tablet": {
                "cell": "zone1",
                "uid": 300
              },
              "binlog_source": {
                "keyspace": "customer",
                "shard": "0",
                "tablet_type": "UNKNOWN",
                "key_range": null,
                "tables": [
                ],
                "filter": {
                  "rules": [
                    {
                      "match": "/.*",
                      "filter": "-80"
                    }
                  ],
                  "fieldEventMode": "ERR_ON_MISMATCH"
                },
                "on_ddl": "IGNORE",
                "external_mysql": "",
                "stop_after_copy": false
              },
              "position": "MySQL56/715d1016-7ab7-11eb-b361-248943b3b987:1-38",
              "stop_position": "",
              "state": "Running",
              "db_name": "vt_customer",
              "transaction_timestamp": {
                "seconds": "0",
                "nanoseconds": 0
              },
              "time_updated": {
                "seconds": "1614622642",
                "nanoseconds": 0
              },
              "message": "",
              "copy_states": [
              ]
            }
          ],
          "tablet_controls": [
          ],
          "is_primary_serving": false
        },
        "80-/zone1-0000000400": {
          "primary_replication_statuses": [
            {
              "id": "1",
              "shard": "80-",
              "tablet": {
                "cell": "zone1",
                "uid": 400
              },
              "binlog_source": {
                "keyspace": "customer",
                "shard": "0",
                "tablet_type": "UNKNOWN",
                "key_range": null,
                "tables": [
                ],
                "filter": {
                  "rules": [
                    {
                      "match": "/.*",
                      "filter": "80-"
                    }
                  ],
                  "fieldEventMode": "ERR_ON_MISMATCH"
                },
                "on_ddl": "IGNORE",
                "external_mysql": "",
                "stop_after_copy": false
              },
              "position": "MySQL56/715d1016-7ab7-11eb-b361-248943b3b987:1-38",
              "stop_position": "",
              "state": "Running",
              "db_name": "vt_customer",
              "transaction_timestamp": {
                "seconds": "0",
                "nanoseconds": 0
              },
              "time_updated": {
                "seconds": "1614622642",
                "nanoseconds": 0
              },
              "message": "",
              "copy_states": [
              ]
            }
          ],
          "tablet_controls": [
          ],
          "is_primary_serving": false
        }
      }
    }
  ]
}

Old CLI / code

❯ vtctlclient -server "localhost:15999" Workflow customer.cust2cust show
{
        "Workflow": "cust2cust",
        "SourceLocation": {
                "Keyspace": "customer",
                "Shards": [
                        "0"
                ]
        },
        "TargetLocation": {
                "Keyspace": "customer",
                "Shards": [
                        "-80",
                        "80-"
                ]
        },
        "MaxVReplicationLag": 0,
        "ShardStatuses": {
                "-80/zone1-0000000300": {
                        "MasterReplicationStatuses": [
                                {
                                        "Shard": "-80",
                                        "Tablet": "zone1-0000000300",
                                        "ID": 1,
                                        "Bls": {
                                                "keyspace": "customer",
                                                "shard": "0",
                                                "filter": {
                                                        "rules": [
                                                                {
                                                                        "match": "/.*",
                                                                        "filter": "-80"
                                                                }
                                                        ]
                                                }
                                        },
                                        "Pos": "MySQL56/715d1016-7ab7-11eb-b361-248943b3b987:1-38",
                                        "StopPos": "",
                                        "State": "Running",
                                        "DBName": "vt_customer",
                                        "TransactionTimestamp": 0,
                                        "TimeUpdated": 1614631626,
                                        "Message": "",
                                        "CopyState": null
                                }
                        ],
                        "TabletControls": null,
                        "MasterIsServing": false
                },
                "80-/zone1-0000000400": {
                        "MasterReplicationStatuses": [
                                {
                                        "Shard": "80-",
                                        "Tablet": "zone1-0000000400",
                                        "ID": 1,
                                        "Bls": {
                                                "keyspace": "customer",
                                                "shard": "0",
                                                "filter": {
                                                        "rules": [
                                                                {
                                                                        "match": "/.*",
                                                                        "filter": "80-"
                                                                }
                                                        ]
                                                }
                                        },
                                        "Pos": "MySQL56/715d1016-7ab7-11eb-b361-248943b3b987:1-38",
                                        "StopPos": "",
                                        "State": "Running",
                                        "DBName": "vt_customer",
                                        "TransactionTimestamp": 0,
                                        "TimeUpdated": 1614631626,
                                        "Message": "",
                                        "CopyState": null
                                }
                        ],
                        "TabletControls": null,
                        "MasterIsServing": false
                }
        }
}

Related Issue(s)

[vtctld] Add GetWorkflows rpc to VtctldServer #7574

Checklist

Should this PR be backported? No
Tests were added or are not required
- I definitely deferred testing some things to try to get this reviewed. I'll definitely come back for these!!
Documentation was added or is not required

Follow-up Tasks

File issues to add additional tests
File issues to port more functionality from wrangler vexec to this vexec, and backwards-reimplement existing APIs

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

ajm188 · 2021-03-01T20:48:13Z

go/vt/vtctl/grpcvtctldserver/testutil/test_tmclient.go

 	WaitForPositionDelays map[string]time.Duration
 	// keyed by tablet alias. injects a sleep to the end of the function
 	// regardless of parent context timeout or error result.
 	WaitForPositionPostDelays map[string]time.Duration
 	// WaitForPosition(tablet *topodatapb.Tablet, position string) error, so we
 	// key by tablet alias and then by position.
 	WaitForPositionResults map[string]map[string]error
-	// keyed by tablet alias.


Just sorting these, I briefly forgot how the alphabet worked at some point.

ajm188 · 2021-03-01T20:49:16Z

go/vt/vtctl/workflow/server.go

+//
+// It has the same signature as the vtctlservicepb.VtctldServer's GetWorkflows
+// rpc, and grpcvtctldserver delegates to this function.
+func (s *Server) GetWorkflows(ctx context.Context, req *vtctldatapb.GetWorkflowsRequest) (*vtctldatapb.GetWorkflowsResponse, error) {


I'm deferring testing this because I want to start iterating with this in vtadmin. I promise to file issues and come to write tests for these.

ajm188 · 2021-03-01T20:50:52Z

proto/vtctldata.proto

@@ -41,6 +42,86 @@ message ExecuteVtctlCommandResponse {
  logutil.Event event = 1;
 }

+// TableMaterializeSttings contains the settings for one table.
+message TableMaterializeSettings {


No changes here, just moving things around in the file. I'll need to resync after ~~#7562~~ edit: this PR

ajm188 · 2021-03-01T20:51:27Z

proto/vtctldata.proto

+  ReplicationLocation source = 2;
+  ReplicationLocation target = 3;
+  int64 max_v_replication_lag = 4;
+  map<string, ShardReplicationStatus> shard_statuses = 5;


Suggestion from @rohit-nayak-ps to rename these from ReplicationStatus to Stream which I like much better ✨

ajm188 · 2021-03-01T20:57:21Z

go/vt/vtctl/workflow/vexec/query_planner.go

+
+// VReplicationQueryPlanner implements the QueryPlanner interface for queries on
+// the _vt.vreplication table.
+type VReplicationQueryPlanner struct {


A lot of the stuff in vexec is a refactoring of the existing query planner code in go/vt/wrangler/{vexec,vexec_plan}.go in a publicly exported and reusable form.

ajm188 · 2021-03-02T15:46:34Z

proto/vtctldata.proto

+    bool is_primary_serving = 3;
+  }
+
+  message Stream {


@rohit-nayak-ps let me know what you think of the new naming

This is good!

Signed-off-by: Andrew Mason <amason@slack-corp.com>

… everything Signed-off-by: Andrew Mason <amason@slack-corp.com>

This will require a significant amount of polish, testing, and documentation. I'll also need to see if it even works at all with how wrangler currently does vexec queries. Signed-off-by: Andrew Mason <amason@slack-corp.com>

Signed-off-by: Andrew Mason <amason@slack-corp.com>

…ents Signed-off-by: Andrew Mason <amason@slack-corp.com>

This is to support individual workflow queries, instead of just bulk queries on multiple workflows (i.e. `GetWorkflow` vs `GetWorkflows`, or `workflow ks.workflowname show` in the old API) Signed-off-by: Andrew Mason <amason@slack-corp.com>

Signed-off-by: Andrew Mason <amason@slack-corp.com>

…s/logs Signed-off-by: Andrew Mason <amason@slack-corp.com>

This matches the old implementation, based on the constant values that are always set for that version's planner.params() function. Note also that in the old implementation, `planner.params().insertTemplates == nil`, so `buildInsertPlan` would always return error, which is why we don't even bother to define a function for it here. Only schema migration workflows seem to support inserts. Signed-off-by: Andrew Mason <amason@slack-corp.com>

…up them Signed-off-by: Andrew Mason <amason@slack-corp.com>

Signed-off-by: Andrew Mason <amason@slack-corp.com>

…neric than "Manager" Signed-off-by: Andrew Mason <amason@slack-corp.com>

Signed-off-by: Andrew Mason <amason@slack-corp.com>

rohit-nayak-ps

very nice

rohit-nayak-ps · 2021-03-04T12:05:10Z

go/vt/vtctl/workflow/doc.go

+	vexec / queryplanner APIs to fit with the QueryParams thing that wrangler
+	uses, which _should_ work, but who knows?? Time will tell.
+*/
+package workflow


Not entirely sure what you mean by vexec workflows vs the others. If you mean there will be both wrangler and vexec based functionality, that is correct atm. We have "delegated" some APIs like Workflow to VExec since they are common to all vrep workflow types (MoveTables/Reshard/Materialize).

With V2 workflows some of the Workflow command functionality is provided under the umbrella MoveTables/Reshard commands. There is still a little bit of work to do, but we will be in a position to deprecate the Workflow command once completed. I am still a bit ambivalent about a deprecation since Workflow-functionalities work across all types of workflow and might be more user-friendly: the user doesn't need to know which type of workflow they are operating on. I am also going to work soon on the ability to update the BinlogSource attribute to make it easier to setup custom Materialize workflows instead of VReplicationExec based scripting.

That said, VExec is pretty powerful for arbitrary operations on workflows and we may always need it.

nvm, I see from the code below what you mean by vexec workflows ... Leaving the ramblings above as an fyi ;-)

Yeah, this is definitely not as clear as it could be; I'd prefer you didn't have to read all the code to understand what this is trying to say 😅

I can definitely try to clean this up in a later PR (probably when I add the GetWorkflow), as I want to get this merged soon-ish to unblock us in vtadmin-land to start working with this API.

rohit-nayak-ps · 2021-03-04T12:05:34Z

go/vt/vtctl/grpcvtctldserver/testutil/test_tmclient.go

+	}
+
+	return nil, assert.AnError
+}


rohit-nayak-ps · 2021-03-04T15:06:29Z

go/vt/vtctl/workflow/vexec/query_planner.go

+	// method called something like "VReplicationExec(ctx, query, Options{DryRun: true})"
+	// DryRun(ctx context.Context) error
+
+	// PlanQuery contsructs and returns a QueryPlan for a given statement. The


typo: contsructs => constructs

Signed-off-by: Andrew Mason <amason@slack-corp.com>

[vtctld] Add v0 GetWorkflows rpc and workflow/vexec packages Signed-off-by: Andrew Mason <amason@slack-corp.com> Conflicts: go/vt/proto/vtctldata/vtctldata.pb.go go/vt/proto/vtctlservice/vtctlservice.pb.go go/vt/vtctl/grpcvtctldclient/client_gen.go go/vt/vtctl/grpcvtctldserver/server.go go/vt/wrangler/vexec.go proto/vtctldata.proto proto/vtctlservice.proto

@rafael

Note (@rafael): This one required a bunch of manual intervention [vtctld] Add v0 GetWorkflows rpc and workflow/vexec packages Signed-off-by: Rafael Chacon <rafael@slack-corp.com>

ajm188 requested review from rafael, doeg, setassociative and rohit-nayak-ps March 1, 2021 19:41

ajm188 added this to In progress in Vtctld Service via automation Mar 1, 2021

ajm188 commented Mar 1, 2021

View reviewed changes

ajm188 linked an issue Mar 2, 2021 that may be closed by this pull request

[vtctld] Add GetWorkflows rpc to VtctldServer #7574

Closed

ajm188 commented Mar 2, 2021

View reviewed changes

ajm188 added 19 commits March 2, 2021 18:53

Organize vtctldata.proto and regenerate

59b1a2c

Signed-off-by: Andrew Mason <amason@slack-corp.com>

wip - add GetWorkflows rpc, with some TODOs for later, and generate…

d0fdcca

… everything Signed-off-by: Andrew Mason <amason@slack-corp.com>

wip - first stab at a workflow/vexec API outside of wrangler

0680c19

This will require a significant amount of polish, testing, and documentation. I'll also need to see if it even works at all with how wrangler currently does vexec queries. Signed-off-by: Andrew Mason <amason@slack-corp.com>

wip - move some files around, trying to get closer to good boundaries

bf9a306

Signed-off-by: Andrew Mason <amason@slack-corp.com>

wip - add workflow.Manager to VtctldServer, add CLI entrypoint

40b8fef

Signed-off-by: Andrew Mason <amason@slack-corp.com>

[vtctl/workflow] wip - touch up error handling, add more designy comm…

2ccf392

…ents Signed-off-by: Andrew Mason <amason@slack-corp.com>

[vtctl/vexec] wip - set timeouts on topo calls

52d4a09

Signed-off-by: Andrew Mason <amason@slack-corp.com>

[vexec] wip - thread workflow name through to QueryPlan, update error…

3381daf

…s/logs Signed-off-by: Andrew Mason <amason@slack-corp.com>

[vexec] wip - move error definitions to files that use them most, gro…

77868fc

…up them Signed-off-by: Andrew Mason <amason@slack-corp.com>

[wrangler] wip - rewrite ListAllWorkflows in wrangler to use new vexec

6c9dfd0

Signed-off-by: Andrew Mason <amason@slack-corp.com>

[vexec] refactors and all the docs

630c037

Signed-off-by: Andrew Mason <amason@slack-corp.com>

[vexec] Move QueryPlan and methods to its own file

ed6136b

Signed-off-by: Andrew Mason <amason@slack-corp.com>

[vexec] Add VReplicationQueryPlanner tests

e2fda5c

Signed-off-by: Andrew Mason <amason@slack-corp.com>

[vexec] Add tests for QueryPlans

105160f

Signed-off-by: Andrew Mason <amason@slack-corp.com>

[workflow] Document Manager, sort of

8b49191

Signed-off-by: Andrew Mason <amason@slack-corp.com>

[workflow] Rename, I decided I dislike "Server" slightly less as a ge…

358cfa3

…neric than "Manager" Signed-off-by: Andrew Mason <amason@slack-corp.com>

[workflow] Rename (Shard)ReplicationStatus => (Shard)Stream

098ac4f

Signed-off-by: Andrew Mason <amason@slack-corp.com>

ajm188 force-pushed the am_vtctld_workflows branch from 231d794 to 098ac4f Compare March 3, 2021 00:01

ajm188 marked this pull request as ready for review March 3, 2021 00:48

ajm188 requested review from deepthi and sougou as code owners March 3, 2021 00:48

rohit-nayak-ps approved these changes Mar 4, 2021

View reviewed changes

fix typo

30f4750

Signed-off-by: Andrew Mason <amason@slack-corp.com>

rohit-nayak-ps merged commit 7a5e4fe into vitessio:master Mar 4, 2021

Vtctld Service automation moved this from In progress to Done Mar 4, 2021

ajm188 deleted the am_vtctld_workflows branch March 4, 2021 16:31

ajm188 mentioned this pull request Mar 5, 2021

Slack vitess 2021.01.11 release tinyspeck/vitess#193

Closed

rafael mentioned this pull request Apr 6, 2021

[vtctld do not merge] Custom build for vtctld development (cherry-picks on top of 9.0) tinyspeck/vitess#204

Closed

ajm188 added Component: Cluster management Component: VReplication Type: Enhancement Logical improvement (somewhere between a bug and feature) labels May 23, 2021

ajm188 mentioned this pull request Jul 15, 2021

slack vitess v10.pre tinyspeck/vitess#228

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vtctld] Add v0 GetWorkflows rpc and workflow/vexec packages #7575

[vtctld] Add v0 GetWorkflows rpc and workflow/vexec packages #7575

ajm188 commented Mar 1, 2021 •

edited

ajm188 Mar 1, 2021

ajm188 Mar 1, 2021

ajm188 Mar 1, 2021 •

edited

ajm188 Mar 1, 2021

ajm188 Mar 1, 2021

ajm188 Mar 2, 2021

rohit-nayak-ps Mar 4, 2021

rohit-nayak-ps left a comment

rohit-nayak-ps Mar 4, 2021

rohit-nayak-ps Mar 4, 2021

ajm188 Mar 4, 2021 •

edited

rohit-nayak-ps Mar 4, 2021

rohit-nayak-ps Mar 4, 2021

[vtctld] Add v0 GetWorkflows rpc and workflow/vexec packages #7575

[vtctld] Add v0 GetWorkflows rpc and workflow/vexec packages #7575

Conversation

ajm188 commented Mar 1, 2021 • edited

Description

package go/vt/vtctl/workflow

package go/vt/vtctl/workflow/vexec

Usage

Related Issue(s)

Checklist

Follow-up Tasks

Deployment Notes

Impacted Areas in Vitess

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajm188 Mar 1, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohit-nayak-ps left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajm188 Mar 4, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajm188 commented Mar 1, 2021 •

edited

`package go/vt/vtctl/workflow`

`package go/vt/vtctl/workflow/vexec`

ajm188 Mar 1, 2021 •

edited

ajm188 Mar 4, 2021 •

edited