Skip to content

fix: prevent IVF-RQ merge stalls and speed up priority tracking - #9431

Closed
jackye1995 wants to merge 2 commits into
lance-format:mainfrom
jackye1995:codex/fix-ivfrq-merge-scheduler
Closed

jackye1995 wants to merge 2 commits into
lance-format:mainfrom
jackye1995:codex/fix-ivfrq-merge-scheduler

Conversation

@jackye1995

@jackye1995 jackye1995 commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Large IVF-RQ merge backlogs repeatedly shift sorted priority arrays. Use a shared counted BTreeMap in both I/O schedulers to reduce priority-accounting overhead while preserving duplicate reservations and minimum-priority admission.

Fix a read-backpressure deadlock by loading each scan line's pages concurrently and accepting them in their original order. Add regression coverage for concurrent loading, output order, and priority accounting.

Git history traces the affected implementations to:

  • Priority-accounting overhead: #2721 (30b3df75b, August 2024) introduced sorted-vector insertion/removal; #5773 copied it into the lite scheduler. The original assumption of a small backlog breaks down with many completed but unconsumed reads.
  • Page-loading deadlock: #2968 (b1abfff9b, October 2024) introduced sequential scan-line page awaits. Later pages can hold the read budget needed by the page being awaited.

The two-file shuffle path originated in #6169. #8894 added offset preloading and coalesced reads, but this 1B workload exceeds the 256 MiB offset-preload cap and takes the per-partition fallback. These are confirmed code origins and the observed triggering conditions; a historical first-failing release has not been established by bisect.

Modern distributed IVF-PQ uses the same shuffle and merge implementation. RQ's additional factor columns and code width change page layout and flush-group count, which can change whether the offset-preload cap is exceeded. The earlier successful PQ workload has not been matched against this run, so an RQ-specific regression has not been established.

A complete merge of 1 billion vectors across 31,623 partitions finished in 1h 02m 44s, including output finalization, with the standard scheduler and a warm cache. The full output audit passed exact unique row-ID and per-partition row-count checks, plus per-partition code/factor checksum checks.

The decoder regression times out before the fix and passes afterward. All 15 decoder tests, 31 scheduler tests, workspace Rust clippy, formatting, and 37 CI checks passed.

@github-actions github-actions Bot added A-encoding Encoding, IO, file reader/writer performance labels Sep 19, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Gate recommendation: approve.

The counted ordered map preserves duplicate reservations, minimum-priority admission, and total outstanding counts while eliminating sorted-array shifts on every update. A single shared tracker is preferable to separate or hybrid paths because it addresses the large-backlog cost without adding transition state.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 19, 2026
@jackye1995 jackye1995 changed the title perf(io): avoid quadratic in-flight priority tracking fix: prevent IVF-RQ merge stalls and speed up priority tracking Sep 19, 2026
@github-actions github-actions Bot added the bug Something isn't working label Sep 19, 2026
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 19, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Gate recommendation: approve.

The counted ordered map preserves duplicate reservations and minimum-priority admission while eliminating sorted-array shifts. Polling each scan line’s page futures together breaks the I/O backpressure dependency cycle, and try_join_all retains decoder order; the focused regression covers both progress and output ordering.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 19, 2026
jackye1995 added a commit that referenced this pull request Sep 19, 2026
Allow IVF shuffle offset preloading to be tuned with
`LANCE_SHUFFLE_MAX_PRELOADED_OFFSETS_BYTES`. The default remains 256
MiB; `536870912` allows 512 MiB and `0` disables preloading. Reject
malformed values, apply the setting to writer handoff and reopened
readers, and document its per-shuffle memory cost.

Replayed the same complete 1B-row IVF_RQ shuffle: 31,623 partitions and
381 MiB decoded offsets, with 72 CPU threads and unchanged merge
budgets.

| Merge measurement | #9431 + this PR, 256 MiB | #9431 + this PR, 512
MiB | This PR alone, 512 MiB |
| --- | ---: | ---: | ---: |
| Wall time | 61.01 min | 13.38 min | 13.50 min |
| CPU time | 15,726 s | 2,038 s | 2,058 s |
| Average CPU cores | 4.30 | 2.54 | 2.54 |
| Observed RSS high-water mark | 14.21 GiB | 9.64 GiB | 9.43 GiB |

The final column excludes **both** #9431 changes: it uses the original
sorted-vector scheduler and sequential structural-page loading. All
three full outputs passed exact row-ID/partition-count checks and
code/factor fingerprint checks. The standalone run also captured a
successful process exit and complete resource accounting.

At 512 MiB, this input's offset table fits in memory and enables
coalesced partition reads. This PR alone completed within 1% of the
elapsed and CPU time measured with both PRs. This establishes that #9431
is unnecessary for this particular preloaded replay; it does not fix the
original fallback path when offsets exceed the configured cap. The
61-minute baseline already includes #9431 and is not an unfixed
baseline.

These are single measurements with warm caches: the first two ran
sequentially on one worker, and the standalone run used a matching
instance in us-west-2. Background input writeback continued during the
256 MiB baseline; the two 512 MiB runs began after it finished. All
merges recorded zero physical input reads. Startup and output auditing
are excluded; memory uses one-second RSS high-water observations. Small
differences between the 512 MiB runs should not be treated as
established performance effects.

Formatting, all 39 shuffle tests, full-workspace Rust clippy, and all 37
CI checks passed.
@jackye1995 jackye1995 closed this Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-encoding Encoding, IO, file reader/writer bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance. performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant