You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
perf(scan): overlap remote split reads within decode concurrency - #107
Remote object-store scans can keep more splits waiting for segment bytes while a separate decode permit bounds active decoding. A pending read gives up its decode permit; ordered scans keep their existing path.
ObjectStoreReadAt now accepts an optional, application-supplied I/O-wait callback. It marks metadata requests, range requests, and pending response-stream reads. Spice supplies DataFusion's .io_wait() only for remote stores and carries Ballista's vcore context into Vortex scan and I/O tasks. The callback leaves Vortex independent of DataFusion and is unset for local and in-memory stores.
Integration and evidence
Spice pins this fork at commit 5a3abe10b6890cd0566d819b2c90393c8309c4b5 in spiceai/spiceai#15506. A targeted cargo check -p vortex-io --features object_store,tokio --tests completed successfully; the fork CI's Rust lint, default-feature build, Tokio dispatcher build, MSRV, and wasm build also completed successfully. The fork CI has other failed or cancelled jobs that remain visible in the check list; the coverage job's test log is unavailable, so its failure is not attributed to this change or dismissed as unrelated.
The same-rig cold-tier A/B at Spice commit 3bd6a8e6671cd1c298c688cd16d6c6ca005e6385 measured the split-overlap behavior before these .io_wait() markers were added. Against stored trunk commit 09c9fcc9f214708c9ebd275f7e719a13033a602a, pooled P99 was 1724.114 to 743.138 ms for a tenant scan and 1262.556 to 476.698 ms for a grouped aggregate (1,000 queries per shape per A/B/B/A arm, 20 million cold rows in 14 files, 20 ms S3 proxy delay). All complete result rows matched the retained oracle. The full artifact is /home/owner/coldperf/io-wait-review/abba-3bd6a8e667-20261024T035843Z/. This establishes the split-overlap benefit; it does not measure the new wait callback alone.
A marker-bearing Spice binary at b11d5df5558365172fe94ee97e734d8fb1f403ba has a successful Linux x64 release build. Its 1,000-query-per-shape A/B/B/A on the same fixture measured pooled P99 2197.226 to 755.072 ms for tenant scans and 1289.374 to 481.178 ms for grouped aggregates; trunk A/A spreads were 32.344 and 8.518 ms. All 16,000 timed outputs matched complete oracle rows, and the cold manifest remained unchanged. Artifact: /home/owner/coldperf/io-wait-review/abba-b11d5df555-20261026T024451Z/. Seven candidate proxy BrokenPipeError entries occurred during startup, 5.2–5.6 seconds before first warmup SQL, and none occurred during warmup or timed windows; their cause is unverified. A 16-client aggregate A/B/B/A measured pooled P99 2927.243 to 2257.781 ms (trunk A/A spread 69.579 ms), with all 4,000 complete-row outputs matching and no proxy errors; artifact /home/owner/coldperf/io-wait-review/abba-b11d5df555-20261026T053455Z/. The final Spice head c29d52f752ff037036da802c6d54fb4052b480ab also has a successful release build. Its 20-sample pilot matched all 320 complete-row outputs; its 16-client aggregate A/B/B/A measured pooled P99 2927.764 to 2270.710 ms (trunk A/A spread 96.580 ms), with all 4,000 timed outputs matching the oracle, the cold manifest unchanged, and no proxy errors. Artifact: /home/owner/coldperf/io-wait-review/abba-c29d52f752-20261026T055937Z/. The final head includes newer trunk changes, so the earlier b11d5df555 comparison more closely isolates this scan change.
A clustered cold-tier scan could not be measured: the scheduler requires partition_by for the accelerated table, while Cayenne cold tier does not support partitioned tables. The attempted probe and scheduler log are retained at /home/owner/coldperf/io-wait-review/cluster-lending-control3bd-20261026T015216Z/. The Spice PR includes a focused test comparing vcore lending with and without the Vortex task-context adapter.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Permit wake-up handling and incomplete wait tracking across shared layout futures can leave decode capacity idle and prevent the intended read overlap.
This PR adds opt-in I/O overlap to Vortex’s unordered scans, allowing more remote reads in flight while keeping decode concurrency bounded. Ordered scans retain their existing scheduling path.
Changes:
Add a split gate that releases decode permits during marked segment waits.
Expose an overlap limit through the scan builder and execution pipeline.
Wrap file segment sources with wait tracking and add gate tests.
Checks: Inspected permit handling and shared layout futures. No builds or tests were run.
File
Description
vortex-layout/src/segments/wait_marked.rs
Track pending segment waits for gated splits.
vortex-layout/src/segments/mod.rs
Export the wait-marking source wrapper.
vortex-layout/src/scan/split_gate.rs
Implement decode permits, wait scopes, and gate tests.
vortex-layout/src/scan/scan_builder.rs
Add the optional split I/O overlap setting.
vortex-layout/src/scan/repeated_scan.rs
Apply gating and separate in-flight limits to unordered scans.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Custom permit handoff, cancellation, and stream backpressure warrant human concurrency review, with scan-level integration coverage still missing.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Add scan-level tests for split I/O overlap and in-flight limits
vortex-layout/src/scan/repeated_scan.rs:264
No tests call with_split_io_overlap, so the public scan-stream path is untested even though the gate itself has unit coverage. Add scan-level regression tests using ScanBuilder/execute_stream with a controllable pending source and distinct decode and in-flight limits. Verify that reads overlap without exceeding the in-flight cap, completed outputs retain permits while the consumer is paused, and ordered scans ignore the overlap option.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Shared pruning waits can retain decode permits and suppress the intended read overlap, and the concurrency changes still warrant human review.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Mark per-consumer waits for shared pruning futures
vortex-layout/src/scan/split_gate.rs:238
On cold filtered scans, consumers of the cached SharedPruningResult await it without per-consumer wait marking (vortex-layout/src/layouts/zoned/reader.rs:177). Markers inside the shared future track only its first polling split, so additional waiting splits retain decode permits here and can exhaust them, preventing independent splits from starting reads. Wrap pruning_mask_future with mark_pending_wait at that await and add a regression test with multiple consumers of pending zone-map pruning.
Some(max) if !self.ordered && max > concurrency => Some(SplitGate::new(concurrency)),
_ => None,
This branch has not been deployed
No deployments
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
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
Remote object-store scans can keep more splits waiting for segment bytes while a separate decode permit bounds active decoding. A pending read gives up its decode permit; ordered scans keep their existing path.
ObjectStoreReadAtnow accepts an optional, application-supplied I/O-wait callback. It marks metadata requests, range requests, and pending response-stream reads. Spice supplies DataFusion's.io_wait()only for remote stores and carries Ballista's vcore context into Vortex scan and I/O tasks. The callback leaves Vortex independent of DataFusion and is unset for local and in-memory stores.Integration and evidence
Spice pins this fork at commit
5a3abe10b6890cd0566d819b2c90393c8309c4b5in spiceai/spiceai#15506. A targetedcargo check -p vortex-io --features object_store,tokio --testscompleted successfully; the fork CI's Rust lint, default-feature build, Tokio dispatcher build, MSRV, and wasm build also completed successfully. The fork CI has other failed or cancelled jobs that remain visible in the check list; the coverage job's test log is unavailable, so its failure is not attributed to this change or dismissed as unrelated.The same-rig cold-tier A/B at Spice commit
3bd6a8e6671cd1c298c688cd16d6c6ca005e6385measured the split-overlap behavior before these.io_wait()markers were added. Against stored trunk commit09c9fcc9f214708c9ebd275f7e719a13033a602a, pooled P99 was 1724.114 to 743.138 ms for a tenant scan and 1262.556 to 476.698 ms for a grouped aggregate (1,000 queries per shape per A/B/B/A arm, 20 million cold rows in 14 files, 20 ms S3 proxy delay). All complete result rows matched the retained oracle. The full artifact is/home/owner/coldperf/io-wait-review/abba-3bd6a8e667-20261024T035843Z/. This establishes the split-overlap benefit; it does not measure the new wait callback alone.A marker-bearing Spice binary at
b11d5df5558365172fe94ee97e734d8fb1f403bahas a successful Linux x64 release build. Its 1,000-query-per-shape A/B/B/A on the same fixture measured pooled P99 2197.226 to 755.072 ms for tenant scans and 1289.374 to 481.178 ms for grouped aggregates; trunk A/A spreads were 32.344 and 8.518 ms. All 16,000 timed outputs matched complete oracle rows, and the cold manifest remained unchanged. Artifact:/home/owner/coldperf/io-wait-review/abba-b11d5df555-20261026T024451Z/. Seven candidate proxyBrokenPipeErrorentries occurred during startup, 5.2–5.6 seconds before first warmup SQL, and none occurred during warmup or timed windows; their cause is unverified. A 16-client aggregate A/B/B/A measured pooled P99 2927.243 to 2257.781 ms (trunk A/A spread 69.579 ms), with all 4,000 complete-row outputs matching and no proxy errors; artifact/home/owner/coldperf/io-wait-review/abba-b11d5df555-20261026T053455Z/. The final Spice headc29d52f752ff037036da802c6d54fb4052b480abalso has a successful release build. Its 20-sample pilot matched all 320 complete-row outputs; its 16-client aggregate A/B/B/A measured pooled P99 2927.764 to 2270.710 ms (trunk A/A spread 96.580 ms), with all 4,000 timed outputs matching the oracle, the cold manifest unchanged, and no proxy errors. Artifact:/home/owner/coldperf/io-wait-review/abba-c29d52f752-20261026T055937Z/. The final head includes newer trunk changes, so the earlierb11d5df555comparison more closely isolates this scan change.A clustered cold-tier scan could not be measured: the scheduler requires
partition_byfor the accelerated table, while Cayenne cold tier does not support partitioned tables. The attempted probe and scheduler log are retained at/home/owner/coldperf/io-wait-review/cluster-lending-control3bd-20261026T015216Z/. The Spice PR includes a focused test comparing vcore lending with and without the Vortex task-context adapter.