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

Introduce multiplexed grpc-over-websocket proxy to reduce open websockets at runtime #2510

Closed
niloc132 opened this issue Jun 10, 2022 · 0 comments · Fixed by #2606
Closed

Comments

@niloc132
Copy link
Member

Browsers only support a limited number of websockets at a time (limited by tab/origin/etc), and each grpc stream has a different purpose of some kind - autocomplete, checking for table size changes, and table data subscriptions. The actual value of the limit usually doesn't matter for a simple browser UI, but with PartitionedTable and plot-by implemented, it is necessary to support more concurrent streams than we previously have needed.

The simplest way to support this would be to transmit the "stream id" as part of each frame, so that each end of the connection knows what it is talking about.

There is an issue in improbable-eng/grpc-web that discusses this, and a branch with some in-progress work, but the solution there is to wrap with a new protobuf type. I think the simpler approach would be to alter the framing slightly, appending a 4byte stream id ahead of the proper grpc frame, similarly to how the existing websocket proxy uses a "control flow" byte to signal a half-close.

--

Negotiation between client and server to support this multiplexing feature appears to be possible using websocket subprotocols, where a client that supports more than one system could ask the server to pick the "most specific" protocol it can speak. The downside of using this approach would be that each service has to connect directly, rather than letting multiple services share a single socket. This might not matter for us, as we don't have too many services, but it is something to think about.

Supporting multiple protocols lets us remain compatible with the upstream go proxy and "gracefully" fall back, but with the downside that using the go proxy will go back to the limited stream count behavior.

@niloc132 niloc132 added this to the Jun 2022 milestone Jun 10, 2022
@niloc132 niloc132 self-assigned this Jun 10, 2022
niloc132 added a commit that referenced this issue Oct 27, 2022
Creates a new websocket protocol to let the websocket behave only as a
transport on which multiple streams, across services, can be conveyed.
The websocket will be opened to the path matching the first stream, but
after the websocket has started, other streams can be sent on it, using
custom metadata/header to give the path to the desired rpc.

By reusing existing paths, the client can signal that it supports
either old or new websockets, so it in theory can connect to the old
grpc-websockets implementation. Unfortunately, the reference
implementation can't handle alternative requested subprotocols, and
will just fail the entire request, so this patch breaks our
compatibility with that proxy, and so with our netty implementation.

On the other hand, the server will maintain compatibility for both
protocols for now, enabling stale js api clients to still be able to
connect. We may also find that there are issues with longlived sockets,
and might end up wanting to ask the client in some circumstances to use
the old implementation.

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

Successfully merging a pull request may close this issue.

1 participant