-
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
VReplication: Force flag for traffic switching #16709
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Matt Lord <mattalord@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16709 +/- ##
==========================================
- Coverage 68.94% 68.93% -0.02%
==========================================
Files 1565 1565
Lines 201764 201817 +53
==========================================
+ Hits 139112 139121 +9
- Misses 62652 62696 +44 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Matt Lord <mattalord@gmail.com>
4d66ea2
to
394a0ec
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
394a0ec
to
675a2d8
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
641f027
to
d73f546
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
740ef56
to
70bf086
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
6c19147
to
da9c130
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>
ea21616
to
9c50973
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
9c50973
to
4b5f109
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>
@@ -1984,6 +1984,7 @@ message WorkflowSwitchTrafficRequest { | |||
bool dry_run = 9; | |||
bool initialize_target_sequences = 10; | |||
repeated string shards = 11; | |||
bool force = 12; |
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.
This request keeps getting bigger. I wish we had implemented something similar to workflow options.
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.
WorkflowOptions
are persisted in the workflow record as they affect the workflow generally (set on Create
and used from then on). This flag only applies to a single traffic switch call and shouldn’t be persisted.
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.
I’m also not sure what’s bad about adding command flags and the corresponding proto fields for it? If this is about the actual wire protocol message size, then AFAIK embedding doesn't significantly impact that one way or the other. If that is the concern though, we could move all (non-repeated) fields to optional
so that they are nil pointers instead of zero value structs (I'm actually not sure that this affects the over the wire bytes either way though, from my testing and experimentation I don't think it does, it only affects unmarshalling of the wire protocol message and the bytes used for that in-memory structure). I recently added one new optional
field in #16654 and moved some existing ones to optional here: #16734
I can see e.g. that all of the (non-repeated) fields seem to be optional now in k8s: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/api/core/v1/generated.proto
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Description
This PR adds an expert only
--force
flag to theSwitchTraffic
andReverseTraffic
workflow sub-commands that currently exist for:When specified, potentially non-critical failures such as partial tablet refreshes are considered non-fatal and the traffic switching work continues.
An example use case is:
MoveTables
workflowReverseTraffic
ASAP because application errors are occurring and it's causing a [partial] outageRelated Issue(s)
Checklist