-
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
Support VStream with keyspaces_to_watch #8988
Conversation
293a4dc
to
6e04f20
Compare
6e04f20
to
f1d68bd
Compare
db8bce1
to
78532c7
Compare
The new
|
b81a55e
to
fa7b418
Compare
cc4a7c4
to
a4b169b
Compare
a4b169b
to
d79d53a
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
ee1bae5
to
ad6c19e
Compare
…s to underlying topo.Server Signed-off-by: Matt Lord <mattalord@gmail.com>
ad6c19e
to
4c23cf0
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.
I think it's important that the we don't have dummy implementations of SetReadOnly
in the servers. This can become a source of confusion in the future if someone accidentally uses one of these connections as an interface, and the implementation doesn't enforce it.
If explicit casting or the interface approach is harder, I'd recommend changing SetReadOnly
to return an error, and we can have these underlying implementations fail with a "not supported" error.
Good points. I'll look into this next. Thanks! |
025ede4
to
258f834
Compare
d22f2aa
to
838d436
Compare
Instead handle this entirely within the generic topo Server implementation Signed-off-by: Matt Lord <mattalord@gmail.com>
838d436
to
00ec04d
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.
This is great!
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 too. Nice work!
Description
In order to safely support the
VStream
api against VTGates running with the-keyspaces_to_watch
flag we needed to add a read-onlyVTTopo
server interface to ensure that no changes are made to the topology from the (potentially) explicitly limited view of the topology provided via thisvtgate
.Related Issue(s)
Checklist