grpc-node icon indicating copy to clipboard operation
grpc-node copied to clipboard

Fix: send connection-level WINDOW_UPDATE at session start

Open KoenRijpstra opened this issue 6 months ago • 3 comments

Overview

@grpc/grpc-js previously opened every HTTP/2 connection with the RFC‑default 65 535‑byte connection window, then waited until after the first DATA frames were received before enlarging it. On fast, high‑volume streams this small initial window causes the sender (or any intermediate HTTP/2 proxy) to buffer excess data, creating avoidable latency.

This change immediately enlarges the connection‑level window as soon as the session is created when the application has requested a larger value via

'grpc-node.flow_control_window'

This aligns the Node implementation with other gRPC language stacks that send the larger window before any payload is transmitted.


Core change (single patch)

// createSession() — directly after http2.connect()
const defaultWin = http2.getDefaultSettings().initialWindowSize; // 65 535
const connWin = options["grpc-node.flow_control_window"] as number | undefined;

// Apply user‑requested window immediately so the first DATA burst is not throttled
if (connWin && connWin > defaultWin) {
  try {
    // Node ≥ 14.18
    (session as any).setLocalWindowSize(connWin);
  } catch {
    // Older Node fallback
    const delta = connWin - (session.state.localWindowSize ?? defaultWin);
    if (delta > 0) (session as any).incrementWindowSize(delta);
  }
}

No public API changes. Clients that do not set the option keep the existing behaviour.


Impact

Before After
Initial flow‑control Sender stalls after 65 KB Full requested window available at once
Latency under load Can grow as buffers back up Remains stable
Compatibility Behaviour now matches Go/C/C++/Rust gRPC

Works on Node 14 → 20 (fallback path covers older Node versions).


Changelog entry

### Fixed
* http2: enlarge connection‑level flow‑control window at session start when a larger
  'grpc-node.flow_control_window' is configured.

KoenRijpstra avatar Jun 27 '25 12:06 KoenRijpstra

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: KoenRijpstra / name: Koen Rijpstra (d87227860639449adbb3a8299fcb2681260caf88, aeb7a5fd52e319887787a163c5ccfe852f53e35d, 0e09b9cd59042ec905ea7fec85e489d95e786729, b69bcad1bb03772e451a785e1dabf07616481f67, a8d22dc906f7877b9ed6b152930de0f9caabb122)

The setLocalWindowSize documentation indicates that it should be called in the handler for the 'connect' or 'remoteSettings' event. There is already a 'remoteSettings' handler, so it should be moved there. This change should still work as intended because the session isn't used until after that event is handled anyway.

murgatroid99 avatar Jun 27 '25 17:06 murgatroid99

@murgatroid99

The setLocalWindowSize documentation indicates that it should be called in the handler for the 'connect' or 'remoteSettings' event. There is already a 'remoteSettings' handler, so it should be moved there. This change should still work as intended because the session isn't used until after that event is handled anyway.

I’ve moved it to remoteSettings, and it still works as expected 🚀. Mind giving it another review?

KoenRijpstra avatar Jun 27 '25 19:06 KoenRijpstra

@murgatroid99 When you have a moment, could you take another look and let me know if it’s good to go?

KoenRijpstra avatar Jul 06 '25 07:07 KoenRijpstra

any updates here? We need the same fix

bernardoVale avatar Aug 06 '25 20:08 bernardoVale