Skip to content

perf(scan): overlap remote split reads within decode concurrency - #107

Open
peasee wants to merge 6 commits into
spiceai-55from
peasee/scan-split-io-overlap
Open

peasee wants to merge 6 commits into
spiceai-55from
peasee/scan-split-io-overlap

Conversation

@peasee

@peasee peasee commented Oct 24, 2026 •

Copy link
Copy Markdown

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.

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.

Signed-off-by: peasee <98815791+peasee@users.noreply.github.com>
Signed-off-by: peasee <98815791+peasee@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 24, 2026 02:10
@peasee peasee added the changelog/performance A performance improvement label Oct 24, 2026
Signed-off-by: peasee <98815791+peasee@users.noreply.github.com>

Copilot AI 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.

🟡 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.

2 open issues
What changed in this PR

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.
vortex-layout/​src/​scan/​mod.rs Register the split gate module.
vortex-layout/​Cargo.toml Enable Tokio features needed by gate tests.
vortex-file/​src/​open.rs Wrap file segment sources with wait tracking.

Review effort: Balanced

Comment thread vortex-layout/src/scan/split_gate.rs Resolved
Copilot AI balanced review requested due to automatic review settings October 24, 2026 02:14

Copilot AI 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.

🔵 Needs a closer look

Permit wakeups can leave decode capacity idle, and shared layout futures do not attribute I/O waits to every affected split.

2 open issues

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 24, 2026 02:56

Copilot AI 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.

🔵 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

Medium severity 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.

2 resolved since last review

Review effort: Balanced

@peasee peasee self-assigned this Oct 26, 2026
Copilot AI balanced review requested due to automatic review settings October 26, 2026 01:35

Copilot AI 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.

🔵 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

Medium severity 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.

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 26, 2026 01:42

Copilot AI 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.

🟡 Changes recommended

The new gated stream path needs scan-level tests for admission bounds, output backpressure, and ordered-scan bypass.

1 open issue

Review effort: Balanced

Comment on lines +261 to +263
let gate = match self.max_splits_in_flight {
Some(max) if !self.ordered && max > concurrency => Some(SplitGate::new(concurrency)),
_ => None,

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/performance A performance improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants