Skip to content

perf(eap): Stream RowBinary INSERTs instead of buffering - #8462

Open
phacops wants to merge 2 commits into
masterfrom
perf/stream-rowbinary-inserts
Open

perf(eap): Stream RowBinary INSERTs instead of buffering#8462
phacops wants to merge 2 commits into
masterfrom
perf/stream-rowbinary-inserts

Conversation

@phacops

@phacops phacops commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

RowBinary consumers now stream compressed INSERT bodies to ClickHouse as rows arrive, instead of accumulating the full uncompressed batch in Reduce before the write.

--use-row-binary (already on for EAP items) swaps Reduce → RowBinaryWriterStep for StreamingClickhouseWriter. Each message is LZ4-framed into ClickHouse-native 1 MiB blocks and pushed onto an in-flight INSERT … FORMAT RowBinary POST; uncompressed bytes are dropped in submit(). JSON storages stay on the buffered writer.

This is a re-land of #8001 with the two revert causes fixed:

  • Concurrency. perf(eap): Stream RowBinary INSERTs to ClickHouse instead of buffering #8001 allowed one in-flight POST and rejected submits for the duration of insert_distributed_sync=1, which showed up as consumer pause time. This keeps clickhouse_concurrency slots and completes them in order so a later offset cannot commit before an earlier write.
  • Partial-body retries. Closing the HTTP body is end-of-INSERT for ClickHouse. The live attempt streams as blocks are produced, but retries wait for body_complete so they replay the full compressed body, not a prefix. Abandoning a batch (rebalance / join timeout) aborts the HTTP task before dropping the sender, so a connection reset cannot land as a successful truncated write.

The first-attempt request timeout is request_ms + max_batch_time so the open connection covers accumulation plus the write; retries use plain request_ms. Peak writer memory for a batch of size B is the compressed retry buffer (~0.3×B) rather than the uncompressed Reduce accumulator.

Watch EAP canary RSS and pause time on deploy. The ClickHouse MEMORY_LIMIT_EXCEEDED / max_insert_block_size lever from #7939 is unchanged.

Replace Reduce → RowBinaryWriterStep with a streaming writer so EAP
consumers compress and POST RowBinary blocks as messages arrive instead
of holding the full uncompressed batch.

Unlike #8001, keep clickhouse_concurrency in-flight slots and only retry
after the body is complete so a mid-stream failure cannot ACK a truncated
insert.

Refs LINEAR-EAP-517
@phacops
phacops marked this pull request as ready for review September 10, 2026 21:13
@phacops
phacops requested a review from a team as a code owner September 10, 2026 21:13

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1a628e4. Configure here.

Comment thread rust_snuba/src/strategies/clickhouse/streaming_writer.rs
RunTaskInThreads can deliver many completed messages in one cycle with
no intervening poll. Close the INSERT body in submit once max_batch_size
is reached so a single streamed write cannot grow unbounded.
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