grpc-js: support grpc-node.flow_control_window on the server#3058
Open
imperatormk wants to merge 1 commit into
Open
grpc-js: support grpc-node.flow_control_window on the server#3058imperatormk wants to merge 1 commit into
imperatormk wants to merge 1 commit into
Conversation
The grpc-node.flow_control_window channel option was added for the client transport in grpc#2864, but the server never read it, so a server's HTTP/2 receive window stayed at the Node default of 64 KB with no way to raise it. On high-RTT links this throttles large uploads: each 64 KB requires one round-trip for a WINDOW_UPDATE, so a 19 MB message can take tens of seconds. This sets settings.initialWindowSize from the option on server construction (per-stream window) and raises the connection-level window per session via setLocalWindowSize, with an incrementWindowSize fallback for older Node. This mirrors the existing client implementation in transport.ts. Adds a test that round-trips a large message with the option set.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
grpc-node.flow_control_windowwas added for the client transport in #2864, but the server never reads it. As a result a server's HTTP/2 receive window is always pinned to Node's default of 64 KB, with no channel option to raise it.On high-RTT links this throttles large uploads. With a 64 KB window, every 64 KB of data requires a full round-trip for a
WINDOW_UPDATEbefore more can be sent, so throughput is bounded bywindow / RTT. A ~19 MB unary upload to a server one or two hops away can take tens of seconds, and the data arrives as a long sequence of 16 KB frames with periodic multi-second stalls. The same option already fixes this on the client side; the server side had no equivalent.grpc.max_send_message_length/grpc.max_receive_message_lengthdo not help here, since they cap message size, not the flow-control window.Change
Make the server honor
grpc-node.flow_control_window, mirroring the existing client implementation intransport.ts:settings.initialWindowSizefrom the option. This advertises a larger per-stream window in the SETTINGS frame for new sessions.session.setLocalWindowSize(...), with anincrementWindowSizefallback for older Node versions. This is guarded byflowControlWindow > defaultWindowso it is a no-op when the option is unset or not larger than the default.Testing
Added a test in
test/test-server.tsthat setsgrpc-node.flow_control_windowon the server and round-trips a message larger than the default window. The fulltest-serversuite passes.I reproduced the original problem and verified the fix out of cluster using
tc netemto inject RTT on loopback: a 19 MB unary call at ~20 ms RTT took ~15 s with the default window and ~3.4 s with a 4 MB window, with the remaining time being application work. The effect was identical across grpc-js 1.10, 1.12, and 1.14, confirming this is the default-window behavior rather than a regression.Note for reviewers
setLocalWindowSizeis present in current@types/node, butincrementWindowSizeis not, so the fallback references it through a small local interface rather thanany. The client code intransport.tsusesas anyfor the same call; happy to match that style if preferred.