zstd request / response compression#836
Conversation
The `compression.request` option now accepts an explicit codec: `true`
(or "gzip") keeps gzip; "zstd" compresses the outgoing request body with
zstd via the built-in `zlib` zstd support (Node.js >= 22.15.0). zstd gives
a similar-or-better ratio than gzip at lower CPU cost, and ClickHouse
decompresses it faster server-side (gzip is decompressed single-threaded).
- client-common: widen `compression.request` to `boolean | "gzip" | "zstd"`
("true" => gzip, for backwards compatibility); thread the codec through
`CompressionSettings.compress_request` and emit the matching
`Content-Encoding` header. New exported `RequestCompressionMethod` type.
- client-node: select `Zlib.createZstdCompress()` vs `Zlib.createGzip()` in
the request pipeline; fail fast at client creation with a clear error if
"zstd" is requested on a Node.js without zlib zstd support.
- Web client is unaffected (it does not compress request bodies).
- Unit test + CHANGELOG entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Symmetric to the request-side change: `compression.response` now accepts an
explicit codec ("gzip" default when true, or "zstd"). When "zstd" is selected
the client sends `Accept-Encoding: zstd` and decompresses the response with the
built-in `zlib` zstd support (Node.js >= 22.15.0).
- client-common: widen `compression.response` / `decompress_response` to
`boolean | "gzip" | "zstd"`; emit the matching `Accept-Encoding`; new exported
`ResponseCompressionMethod` type. `withHttpSettings` accepts the codec (still
only toggles `enable_http_compression`).
- client-node: handle `Content-Encoding: zstd` on responses via
`Zlib.createZstdDecompress()`; thread the codec through the query path's
response-compression negotiation. Decompression keys off the server's actual
`Content-Encoding`, so it degrades gracefully against servers that reply with
gzip or no compression.
- Web client unaffected (the browser negotiates/decompresses responses).
- Unit test + CHANGELOG entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds explicit "zstd" support for request/response compression in the Node.js client while keeping boolean compression options backward-compatible.
Changes:
- Extend compression configuration/types to accept
"gzip"or"zstd"in addition to booleans. - Implement zstd request compression and response decompression using Node’s built-in
zlibzstd APIs. - Add unit tests and update changelog for the new zstd functionality.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client-node/src/connection/socket_pool.ts | Adds zstd request body compression via zlib.createZstdCompress() |
| packages/client-node/src/connection/node_base_connection.ts | Selects Accept-Encoding codec based on settings/config and adjusts decompression toggle |
| packages/client-node/src/connection/compression.ts | Adds zstd response decompression via zlib.createZstdDecompress() |
| packages/client-node/src/config.ts | Validates Node supports zstd request compression at client creation |
| packages/client-node/tests/unit/node_connection_compression.test.ts | Adds tests for zstd request compression and response decompression |
| packages/client-common/src/utils/connection.ts | Emits Accept-Encoding / Content-Encoding for gzip vs zstd |
| packages/client-common/src/index.ts | Re-exports new compression method types |
| packages/client-common/src/connection.ts | Introduces RequestCompressionMethod / ResponseCompressionMethod and widens compression settings types |
| packages/client-common/src/config.ts | Updates public config option types/docs for "gzip" / "zstd" |
| CHANGELOG.md | Documents new zstd compression options and Node version requirement |
| if ( | ||
| params.compression.compress_request === "zstd" && | ||
| typeof Zlib.createZstdCompress !== "function" | ||
| ) { | ||
| throw new Error( | ||
| "zstd request compression requires Node.js >= 22.15.0, where zstd " + | ||
| "support was added to the built-in zlib module. Upgrade Node.js, or " + | ||
| "use gzip request compression (`compression: { request: true }`) instead.", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Good catch — fixed in b0bef48. Client creation now validates both compress_request and decompress_response for "zstd" through a shared validateCompressionSupport() helper with a single clear error message.
| const compressor = | ||
| params.enable_request_compression === "zstd" | ||
| ? Zlib.createZstdCompress() | ||
| : Zlib.createGzip(); | ||
| Stream.pipeline(bodyStream, compressor, request, callback); |
There was a problem hiding this comment.
Fixed in b0bef48: the compressor is now built via a createRequestCompressor() helper that throws a clear error (instead of a TypeError) when zstd isn't available. For context, request compression comes from client config (compression.compress_request) and isn't settable per-request, so the creation-time validateCompressionSupport() check already guards this path — the helper is belt-and-suspenders.
There was a problem hiding this comment.
Update: reverted to inline Zlib.createZstdCompress() / createGzip() selection in 9e06294 and dropped the createRequestCompressor helper. Request compression comes from client config and is validated at client creation, so it can never reach this path on an unsupported runtime - the helper was unnecessary abstraction.
| if (encoding === "gzip" || encoding === "zstd") { | ||
| const decompress = | ||
| encoding === "zstd" ? Zlib.createZstdDecompress() : Zlib.createGunzip(); | ||
| return { |
There was a problem hiding this comment.
Fixed in b0bef48 — the zstd branch now checks typeof Zlib.createZstdDecompress === "function" and returns a structured { error } (handled by the normal decompression error path) instead of crashing with ... is not a function.
There was a problem hiding this comment.
Update: reverted the extra capability check here in 9e06294. zstd response decompression is only requested (Accept-Encoding: zstd) when decompress_response: "zstd", which is validated at client creation, so this path only sees zstd on runtimes that support it - the plain branch is sufficient.
| clickhouse_settings.enable_http_compression === 1 | ||
| ? this.params.compression.decompress_response === "zstd" | ||
| ? "zstd" | ||
| : "gzip" | ||
| : false; |
There was a problem hiding this comment.
I think this one is already correct: when enable_http_compression === 1 and decompress_response isn't "zstd", the ternary evaluates to "gzip" (truthy) — not false — so Accept-Encoding: gzip is still sent and the enforce-via-settings behavior is preserved. To make that unambiguous I rewrote it as an explicit let+if in b0bef48 (no behavior change; the codec defaults to gzip and is only zstd when explicitly configured on the client).
There was a problem hiding this comment.
Update: reverted to the original ternary in 9e06294. As noted, it was already correct (it yields "gzip", not false, when enforcing via settings), so the refactor was unnecessary churn on correct code.
| ); | ||
| }); | ||
|
|
||
| it('decompresses a zstd response and sends Accept-Encoding: zstd if response: "zstd"', async () => { |
There was a problem hiding this comment.
Fixed in b0bef48 — both zstd tests are guarded with it.skipIf(!zstdSupported) (zstdSupported = typeof Zlib.createZstdCompress === "function"), so they skip on Node.js runtimes whose zlib has no zstd, e.g. the Node 20 entry in the CI matrix.
| request.emit( | ||
| "response", | ||
| buildIncomingMessage({ | ||
| body: Zlib.zstdCompressSync(Buffer.from(responseBody)), |
There was a problem hiding this comment.
Covered by the same fix in b0bef48: this test is now guarded with it.skipIf(!zstdSupported), so it skips on Node runtimes without zlib zstd (< 22.15).
| ).toBe("gzip"); | ||
| }); | ||
|
|
||
| it('sends a zstd-compressed request if compress_request: "zstd"', async () => { |
There was a problem hiding this comment.
Same fix in b0bef48 — guarded with it.skipIf(!zstdSupported), skipped on Node < 22.15.
| next(); | ||
| }, | ||
| final() { | ||
| Zlib.zstdDecompress(chunks, (_err, result) => { |
There was a problem hiding this comment.
Same fix in b0bef48 — guarded with it.skipIf(!zstdSupported), skipped on Node < 22.15.
jstastny
changed the title
… older Node - Validate both `compress_request` and `decompress_response` for zstd at client creation (was request-only), via a new `validateCompressionSupport()` helper with a single clear error message. - Guard the request-compressor and response-decompressor call sites so an unsupported runtime surfaces a clear error / structured decompression error instead of a `TypeError` (`createRequestCompressor()` helper; explicit zstd capability check in `decompressResponse()`). - Clarify the response "enforce-via-settings" branch in node_base_connection (behavior unchanged: codec defaults to gzip; zstd only when explicitly configured on the client). - Skip the zstd unit tests on Node.js runtimes whose `zlib` lacks zstd (`it.skipIf`), so the suite passes on the Node 20 entry of the CI matrix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| const request = new Stream.Writable({ | ||
| write(chunk, encoding, next) { | ||
| chunks = Buffer.concat([chunks, chunk]); | ||
| next(); | ||
| }, | ||
| final() { | ||
| Zlib.zstdDecompress(chunks, (_err, result) => { | ||
| finalResult = result; | ||
| }); | ||
| }, | ||
| }) as ClientRequest; |
There was a problem hiding this comment.
Fixed in fa3c97b — the test's Writable.final(callback) now invokes callback(err) after decompression completes (propagating any error), so stream completion is signaled properly instead of relying on undefined semantics.
| // trigger stream pipeline | ||
| await sleep(0); | ||
| request.emit("socket", socketStub); | ||
| await sleep(100); | ||
|
|
||
| expect(finalResult!.toString("utf8")).toEqual(values); |
There was a problem hiding this comment.
Fixed in fa3c97b — replaced the fixed sleep(100) with awaiting a promise that resolves/rejects from the zstdDecompress callback, so the assertion is deterministic and not timing-dependent.
| nodeConfig: NodeClickHouseClientConfigOptions, | ||
| params: ConnectionParams, | ||
| ) => { | ||
| validateCompressionSupport(params.compression); |
There was a problem hiding this comment.
Added in fa3c97b — a validateCompressionSupport describe covering: (1) throws for compress_request: "zstd" and for decompress_response: "zstd" when the corresponding Zlib.createZstd* is missing (simulated via Object.defineProperty, since zlib's exports are configurable: true, writable: false), and (2) does not throw for true / "gzip" / false.
…pressionSupport tests - Make the zstd request-compression test deterministic: the Writable's `final(callback)` now invokes the callback (propagating errors), and the assertion awaits a promise resolved from `zstdDecompress` instead of a fixed `sleep(100)`. - Add unit tests for `validateCompressionSupport`: throws for zstd request / response when the corresponding zlib zstd API is missing (simulated via `Object.defineProperty`), and does not throw for `true` / `"gzip"` / `false`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| function withMissingZlibFn( | ||
| name: "createZstdCompress" | "createZstdDecompress", | ||
| fn: () => void, | ||
| ) { | ||
| const descriptor = Object.getOwnPropertyDescriptor(Zlib, name)!; | ||
| Object.defineProperty(Zlib, name, { | ||
| value: undefined, | ||
| configurable: true, | ||
| writable: true, | ||
| }); | ||
| try { | ||
| fn(); | ||
| } finally { | ||
| Object.defineProperty(Zlib, name, descriptor); | ||
| } | ||
| } |
There was a problem hiding this comment.
Resolved in 9e06294 by removing these zlib-monkey-patching tests entirely, along with the validateCompressionSupport helper they covered (now an inline guard at client creation). The it.skipIf(!zstdSupported) guards keep the suite green on Node 20.
| export function createRequestCompressor( | ||
| method: boolean | RequestCompressionMethod, | ||
| ): Stream.Transform { | ||
| if (method === "zstd") { | ||
| if (typeof Zlib.createZstdCompress !== "function") { | ||
| throw new Error(ZSTD_UNSUPPORTED_MESSAGE); | ||
| } | ||
| return Zlib.createZstdCompress(); | ||
| } | ||
| return Zlib.createGzip(); | ||
| } |
There was a problem hiding this comment.
Resolved in 9e06294 - removed createRequestCompressor. The compressor is selected inline again and only ever with a truthy codec, so the false-returns-gzip case no longer exists.
| const ZSTD_UNSUPPORTED_MESSAGE = | ||
| "zstd compression requires Node.js >= 22.15.0, where zstd support was added " + | ||
| "to the built-in zlib module. Upgrade Node.js, or use gzip compression instead."; |
There was a problem hiding this comment.
Applied in 9e06294 - the guard now uses a feature-detection message that leads with the missing zlib zstd APIs and includes the detected runtime (process.versions.node), rather than asserting a hardcoded minimum version.
…td guard Reverts the helper/abstraction churn from the review rounds back toward the original implementation, keeping only the genuinely-needed pieces: - Inline the zstd capability guard at client creation, covering BOTH request and response (the one real gap), with a feature-detection error message. - Remove the `validateCompressionSupport` and `createRequestCompressor` helpers; the request compressor is selected inline again (the creation-time guard makes it safe - request compression is client config, not per-request). - Restore the original response-compression ternary in node_base_connection (it was already correct) and the plain `decompressResponse` branch. - Tests: keep `it.skipIf` so the zstd cases skip on Node < 22.15; drop the zlib-monkey-patching unit tests that themselves caused a Node-20 regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @jstastny for raising! |
|
Maybe this is not backward compatible, but I can imagine it makes sense to explicitly pass the codec and not mix booleans with strings in the payload. |
| if (encoding === "gzip" || encoding === "zstd") { | ||
| const decompress = | ||
| encoding === "zstd" ? Zlib.createZstdDecompress() : Zlib.createGunzip(); |
There was a problem hiding this comment.
Good catch - fixed in 0ff391f. decompressResponse now only takes the zstd branch when typeof Zlib.createZstdDecompress === "function"; otherwise an unexpected Content-Encoding: zstd falls through to the existing Unexpected encoding structured error (same as any other unknown encoding) instead of throwing a TypeError mid-stream. So a client on a non-zstd runtime no longer crashes if a server/proxy returns zstd unsolicited.
| stubClientRequest, | ||
| } from "../utils/http_stubs"; | ||
|
|
||
| const zstdSupported = typeof Zlib.createZstdCompress === "function"; |
There was a problem hiding this comment.
Done in 0ff391f - zstdSupported now checks every zstd zlib API the tests use (createZstdCompress, createZstdDecompress, zstdCompressSync, zstdDecompress), not just createZstdCompress.
| ...(enable_response_compression | ||
| ? { | ||
| "Accept-Encoding": | ||
| enable_response_compression === "zstd" ? "zstd" : "gzip", | ||
| } |
There was a problem hiding this comment.
Keeping this as a single-codec Accept-Encoding for now. compression.response: "zstd" is an explicit codec choice, and the decompression path already handles whatever the server actually returns (gzip/zstd/identity, with a clean error for anything unsupported). Advertising zstd, gzip would change the meaning of the explicit choice - you could silently get gzip - which I'd rather keep predictable. Happy to switch to q-value negotiation if you'd prefer it as the intended behavior.
| enable_response_compression?: boolean | ResponseCompressionMethod; | ||
| enable_request_compression?: boolean | RequestCompressionMethod; |
There was a problem hiding this comment.
These are internal connection-params field names, not public API (the public option is compression.request / compression.response), and the boolean-or-codec convention is captured by their types. Renaming would ripple through several internal call sites for a cosmetic gain, so I'd prefer to keep them as-is to avoid the churn.
- decompressResponse: only take the zstd branch when zlib actually provides createZstdDecompress; otherwise fall through to the existing "Unexpected encoding" structured error. Prevents a TypeError mid-stream if a server or proxy returns Content-Encoding: zstd on a Node build without zstd support - even for clients that never configured zstd. - tests: gate the zstd cases on every zstd zlib API they use (createZstdCompress / createZstdDecompress / zstdCompressSync / zstdDecompress), not just createZstdCompress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| const { controller, controllerCleanup } = this.getAbortController(params); | ||
| // allows enforcing the compression via the settings even if the client instance has it disabled | ||
| const enableResponseCompression = | ||
| clickhouse_settings.enable_http_compression === 1; | ||
| clickhouse_settings.enable_http_compression === 1 | ||
| ? this.params.compression.decompress_response === "zstd" | ||
| ? "zstd" | ||
| : "gzip" | ||
| : false; |
| if (params.enable_request_compression) { | ||
| Stream.pipeline(bodyStream, Zlib.createGzip(), request, callback); | ||
| const compressor = | ||
| params.enable_request_compression === "zstd" | ||
| ? Zlib.createZstdCompress() | ||
| : Zlib.createGzip(); | ||
| Stream.pipeline(bodyStream, compressor, request, callback); |
| // zstd needs Node.js >= 22.15.0 (zstd in the built-in zlib); fail fast with a | ||
| // clear error rather than a TypeError deep in a later insert/query. | ||
| if ( | ||
| (params.compression.compress_request === "zstd" && | ||
| typeof Zlib.createZstdCompress !== "function") || | ||
| (params.compression.decompress_response === "zstd" && | ||
| typeof Zlib.createZstdDecompress !== "function") | ||
| ) { | ||
| throw new Error( | ||
| "zstd compression is not supported by this Node.js runtime (v" + | ||
| process.versions.node + | ||
| "): the built-in zlib module does not provide the zstd APIs (added " + | ||
| "in Node.js 22.15.0). Use gzip compression instead.", | ||
| ); | ||
| } |
Thanks @bobdevries — you're right that overloading one field with For me it comes down to the trade-off against backward compatibility. I don't feel strongly enough to override your call on the repo's conventions, though. If you'd prefer the codec-only shape and want to treat it as a breaking change for a future major, I'm happy to switch the types/guard/tests over. My default, unless you feel strongly, would be to keep it backward compatible. |
Summary
Adds zstd as a selectable codec for both directions of HTTP compression in
@clickhouse/client(Node.js):compression.request: "zstd"compression.response: "zstd"Fully backwards compatible:
request: true/response: truestill mean gzip.Addresses the ZSTD item of #120.
Motivation
#120 (open since 2022) stalled on a concrete blocker: in 2023 there was no robust
Node compression library to lean on (
node-lz4didn't support modern Node, andlz4-napilacked streaming). That blocker no longer applies to zstd —Node.js added zstd to the built-in
zlibmodule in v22.15.0 / v23.8.0(
zlib.createZstdCompress/createZstdDecompress, as streamingTransforms).So zstd needs no third-party dependency and reuses the exact streaming
pipeline gzip already uses.
zstd is also a better default than gzip for inserts: a similar-or-better ratio at
materially lower CPU. And per @mshustov's note on #120, ClickHouse decompresses
gzip single-threaded, so zstd lowers server-side ingest cost as well. This is most
impactful for write-heavy workloads (e.g. high-volume inserts over a metered
private link).
API
compression.requestandcompression.responsenow acceptboolean | "gzip" | "zstd".true→ gzip (back-compat);"zstd"selects zstd.Implementation
compression.request/compression.response(and the internalcompress_request/decompress_response) toboolean | "gzip" | "zstd";new exported types
RequestCompressionMethod/ResponseCompressionMethod.withCompressionHeadersemits the matchingContent-Encoding/Accept-Encoding(gziporzstd).zlib.createZstdCompress()vscreateGzip()in the requestpipeline.
Content-Encoding: zstdviazlib.createZstdDecompress().Decompression keys off the server's actual
Content-Encoding, so itdegrades gracefully if the server replies with gzip or no compression.
runtime throws a clear error at client creation (
@clickhouse/clientsupports
node >=16).browser negotiates/decompresses responses.
Scope & notes
lz4/bz2/snappystill requirethird-party native addons (the original blocker) and are out of scope here.
deflateandbrotliare also native to Node'szliband could be addedlater with the same pattern if there's interest.
happy to squash.
Test plan
Content-Encoding: zstdand the bodyround-trips through
zstdDecompress; response sendsAccept-Encoding: zstdand a zstd-encoded response body is decompressed correctly. The gzip path is
unchanged (back-compat test still passes).
tsctypecheck andeslint --max-warnings=0clean acrosscommon / node / web; full build green.
request_compression/response_compressionsuites with a zstd case(requires a ClickHouse version with zstd HTTP transport support, 22.10+).
Compatibility
server that supports zstd HTTP
Content-Encoding(server-side since 22.10).Checklist
Delete items not relevant to your PR: