Repository navigation
fix: recover blob reads racing WAL reclamation - #411
Merged
Merged
Conversation
beinan
marked this pull request as ready for review
October 20, 2026 19:25
beinan
added a commit
that referenced
this pull request
Oct 20, 2026
Blob misses currently reacquire an exclusive store guard to refresh the resident dataset. A concurrent append/flush holds a shared guard, so one absent-ID lookup can wait behind the write and queue later blob hits behind itself. Keep blob lookups under a shared guard and refresh cloned base snapshots around WAL fallback. After a complete WAL miss, check the latest base again so a row merged and drained between the initial base check and WAL discovery remains readable. Preserve resident and pinned versions, read caps, the base-hit fast path, and the bounded missing-file retries from #411. Validation: - Before the fix, the server regression times out while an independent shared guard is held. - Before the fix, the deterministic core regression returns `None` for a row flushed before the read and merged during the base-to-WAL transition. - After the fix: 31 blob tests (21 core, 10 server) and 4 point-lookup tests (2 core, 2 server) pass, including both regressions. `cargo fmt --all -- --check` also passes. Full CI remains in progress. - Isolated Azure staging with a 291,593,422-byte same-table write, 53,248-byte blob reads, 32 pending generations, and concurrent hit/miss streams produced the maximum header latencies below. Each run used a fresh storage prefix; no OOM or container restart occurred. These are three sequential synthetic runs, not production p99 measurements. | Image/run | Hit maximum | Miss maximum | |---|---:|---:| | c9aa control | 4.046 s | 4.149 s | | Patched run 1 | 0.0021 s | 0.159 s | | Patched run 2 | 0.0058 s | 2.100 s | The second patched miss still took 2.1 s: this does not resolve every source of read latency. The runtime image IDs matched the immutable image built from this PR's exact head (`768495e`) in services build [2159160](https://dev.azure.com/MicrosoftAI/a05dc5e9-6da2-404a-8da3-c43822735c75/_build/results?buildId=2159160). A separate isolated five-worker fixture with 100,586,357-byte payloads passed six status checks and four full payload comparisons during five concurrent writes. All three lookup types (base hit, WAL hit, absent ID) overlapped actual merges; the four writer shards reclaimed 32 generations. Maximum read header latency was 0.849 s, with no OOM/restarts. This is a bounded correctness run, not a soak test. All four fixture Pods were deleted and verified absent. Production has not been changed. Co-authored-by: Beinan Wang <beinanwang@microsoft.com>
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
1 participant
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.
A rollout blob read can open a WAL generation, then lose its data files when a concurrent merge commits and reclaims that generation. The subsequent ID scan or payload read currently returns a missing-file error to HTTP as a 500; the existing final-miss refresh does not handle errors.
Retry missing-file failures at most twice from a refreshed, cloned base snapshot. Successful base hits keep their existing fast path, historical reads stay pinned, and persistent missing data remains an error. The resident dataset is unchanged and no writer lock or merge scheduling behavior changes.
Validation:
3be40eef: pause immediately after opening the generation, merge and reclaim it, then resume the read. Confirmed failure withNotFoundon the generation data file (not a timing-only stress result).cargo test --locked -p lance-context-core --lib blob -- --nocapture: 20 passed, including the deterministic race, persistent-loss guard, WAL read cap, and pinned historical snapshot checks.cargo test --locked -p lance-context-core --lib point_lookup_ -- --nocapture: 2 passed, covering base-first lookup and immutable-ID precedence.cargo fmt --allandgit diff --checkpassed.An isolated Azure workload also reproduced this race on the older image. This is not yet an attribution of production slow reads or proof of production performance improvement.