From bc5c5f66e668fcd61a029bd2374297c36f14f7cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20=C5=81=C4=85giewka?= Date: Sun, 13 Apr 2025 08:35:30 +0200 Subject: [PATCH] feat!: use maxConcurrentStreams as multiplexing factor For H2 sessions pipelining is misleading and is a concept of HTTP/1.1. This makes use of `maxConcurrentStreams` parameter to be equivalent of what H2 calls multiplexing. It makes sure that clients do not have to change `pipelining` in order to achieve multiplexing over a H2 stream. Closes #4143. --- benchmarks/benchmark-http2.js | 6 +++--- docs/docs/api/Client.md | 2 +- lib/core/symbols.js | 1 + lib/dispatcher/client.js | 6 ++++++ types/client.d.ts | 2 +- 5 files changed, 12 insertions(+), 5 deletions(-) diff --git a/benchmarks/benchmark-http2.js b/benchmarks/benchmark-http2.js index 46ac89f97bf..227c8266bb1 100644 --- a/benchmarks/benchmark-http2.js +++ b/benchmarks/benchmark-http2.js @@ -15,7 +15,7 @@ const servername = 'agent1' const iterations = (parseInt(process.env.SAMPLES, 10) || 10) + 1 const errorThreshold = parseInt(process.env.ERROR_THRESHOLD, 10) || 3 const connections = parseInt(process.env.CONNECTIONS, 10) || 50 -const pipelining = parseInt(process.env.PIPELINING, 10) || 10 +const maxConcurrentStreams = parseInt(process.env.MULTIPLEXING, 10) || 10 const parallelRequests = parseInt(process.env.PARALLEL, 10) || 100 const headersTimeout = parseInt(process.env.HEADERS_TIMEOUT, 10) || 0 const bodyTimeout = parseInt(process.env.BODY_TIMEOUT, 10) || 0 @@ -61,7 +61,7 @@ const http2NativeClient = http2.connect(httpsBaseOptions.url, { const Class = connections > 1 ? Pool : Client const dispatcher = new Class(httpsBaseOptions.url, { allowH2: true, - pipelining, + maxConcurrentStreams, connections, connect: { rejectUnauthorized: false, @@ -73,7 +73,7 @@ const dispatcher = new Class(httpsBaseOptions.url, { setGlobalDispatcher(new Agent({ allowH2: true, - pipelining, + maxConcurrentStreams, connections, connect: { rejectUnauthorized: false, diff --git a/docs/docs/api/Client.md b/docs/docs/api/Client.md index 680375d1479..75480135a36 100644 --- a/docs/docs/api/Client.md +++ b/docs/docs/api/Client.md @@ -24,7 +24,7 @@ Returns: `Client` * **keepAliveTimeoutThreshold** `number | null` (optional) - Default: `2e3` - A number of milliseconds subtracted from server *keep-alive* hints when overriding `keepAliveTimeout` to account for timing inaccuracies caused by e.g. transport latency. Defaults to 2 seconds. * **maxHeaderSize** `number | null` (optional) - Default: `--max-http-header-size` or `16384` - The maximum length of request headers in bytes. Defaults to Node.js' --max-http-header-size or 16KiB. * **maxResponseSize** `number | null` (optional) - Default: `-1` - The maximum length of response body in bytes. Set to `-1` to disable. -* **pipelining** `number | null` (optional) - Default: `1` - The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Carefully consider your workload and environment before enabling concurrent requests as pipelining may reduce performance if used incorrectly. Pipelining is sensitive to network stack settings as well as head of line blocking caused by e.g. long running requests. Set to `0` to disable keep-alive connections. +* **pipelining** `number | null` (optional) - Default: `1` - The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Carefully consider your workload and environment before enabling concurrent requests as pipelining may reduce performance if used incorrectly. Pipelining is sensitive to network stack settings as well as head of line blocking caused by e.g. long running requests. Set to `0` to disable keep-alive connections. Ineffective for H2 sessions with user provided maxConcurrentStreams. * **connect** `ConnectOptions | Function | null` (optional) - Default: `null`. * **strictContentLength** `Boolean` (optional) - Default: `true` - Whether to treat request content length mismatches as errors. If true, an error is thrown when the request content-length header doesn't match the length of the request body. **Security Warning:** Disabling this option can expose your application to HTTP Request Smuggling attacks, where mismatched content-length headers cause servers and proxies to interpret request boundaries differently. This can lead to cache poisoning, credential hijacking, and bypassing security controls. Only disable this in controlled environments where you fully trust the request source. * **autoSelectFamily**: `boolean` (optional) - Default: depends on local Node version, on Node 18.13.0 and above is `false`. Enables a family autodetection algorithm that loosely implements section 5 of [RFC 8305](https://tools.ietf.org/html/rfc8305#section-5). See [here](https://nodejs.org/api/net.html#socketconnectoptions-connectlistener) for more details. This option is ignored if not supported by the current Node version. diff --git a/lib/core/symbols.js b/lib/core/symbols.js index fd7af0c10e1..bc772b3d031 100644 --- a/lib/core/symbols.js +++ b/lib/core/symbols.js @@ -62,6 +62,7 @@ module.exports = { kListeners: Symbol('listeners'), kHTTPContext: Symbol('http context'), kMaxConcurrentStreams: Symbol('max concurrent streams'), + kMaxConcurrentStreamsManual: Symbol('max concurrenct streams set manually'), kHTTP2InitialWindowSize: Symbol('http2 initial window size'), kHTTP2ConnectionWindowSize: Symbol('http2 connection window size'), kEnableConnectProtocol: Symbol('http2session connect protocol'), diff --git a/lib/dispatcher/client.js b/lib/dispatcher/client.js index 101acb60123..ddac9a6650c 100644 --- a/lib/dispatcher/client.js +++ b/lib/dispatcher/client.js @@ -52,6 +52,7 @@ const { kOnError, kHTTPContext, kMaxConcurrentStreams, + kMaxConcurrentStreamsManual, kHTTP2InitialWindowSize, kHTTP2ConnectionWindowSize, kResume, @@ -72,6 +73,10 @@ const getDefaultNodeMaxHeaderSize = http && const noop = () => {} function getPipelining (client) { + if (client[kHTTPContext]?.version === 'h2' && client[kMaxConcurrentStreamsManual]) { + return client[kMaxConcurrentStreams] ?? client[kHTTPContext]?.defaultPipelining ?? 100 + } + return client[kPipelining] ?? client[kHTTPContext]?.defaultPipelining ?? 1 } @@ -259,6 +264,7 @@ class Client extends DispatcherBase { this[kHTTPContext] = null // h2 this[kMaxConcurrentStreams] = maxConcurrentStreams != null ? maxConcurrentStreams : 100 // Max peerConcurrentStreams for a Node h2 server + this[kMaxConcurrentStreamsManual] = maxConcurrentStreams != null // HTTP/2 window sizes are set to higher defaults than Node.js core for better performance: // - initialWindowSize: 262144 (256KB) vs Node.js default 65535 (64KB - 1) // Allows more data to be sent before requiring acknowledgment, improving throughput diff --git a/types/client.d.ts b/types/client.d.ts index a6e20221f68..156e61d682f 100644 --- a/types/client.d.ts +++ b/types/client.d.ts @@ -62,7 +62,7 @@ export declare namespace Client { keepAliveTimeoutThreshold?: number; /** TODO */ socketPath?: string; - /** The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Default: `1`. */ + /** The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Ineffective for H2 sessions with user provided maxConcurrentStreams. Default: `1`. */ pipelining?: number; /** @deprecated use the connect option instead */ tls?: never;