TCP B3-B8: compression frames (CityHash v1.0.2, LZ4/ZSTD), on by default - #568
TCP B3-B8: compression frames (CityHash v1.0.2, LZ4/ZSTD), on by default#568alex-clickhouse wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds default LZ4 compression to the native TCP protocol, including framed streaming, CityHash v1.0.2 checksums, ZSTD support, configuration, and tests.
Changes:
- Implements compression frame encoding, decoding, and checksums.
- Integrates framed block I/O into TCP queries and inserts.
- Adds compression options and comprehensive unit/integration coverage.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
ClickHouse.Driver.Tests/CompressionFrameTests.cs |
Tests frame layout and validation. |
ClickHouse.Driver.Tests/CityHash102Tests.cs |
Tests CityHash reference vectors. |
ClickHouse.Driver.Tcp/Protocol/Query.cs |
Writes the query compression flag. |
ClickHouse.Driver.Tcp/Protocol/ClickHouseTcpConnection.cs |
Integrates compressed block I/O. |
ClickHouse.Driver.Tcp/Format/BlockWriter.cs |
Separates block envelope and body writing. |
ClickHouse.Driver.Tcp/Format/BlockReader.cs |
Separates block envelope and body reading. |
ClickHouse.Driver.Tcp/Compression/FramedPackets.cs |
Defines framed packet types. |
ClickHouse.Driver.Tcp/Compression/CompressedFrameWriter.cs |
Streams outgoing compression frames. |
ClickHouse.Driver.Tcp/Compression/CompressedFrameReader.cs |
Streams and validates incoming frames. |
ClickHouse.Driver.Tcp/Client/IConnectionFactory.cs |
Passes compressors to connections. |
ClickHouse.Driver.Tcp/Client/ClickHouseTcpConnectionStringBuilder.cs |
Adds the compression connection-string option. |
ClickHouse.Driver.Tcp/Client/ClickHouseTcpClientOptions.cs |
Adds compressor configuration and defaults. |
ClickHouse.Driver.Tcp/ClickHouse.Driver.Tcp.csproj |
References shared compression code. |
ClickHouse.Driver.Tcp.Tests/Integration/CompressionIntegrationTests.cs |
Tests compression against ClickHouse. |
ClickHouse.Driver.Tcp.Tests/Compression/FramedPacketsTests.cs |
Tests packet framing selection. |
ClickHouse.Driver.Tcp.Tests/Compression/CompressedFrameStreamingTests.cs |
Tests streaming and frame boundaries. |
ClickHouse.Driver.Tcp.Tests/Client/CompressionOptionTests.cs |
Tests compression configuration. |
ClickHouse.Driver.Tcp.Tests/Client/ClickHouseTcpClientOptionsTests.cs |
Covers compressor option copying. |
ClickHouse.Driver.Tcp.Tests/ClickHouse.Driver.Tcp.Tests.csproj |
References shared compression types. |
ClickHouse.Driver.Common/Compression/CompressionFrame.cs |
Implements frame encoding and decoding. |
ClickHouse.Driver.Common/Compression/CityHash102.cs |
Implements CityHash v1.0.2. |
ClickHouse.Driver.Common/AssemblyInfo.cs |
Grants TCP assemblies internal access. |
Suppressed comments (1)
ClickHouse.Driver.Tcp/Client/ClickHouseTcpClientOptions.cs:285
- Making LZ4 framing the default adds hashing, compression, copying, and frame I/O to every TCP query, but the PR provides no throughput/allocation comparison. The repository's performance guidance requires BenchmarkDotNet measurements for common-path changes; include representative query-read results for
noneversus default LZ4 before enabling it globally.
public IClickHouseCompressor Compressor { get; init; } = ResolveCompressor(DefaultCompression);
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Query.Write(writer, negotiated, clientMetadata, queryId, sql, settings, parameters, compressor is not null); | ||
| await WriteEndOfInputBlockAsync(cancellationToken).ConfigureAwait(false); |
| /// Frame codec for the native protocol: <c>lz4</c>, <c>zstd</c>, or <c>none</c>. Defaults to | ||
| /// <c>none</c>. Maps to <see cref="ClickHouseTcpClientOptions.Compressor"/>. |
| private const int MaxBodySize = 1 << 30; | ||
| private const int MaxPlaintextSize = 1 << 30; |
| /// <summary> | ||
| /// The <c>Compression</c> connection-string value used when the key is absent. LZ4 is what | ||
| /// <c>clickhouse-client</c> uses on this protocol: the cheapest codec in CPU and the lightest on the | ||
| /// server. Pass <c>Compression=none</c>, or a null <see cref="Compressor"/>, to turn it off. | ||
| /// </summary> | ||
| internal const string DefaultCompression = CompressionLz4; |
|
Thanks — one of these was a real bug. Addressed in 9f29035. Fixed: a failed framed end-of-input marker left the connection reusable. Confirmed and fixed. The query path encodes the Query packet inside a try whose catch resets the buffer and returns the connection to
Fixed: the frame size caps. Right, and the comment claiming a bad length could not drive a huge rent was self-contradictory. Both sizes come from the peer and each sizes a rent before the checksum can be verified. Now 128 MiB rather than 1 GiB — ample over the server's ~1 MiB flush, and the same figure Fixed: both documentation defects. The builder still documented the default as Declined: the On the benchmark for default-on LZ4 (suppressed comment): fair, and unresolved. I tried to answer it and the honest result is that I cannot with the instrument available:
So the default currently rests on matching Full suite after the fixes: 3018 pass, 0 fail on net9.0 against a real server. |
9f29035 to
09b01c4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
09b01c4 to
589ca4b
Compare
589ca4b to
5869122
Compare
5869122 to
5ec75fb
Compare
5ec75fb to
4d53319
Compare
4d53319 to
420993d
Compare
420993d to
296f2f4
Compare
296f2f4 to
c5556e8
Compare
c5556e8 to
b4850c2
Compare
b4850c2 to
c10b42f
Compare
c10b42f to
3219859
Compare
The frame's 16-byte checksum is CityHash128 from the historical 1.0.2 release, not the modern Google one: release 1.1 changed the 128-bit functions, so a 1.1+ implementation disagrees on every input and the server rejects every frame it writes. No NuGet package ships 1.0.2, so this is a port of the reference city.cc that ClickHouse vendors as contrib/cityhash102. Only the 128-bit entry point is ported. CityHash64 and HashLen17to32 are unreachable from it, so porting them would add untested surface. Two details a port gets wrong silently. CityMurmur compares a signed ssize_t, so a length below 16 must compare as negative. The tail of a long input walks backwards from the current position and reads bytes before it, which is in range only because the main loop already consumed 128 of them. The expected values in the tests come from compiling that reference and hashing the same inputs, so they pin the port against the implementation the server uses rather than against our own output. The lengths cover every branch, including the 8-to-15 case that seeds from an empty span and the lengths past 128 whose tail walks backwards. Co-Authored-By: Claude <noreply@anthropic.com>
A frame is a 16-byte CityHash128 checksum, a 9-byte header of method and two sizes, then the body. The two spans it declares are not the same and are easy to confuse: the checksum covers the header plus the body, while compressed_size counts the header plus the body and excludes the checksum itself. Both are asserted separately. Pure span work with the caller owning the I/O, because the same frame format appears on the native protocol's packet bodies and on HTTP's compress/decompress stream, so the HTTP path can reuse this later. Decoding follows the frame's own method byte, not what the caller configured to write: the server picks its codec per query from network_compression_method, so a client that asked for LZ4 can still be sent ZSTD. Both declared sizes come from the peer, so both are range-checked before they size a rent, and an unknown method byte names the ones we do support rather than failing opaquely. Co-Authored-By: Claude <noreply@anthropic.com>
A body arrives and leaves as a stream of frames rather than one frame, so these stream them, and the block and column codecs above read and write plaintext without learning that compression is on. Only the body is framed: the packet type code and the table name stay on the raw stream, which is why the switch happens between them rather than around the whole packet. The reader pulls frames through the connection's raw reader instead of the socket, so there stays one owner of buffered socket bytes. Its read-ahead stops at the current frame and it serves a short read at the boundary, pulling the next frame only when a caller asks for more: reading ahead past a block's last frame would consume the next packet's uncompressed envelope as frame bytes. EndBlock asserts a block consumed its frames exactly. The sender flushes at each block end, so a block's last frame ends where the block does, and anything left over means the decoders and the peer disagree about the body's length. The writer cuts plaintext into frames of at most the server's own ~1 MiB and flushes each one, so peak buffering is a single frame however large the block. A frame boundary inside a body is legal, since a reader finds a block's end from its own dimensions rather than from the framing. Co-Authored-By: Claude <noreply@anthropic.com>
The Query packet's compression flag stops being a hardcoded false, and a Compressor option chooses the codec (Compression=lz4|zstd|none on a connection string). The default stays off in this commit so nothing about existing behaviour moves; the next one flips it. Only a body is framed, so the name a packet carries is split from it on both sides: BlockReader and BlockWriter grow body-only entry points, and the connection reads the name from the raw reader before choosing which reader decodes the rest. That leaves the block reader and all forty column codecs untouched — they are handed a reader and cannot tell which one it is. Not every block-bearing packet is framed. Log and ProfileEvents carry blocks and go through the same block reader, yet the server sends them uncompressed at our protocol target, so framing them would read a block name as a frame checksum. FramedPackets states the list once — Data, Totals, Extremes — and a test pins it, including that every other packet stays out. The Go client's ServerCode.Compressible admits the same three. The codec is fixed for a connection's life rather than per query, because the option is client-level; the wire flag is still written per query. A per-query override needs an API that can say "off for this query" when the client default is on, which a nullable codec cannot express, so it is left for its own change. An HTTP-only codec is refused when the client is built: GZip and Brotli throw from MethodByte, and finding that mid-query would leave the Query packet already promising the server frames. IsReusable now also refuses a connection holding undelivered plaintext, the same fault as leftover raw bytes one layer up. An abandoned response needed no new handling: the query path already terminates a connection that did not complete, so frame state cannot outlive it. Verified against a real server: 3015 pass, 0 fail. The new integration cases cover both codecs over bodies spanning several frames in both directions, Log packets arriving mid-stream while compressed, framed Totals, sequential compressed queries on one pooled connection, and the server answering ZSTD to a client that asked for LZ4. Co-Authored-By: Claude <noreply@anthropic.com>
One line, so that if this breaks something the cause is not in doubt. LZ4 is what clickhouse-client uses on this protocol: cheapest in CPU and lightest on the server. Compression=none, or a null Compressor, turns it off. The whole TCP suite now runs compressed: 3015 pass, 0 fail, so every existing integration test round-trips through the frame layer rather than only the cases written for it. A test pins the default in all three places it is observable — the options record, a connection string with no Compression key, and the builder. Co-Authored-By: Claude <noreply@anthropic.com>
Four findings from the Copilot review, one of them a real bug. **A failed framed end-of-input marker left the connection reusable.** The query path encodes the Query packet into the buffer inside a try whose catch resets the buffer and returns the connection to Ready, on the premise that nothing can have been sent yet. Framing the marker broke that premise: each frame flushes as it is emitted, so a socket failure or a cancellation there left the Query packet on the wire while the connection went back to the pool looking healthy, and the next caller to lease it would read the previous query's response. Only Query.Write stays in that block now; the marker moved into the outer try, whose failure terminates. The insert path already terminated on any failure, so it needed no change. A test pins it, and the old structure was restored to confirm the test fails on it — Ready instead of Terminated, IsReusable true instead of false. The insert case passes either way, so it is a guard rather than a regression test. **The frame size caps allowed two ~1 GiB rents.** Both sizes come from the peer and each one sizes a rent before the checksum can be checked, so 1 GiB apiece contradicted the comment claiming a bad length could not drive a huge rent. Now 128 MiB, ample over the server's ~1 MiB flush, and the same figure the Go client uses. **Two documentation defects.** The connection-string builder still documented the default as none after the flip to lz4, and Validate's summary and exception tags had been left stranded above ResolveCompressor, so Validate was undocumented and the tags described the wrong member. Declined: a changelog.d fragment. Every PR in this stack deliberately adds none (R3, confirmed 2026-08-17), and the client is experimental with no packaging yet, so there is no released behaviour to describe. R3 now names this default explicitly so the sweep cannot miss it. Also filed T1a: the default is on with LZ4 but no benchmark backs it. Wire size is a measured 1.85-2.00x win, while a loopback probe was inconclusive and possibly negative — it pays the CPU with no bandwidth to save. T1/T2 must measure over a realistic path and revisit the default if LZ4 costs there. Co-Authored-By: Claude <noreply@anthropic.com>
3219859 to
7464bed
Compare
Compression on the native protocol, in both directions, on by default with LZ4.
Compression=lz4|zstd|noneon a connection string, orClickHouseTcpClientOptions.Compressorfor a codec instance.The frame, and the two spans that are easy to confuse
The checksum covers the header and body, so the declared sizes are protected too;
compressed_sizecounts the header but not the checksum. Both spans are asserted separately.CityHash needed its own implementation. ClickHouse uses CityHash v1.0.2, the historical variant; release 1.1 changed the 128-bit functions, so any 1.1+ package disagrees on every input and every frame it writes is rejected. No NuGet package ships 1.0.2, so
CityHash102is a port of the referencecity.ccthat ClickHouse vendors ascontrib/cityhash102. Two details a port gets wrong silently, both commented in place:CityMurmurcompares a signedssize_t, so a length below 16 must compare as negative; and the tail of a long input walks backwards past the current position, which is in range only because the main loop already consumed 128 bytes.The expected values in the tests come from compiling that reference and hashing the same inputs, so they pin the port against the implementation the server uses rather than against our own output. As an end-to-end check, that same reference reproduces byte for byte the checksum on a real 4 KB LZ4 frame the server produced over HTTP
compress=1(low=0x931eb99176f35655 high=0xed539c8c0e18acc2). The generator is kept atignored-docs/tcp/cityhash102-vectors.cc.Design decisions and trade-offs
Where plaintext lands: two readers, not one. The frame decoder reads compressed bytes from the existing raw buffer — one owner of socket bytes — and decodes into a plaintext array. The question is how values are then decoded from it:
ReadBufferClickHouseBinaryReaderIByteSourceinterfaceB is tempting — one less copy, one less buffer, and a simpler leftover invariant — but its cost is global: it de-inlines a
sealedclass onReadVarUIntAsync, the hottest loop in the driver, for everyone. And A's copy is smaller than it looks, becauseReadBuffer.ReadIntoAsyncalready reads bulk payloads (whole columns) straight from the frame into the destination; the second buffer only handles the metadata minority of bytes. Filed as B3a with the condition that a T1 read-throughput number must justify it first.Read-ahead stops at the current frame.
ReadBufferis deliberately greedy — asked for one byte it will take 16 KiB — and that is not changed. Instead the frame source refuses to serve across a boundary, so a greedy request gets a short read. The rule that falls out is: a frame is pulled only inside a read that is still unsatisfied, i.e. only while a decoder is waiting. This is load-bearing rather than tidy: reading ahead past a block's last frame would consume the next packet's uncompressed envelope as frame bytes, and a test asserts the envelope survives untouched.EndBlock()asserts a block consumed its frames exactly. The format's invariant runs one way only — every block end is a frame boundary, but a frame boundary is not a block end — and the forward direction is what makes an assertion possible. Leftover plaintext means the column decoders and the peer disagree about the body's length; failing there beats desynchronising three blocks later.Only
Data,TotalsandExtremesbodies are framed. This was the sharpest trap.LogandProfileEventsalso carry blocks and go through the same block reader, but the server sends them uncompressed at our 54460 target (they become compressible only at 54481+). Framing them would read a block name as a frame checksum, breaking any query against a server withsend_logs_levelset.FramedPacketsstates the list once and a test pins it, including that every other packet stays out.Write side streams frames. Plaintext is cut into frames of at most the server's own ~1 MiB and each finished frame is flushed, so outstanding compressed bytes stay at one frame however large the block. The existing between-column flush threshold still bounds plaintext one level up, so the two mechanisms compose.
Default on with LZ4, as the cheapest codec in CPU and the lightest on the server — what
clickhouse-clientuses on this protocol. It is flipped in its own one-line commit, after the wiring commit, so that if the flip breaks something the cause is not in doubt.A per-query override is deliberately absent. The wire flag is per query, but a nullable
IClickHouseCompressorcannot express "off for this query" when the client default is on — null already means inherit. That wants a small three-state type and its own decision, filed as B3b. Today the codec is fixed for a connection's life; the flag is still written per query.An HTTP-only codec is refused at construction.
GZipCompressorandBrotliCompressorthrow fromMethodByte. Finding that mid-query would leave the Query packet already promising the server frames.Differences from the Go client
I read
ch-goandclickhouse-gorather than working from memory, and the comparison changed two things in this PR.Same split point. ch-go reads the table name from the raw stream, then calls
EnableCompression()before decoding the block. So "compression switches on mid-packet" is forced by the format, not by our layering.They swap the source in place; we cannot.
proto.Readerholdsraw,dataanddecompressed, and toggling isr.data = r.decompressed. That is safe only because their plaintext path does exactio.ReadFullreads and therefore never buffers, while their greedy buffer (bufio, 128 KiB) sits below the split. OurReadBufferis greedy by design, so an in-place swap would serve greedily-read frame bytes as plaintext. Hence two buffers.The flip side favours ours. In ch-go, a column decoder that under-reads leaves plaintext in
r.data;DisableCompression()swaps away, and the nextEnableCompression()findsr.pos < len(r.data)and serves stale bytes from the previous block's frame before pulling a new one. Silent desync, and the checksum cannot catch it because every frame was individually valid. OurEndBlock()throws at the block that caused it.ServerCode.Compressible()admits the same three packets we do — Data, Totals, Extremes. That cross-check is what caught the Log/ProfileEvents trap here before it caught us.They buffer, we stream.
encodeBlockwrites the body into a buffer, compresses that region and splices the frame back over it, one frame per block, with a source comment admitting it:// TODO(tdakkota): find out if we can actually stream compressed blocks.That holds the whole plaintext and a compressed copy, which is whyclickhouse-goneedsMaxCompressionBuffer. Ours is bounded at one frame.Smaller points. Their header offsets confirm ours byte for byte (
hMethod = 16,hRawSize = 17,hDataSize = 21). Their size caps are 128 MB where mine were 1 GiB, copied from our reader's string cap — theirs is the saner bound on a peer-controlled allocation and I have left a note to tighten.CompressionNonein clickhouse-go means do not frame, matching our nullCompressor, so method0x02is in practice read-only. AndencodedLZ4HC == encodedLZ4, so compression effort never changes the wire method — good for ourLz4Level.Verification
UInt64s straddling boundaries with a frame target that is not a multiple of 8, the next packet's envelope left unread, the leftover assertion, corruption).Logpackets arriving mid-stream while compressed, framedTotals, sequential compressed queries on one pooled connection, a compressed and an uncompressed client against the same server, and the server answering ZSTD to a client that asked for LZ4 (the reader dispatches per frame, not on what we configured).Notes
ClickHouse.Driver.Tcp.Testsproduces a report in which all 460 classes readline-rate="0", including files this PR does not touch, while 3015 tests pass — so the report is empty rather than the code uncovered. Pre-existing and filed as S18; likely theFody/InlineMethodweaver dropping the sequence points coverlet counts.🤖 Generated with Claude Code