Skip to content

Grow column bodies and wire strings as the data arrives - #591

Open
iliaal wants to merge 2 commits into
ClickHouse:masterfrom
iliaal:fix/declared-size-allocations
Open

iliaal wants to merge 2 commits into
ClickHouse:masterfrom
iliaal:fix/declared-size-allocations

Conversation

@iliaal

@iliaal iliaal commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Several column loaders size their buffers from server-declared counts before any data has arrived. A packet of a few dozen bytes can therefore make the client allocate gigabytes:

  • ColumnVector, ColumnEnum and ColumnFixedString LoadBody resize to the declared row count.
  • ColumnString::LoadBody reserves the declared row count and allocates each declared string length up front.
  • WireFormat::ReadString zero-fills up to the declared length.

For example, a UInt64 Data block that declares 2^28 rows and then sends nothing reaches 2 GiB of resident memory in under a second. So does a single 2^31-byte String length or FixedString(2147483648) with one row.

Fix: new WireFormat::ReadPODArray starts at 1 MiB, reserves eightfold ahead as data arrives, and zero-fills only the chunk it's about to read. The column bodies and ReadString use it. ColumnString caps its initial row reservation and reads strings above 1 MiB into a dedicated block that grows the same way. The growth arithmetic (WireFormat::NextReadCapacity) can't overflow a 32-bit size_t.

Cost: default 64K-row blocks fit the first step and pay nothing extra. A single 80 MB column body copies 73 MiB while growing (1 + 8 + 64 MiB).

ColumnLowCardinality::Load also ignored the result of loading the index column. With incremental loading a truncated index would come back shorter than the declared row count, so it now throws ProtocolError.

On 32-bit, Client::Impl::ReadBlock() truncated the uint64_t column and row counts to size_t, so a block declaring 2^41 rows came back empty instead of failing. Counts that don't fit now throw ProtocolError.

The FixedString width × rows overflow check is left to #579.

Tests:

  • ut/allocation_tracker.cpp records the largest allocation inside a scope. It self-checks that it sees library allocations, and the tests that depend on it skip if it doesn't (for example with a DLL build).
  • Seven regression tests assert that a truncated body with a huge declared size throws without any single allocation above 8 MiB. They fail on master, e.g. actual: 134217728 vs 8388608.
  • Round-trip tests cover numeric bodies and strings above 8 MiB, which exercises the growth path.
  • ColumnLowCardinalityString_Load_TruncatedIndex covers the short index.
  • CodedStreamCase.NextReadCapacityDoesNotOverflow checks the growth arithmetic in uint32_t.
  • ClientProtocol.RowCountBeyondSizeTThrowsProtocolError runs only where size_t is narrower than 64 bits and skips elsewhere. I ran it in an i386 build (with -Werror off, since master doesn't build warning-clean there): it fails on master and passes with the fix.

ColumnVector, ColumnEnum and ColumnFixedString resized to the declared
row count before reading, ColumnString reserved the declared row count
and allocated each declared string length up front, and
WireFormat::ReadString resized to the declared length, so a packet of a
few dozen bytes could make the client allocate gigabytes.

I added WireFormat::ReadPODArray, which starts at 1 MiB, reserves
eightfold ahead and zero-fills only what it is about to read, and use
it for these bodies and for ReadString. ColumnString caps its initial
row reservation and reads strings above 1 MiB into a block of their
own that grows the same way. Default 64K-row blocks fit the first step
and cost nothing extra; a single 80 MB column body copies 73 MiB while
growing.

LowCardinality's Load also ignored a short index column, which with
incremental loading would deliver fewer rows than declared, so it now
throws ProtocolError.
Client::Impl::ReadBlock() reads the block's column and row counts as
uint64_t and passes the row count to Column::Load(), which takes a
size_t. On 32-bit, a server that declares 2^41 rows has the count
truncated to 0, and the client returns an empty block instead of an
error. Counts that do not survive the conversion now throw
ProtocolError.

The test only runs where size_t is narrower than 64 bits and skips
elsewhere.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant