perf(driver): stream ClickHouse HTTP responses instead of full-text buffering - #74
Merged
Merged
Conversation
…uffering. Closes #49 db.query's tabular path called reqwest::Response::text(), forcing the entire HTTP response body into one contiguous String before any parsing started. For a large analytical SELECT (tens of MB of JSON lines), the raw text and the parsed QueryResult rows coexisted in memory at once, risking OOM under the 256 MB ClickHouse Sandbox ceiling. - New ClickHouseClient::post_sql_response returns the raw streaming reqwest::Response on success (reusing the existing readonly-retry logic) instead of buffering it into a String. - New src/driver/streaming::stream_compact_output reads that response's bytes_stream() through a LinesCodec-based FramedRead and parses rows incrementally as they arrive, stopping — without reading the rest of the network response — once `limit` rows are parsed or a 200 MB safety byte cap is hit even when no limit was given. - Extracted parse_columns/parse_and_normalize_row out of parse_compact_output so both the buffered (schema introspection, mock mode, tree building) and streaming (large query results) parsers share identical row normalization. - QueryResult gained isTruncated, set whenever limit or the safety cap cut a result short, so callers can tell rows isn't the complete set. - handle_query's tabular branch now goes through post_sql_response + stream_compact_output; the non-tabular (mutation/DDL) branch is unchanged, since those responses are always small.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
db.query's tabular path calledreqwest::Response::text(), forcing the entire HTTP response body into one contiguousStringbefore any parsing started. For a large analyticalSELECT(tens of MB of JSON lines), the raw text and the parsedQueryResultrows coexisted in memory at once, risking OOM under the 256 MB ClickHouse Sandbox ceiling.ClickHouseClient::post_sql_response(src/driver/client.rs) returns the raw streamingreqwest::Responseon success — reusing the existing readonly-setting-conflict retry logic — instead of buffering it into aString.src/driver/streaming::stream_compact_outputreads that response'sbytes_stream()through aLinesCodec-basedFramedReadand parses rows incrementally as they arrive, stopping — without reading the rest of the network response — oncelimitrows are parsed or a 200 MB safety byte cap is hit, even when nolimitwas given at all.parse_columns/parse_and_normalize_rowout ofparse_compact_output(src/mapper/row_compact.rs) so both the buffered parser (schema introspection, mock mode, tree building — all small, bounded responses) and the new streaming parser (large query results) share identical row normalization instead of two copies of the same logic.QueryResultgainedisTruncated, set wheneverlimitor the safety cap cut a result short, so callers can tellrowsisn't the complete set — also backfilled onto the bufferedparse_compact_outputpath for consistency.handle_query's tabular branch now goes throughpost_sql_response+stream_compact_output; the non-tabular (mutation/DDL) branch is unchanged since those ClickHouse responses are always small.Closes #49
Test plan
cargo test— 110/110 passing, including newdriver::streamingtests (row parsing, limit truncation +isTruncated, chunk-boundary line reassembly — bytes delivered one at a time to prove the codec correctly reassembles lines split across network chunks, empty body, malformed input) and updatedrow_compacttests forisTruncatedcargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warnings