VStream does not work when keyspaces_to_watch is set on vtgate

5antelope commented 3 years ago

Overview of the Issue

VStream relies on topo server to create a tablet picker: https://github.com/vitessio/vitess/blob/990d49ea576ed480a61f415be855fdebf25d99b5/go/vt/vtgate/vstream_manager.go#L431

After https://github.com/vitessio/vitess/pull/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: https://github.com/vitessio/vitess/blob/c1c9276bcfd6b5191fc493a11b6dc523d3179066/go/vt/srvtopo/keyspace_filtering_server.go#L67

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 commented 3 years ago

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 https://github.com/vitessio/vitess/pull/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 commented 3 years ago

Closing this as fixed via https://github.com/vitessio/vitess/pull/8988

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

vitessio / vitess