Prefer using REPLICA tablets when selecting vreplication sources#10040
Prefer using REPLICA tablets when selecting vreplication sources#10040rohit-nayak-ps merged 6 commits intovitessio:mainfrom
Conversation
Signed-off-by: Matt Lord <mattalord@gmail.com>
rohit-nayak-ps
left a comment
There was a problem hiding this comment.
one additional heptext needs to be modified, but otherwise lgtm
Signed-off-by: Matt Lord <mattalord@gmail.com>
go/vt/vtctl/vtctl.go
Outdated
|
|
||
| cells := subFlags.String("cells", "", "Cell(s) or CellAlias(es) (comma-separated) to replicate from.") | ||
| tabletTypes := subFlags.String("tablet_types", "primary,replica,rdonly", "Source tablet types to replicate from (e.g. primary, replica, rdonly). Defaults to -vreplication_tablet_type parameter value for the tablet, which has the default value of replica.") | ||
| tabletTypes := subFlags.String("tablet_types", "in_order:REPLICA,PRIMARY", "Source tablet types to replicate from (e.g. primary, replica, rdonly). Defaults to --vreplication_tablet_type parameter value for the tablet, which has the default value of in_order:REPLICA,PRIMARY.") |
There was a problem hiding this comment.
nit: we seem to be using uppercase in the default value, but lowercase in the help text. It doesn't matter for the internals, but it will be good to make this consistent everywhere in the user interface (including the vreplication_tablet_type flag to vttablet).
There was a problem hiding this comment.
Where do you see it lower case?
There was a problem hiding this comment.
ah, you mean more generally than the default value itself e.g. (e.g. primary, replica, rdonly). I prefer capitalized but don't feel too strongly about it.
There was a problem hiding this comment.
At least started doing that here ebc0469 and here vitessio/website@679678e
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>
| vrwp.TabletTypes = *tabletTypes | ||
| if vrwp.TabletTypes == "" { | ||
| vrwp.TabletTypes = "primary,replica,rdonly" | ||
| vrwp.TabletTypes = "in_order:REPLICA,PRIMARY" |
There was a problem hiding this comment.
Was rdonly removed on purpose?
There was a problem hiding this comment.
Yeah, according to our docs and --help output we use the default of the vttablet --vreplication_tablet_type flag if you don't specify and that's what we're actually now doing here. After this PR, the only place RDONLY is potentially used by default is for VDiff (maybe rdonly had ended up here due to a cut and paste from the VDiff code?).
Let me know if not using the vttablet --vreplication_tablet_type default was intentional here before and perhaps the docs were incorrect.
There was a problem hiding this comment.
Related to this, I'm not sure why we have this if vrwp.TabletTypes == "" { code block at all since we are specifying a default value here: https://github.com/vitessio/vitess/pull/10040/files#diff-ad37010accc5a29c6751d18124328bb882457e8b2527db688b854c81fe23861cR2333
Maybe that wasn't always working as I'd expect.
There was a problem hiding this comment.
Related to this, I'm not sure why we have this
if vrwp.TabletTypes == "" {code block at all since we are specifying a
This might be defensive coding for when an empty string is specified for tablet types on the command line or in tests.
There was a problem hiding this comment.
@rohit-nayak-ps turns out that you were right! I shouldn't have removed that. Re-adding it fix this: #10421
🤦
There was a problem hiding this comment.
Well, after further testing... this didn't actually change any behavior. We were never switching RDONLY traffic by default -- as the tablet_types flag variable was never an empty string as it has a default value (which is what we changed) -- and we weren't testing for this either. Will correct both in #10421.
…essio#10040) * Prefer using REPLICA tablets when selecting vreplication sources Signed-off-by: Matt Lord <mattalord@gmail.com> * Modify tablet type defaults for generic vrepl workflow handler Signed-off-by: Matt Lord <mattalord@gmail.com> * Add in_order support to the v2 workflow code Signed-off-by: Matt Lord <mattalord@gmail.com> * Minor changes after self review Signed-off-by: Matt Lord <mattalord@gmail.com> * Correct primary tablet type filtering in switchReads Signed-off-by: Matt Lord <mattalord@gmail.com> * Consistently use capitalization for tablet types Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Vilius Okockis <vilius.okockis@vinted.com>
Description
This changes the defaults for tablet selection when choosing vreplication sources so that we use a
REPLICAtablet when one is available and only fall back to using aPRIMARYtablet when necessary.This is a good default as the vreplication workflow can have a very significant impact on the source tablet and should generally not be done on a
PRIMARYtablet when possible since it's likely serving production traffic.Related Issue(s)
Checklist