perf(rust): reduce per-query overhead and coalesce small batches#422
Open
vikrantpuppala wants to merge 1 commit intomainfrom
Open
perf(rust): reduce per-query overhead and coalesce small batches#422vikrantpuppala wants to merge 1 commit intomainfrom
vikrantpuppala wants to merge 1 commit intomainfrom
Conversation
Four small kernel changes that together close the per-query gap vs the existing Thrift backend on small/medium results, with no regressions on large CloudFetch queries. 1. Skip redundant DELETE for inline-Closed statements. The SEA server returns status=Closed alongside inline result data — the statement is already cleaned up server-side, so issuing a DELETE is a wasted round-trip (~250ms). Plumb a `server_side_closed: bool` through ExecuteResult; Statement::execute_single skips registering the statement_id for cleanup when set. 2. Make Drop for Statement non-blocking. Drop previously block_on(close_statement(...)), forcing every caller to pay a synchronous cleanup round-trip even when nothing was waiting for the result. Spawn the close on the runtime instead — best-effort fire-and-forget. Saves ~250ms on every CloudFetch query. 3. Coalesce small batches on the inline path. InlineArrowProvider was emitting 200+ tiny RecordBatches per 100K-row result (one per IPC message). Add a batch_merge_target_rows knob and apply the same coalescing logic the CloudFetch download path uses. Reduces per-batch overhead at language bindings (e.g. PyO3, ODBC). 4. Enable batch_merge_target_rows by default (128k rows). Was 0 (disabled). All consumers now get coalesced batches by default; no API change. Measured on dogfood warehouse, randomized interleaved benchmark vs Thrift backend (median wall time, fetchall_arrow path): size Rust (before) -> Rust (after) Thrift ratio SELECT 1 500ms -> 394ms 387ms 1.02x 10K 950ms -> 893ms 1014ms 0.88x 500K 2600ms -> 2178ms 3305ms 0.66x 1M 3700ms -> 3579ms 3814ms 0.94x 10M 8700ms -> 8677ms 8802ms 0.99x
4 tasks
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
Four small kernel changes that together close the per-query gap vs the existing Thrift backend on small/medium results, with no regressions on large CloudFetch queries.
Skip redundant DELETE for inline-Closed statements. The SEA server returns
status=Closedalongside inline result data — the statement is already cleaned up server-side, so issuing a DELETE is a wasted round-trip (~250ms). Plumbs aserver_side_closed: boolthroughExecuteResult;Statement::execute_singleskips registering the statement_id for cleanup when set.Make
Drop for Statementnon-blocking. Drop previously calledblock_on(close_statement(...)), forcing every caller to pay a synchronous cleanup round-trip even when nothing was waiting for the result. Spawn the close on the runtime instead — best-effort fire-and-forget. Saves ~250ms on every CloudFetch query.Coalesce small batches on the inline path.
InlineArrowProviderwas emitting 200+ tinyRecordBatches per 100K-row result (one per IPC message). Adds abatch_merge_target_rowsparameter and applies the same coalescing logic the CloudFetch download path already uses. Reduces per-batch overhead at language bindings (e.g. PyO3, ODBC).Enable
batch_merge_target_rowsby default (128k rows). Was0(disabled). All consumers now get coalesced batches by default; no API change.Behavior change to call out
Default
batch_merge_target_rowsflips from0to128_000. Consumers that previously saw many small batches per chunk will now see ~1 large batch per chunk (post-merge). Set the option explicitly to0to opt out.Benchmark
Dogfood warehouse, randomized interleaved (Rust vs Thrift) benchmark, 20 runs per size, median wall time on
fetchall_arrowpath:SELECT 1Test plan
cargo +stable fmt --all -- --checkcleancargo clippy --all-targets -- -D warningscleancargo test(full suite, 349 tests) passThis pull request and its description were written by Isaac.