Skip to content
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

VStream does not work when keyspaces_to_watch is set on vtgate #8772

Closed
5antelope opened this issue Sep 6, 2021 · 2 comments
Closed

VStream does not work when keyspaces_to_watch is set on vtgate #8772

5antelope opened this issue Sep 6, 2021 · 2 comments

Comments

@5antelope
Copy link
Member

Overview of the Issue

VStream relies on topo server to create a tablet picker:

tp, err := discovery.NewTabletPicker(vs.ts, []string{vs.vsm.cell}, sgtid.Keyspace, sgtid.Shard, vs.tabletType.String())

After #7873, when we pass keyspaces_to_watch flag, VTGate will initialize a keyspaceFilteringServer which will return error when we try to get topo server out of it:

func (ksf keyspaceFilteringServer) GetTopoServer() (*topo.Server, error) {

This could be problematic if there is a lot of of keyspaces, and the vtgate has to keep connections to all of them in order to support vstream() endpoint.

@setassociative suggested providing a readonly topo interface in keyspaceFilteringServer, instead of raising error directly - which I think makes sense, it can:

  • avoid the online DDL issue that we were trying to address initially (i don't fully grok the problem yet, but a readonly topo interface that avoids writes seems to be good enough for the specific use case)
  • allow vstream with keyspaces_to_watch flag

Reproduction Steps

Launch VTGate with keyspaces_to_watch flag and hit VStream endpoint on vtgate

Binary version

I'm on top of 4358c6

commit 4358c6a85e607c2676135599f36ef3b53705f004 (upstream/main)
Merge: 6afc58d9df 92e07c2293
Author: Harshit Gangal <harshit@planetscale.com>
Date:   Wed Aug 25 00:25:56 2021 +0530
@setassociative
Copy link

Minor nit -- #7873 didn't introduce this; errors have been thrown when trying to access a writable topo server since the introduction of keyspaces_to_watch in #4420. If Vstream every worked on a vtgate with this flag it'll be because the method of selecting a tablet at some point didn't rely on a writable topo server ref.

@mattlord
Copy link
Contributor

Closing this as fixed via #8988

If I missed or misunderstood anything please let me know. Thank for helping make Vitess better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants