-
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
Add Workflow Update Client Command #12622
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. 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
|
Signed-off-by: Matt Lord <mattalord@gmail.com>
7ed8003
to
5ecb846
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
d6cc710
to
c204502
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
365f96b
to
771a96c
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Thank you for the very helpful review @ajm188 ! I believe that I've addressed all of your comments now. |
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
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.
one question about the any
, but i'll approve in case there's no follow-up needed
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.
just formally 👍-ing again
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
Signed-off-by: Matt Lord <mattalord@gmail.com>
I confirmed that we're using nil as the default generally in the vtctldclient code. Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
It worked, but the way that the error handling was done made it seem like it failed (it ONLY showed the errors from the source primary tablets). Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
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.
added tests lgtm as well
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.
e2e test changes look good.
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
This leaves us with a singular place where this is done throughout the client code and allows us to uniformly change it if needed in the future. Signed-off-by: Matt Lord <mattalord@gmail.com>
defc9a1
to
7a83b56
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.
🚢 🚢 🚢
* Document changes from vitessio/vitess#12622 Signed-off-by: Matt Lord <mattalord@gmail.com> * Update vtctldclient docs via: go run ./tools/vtctldclientdocs/ --vitess-dir "/Users/matt/git/vitess" --version-pairs "workflow_update:17.0" Signed-off-by: Matt Lord <mattalord@gmail.com> * Correct info -> note Signed-off-by: Matt Lord <mattalord@gmail.com> --------- Signed-off-by: Matt Lord <mattalord@gmail.com>
Description
This adds a new
update
action to theWorkflow
vtctl client command as well as a newWorkflow update
vtctldclient command. This allows users to easily modify thecells
,tablet-types
, andon-ddl
configuration parameters of a VReplication workflow. There are other parameters — e.g. see the full list of parameters forMoveTables
— that we could potentially add in the future, but for now we're starting with the ones that are always safe to set. The others are dependent on the workflow state and other factors so the application of those changes is more complicated and thus they will only be added in the future based on use-case / demand.This work will allow us to remove the
VReplicationExec
vtctl client command as updating a workflow was the last remaining use case for it. As an example, using theWorkflow update
command would then replace the cumbersomeVReplicationExec
method used to update the cells parameter here: https://vitess.io/docs/16.0/user-guides/migration/troubleshooting/#corrective-actionYou can manually test and play around with the command using the local examples:
Example
vtctldclient
command and output:Related Issue(s)
Checklist