perf(vector): bucket-scoped CoW keymaps + coalescing snapshot queue; fix pre-existing kill-9 doc loss#228
Merged
Merged
Conversation
VectorIndex's key_hash -> V maps were single Arc<HashMap<u64, V>> instances. Under ft-search-off-eventloop, a live FT.SEARCH snapshot holds an extra Arc clone of the whole map while the event loop keeps processing writes; the next write's Arc::make_mut then sees refcount > 1 and full-clones the entire map synchronously on the shard event loop (~47MB @1m vectors). BucketedKeyMap<V> shards the map into 256 fixed buckets, each an independently-cloneable Arc<HashMap<u64, V>>, with bucket selected by (key_hash >> 56). A concurrent snapshot now pins at most 1/256th of the map per write instead of the whole thing. Manual Clone impl avoids a spurious V: Clone bound that #[derive(Clone)] would add. 8 unit tests: get/insert/remove/len round-trip against a reference HashMap under thousands of random ops, snapshot isolation, and the key property test — after snap = map.snapshot(), one insert leaves >= 255 buckets Arc::ptr_eq with the snapshot. Not yet wired into VectorIndex; that follows in the next commit. author: Tin Dang
Closes the Arc<HashMap> full-clone amplification under live search
snapshots (see previous commit for BucketedKeyMap rationale).
Converts VectorIndex's three key_hash -> V maps from
Arc<HashMap<u64, V>> to BucketedKeyMap<V>:
key_hash_to_key: Arc<HashMap<u64, Bytes>> -> BucketedKeyMap<Bytes>
key_hash_to_global_id: Arc<HashMap<u64, u32>> -> BucketedKeyMap<u32>
key_hash_to_vec_checksum: Arc<HashMap<u64, u64>> -> BucketedKeyMap<u64>
and SearchSnapshot.key_hash_to_key to match. All call sites that used
Arc::make_mut(&mut idx.key_hash_to_*).insert/remove(...) now call
.insert/.remove(...) directly on the BucketedKeyMap (the bucket-scoped
CoW happens inside insert()); two SPSC insert paths use the new
get_or_insert_with() helper in place of the former
.entry(...).or_insert_with(...) pattern.
Read paths (FT.SEARCH response building, session filtering, hybrid
search, recovery) take &BucketedKeyMap<Bytes> instead of &HashMap;
these are read-only signature changes with no behavior change since
BucketedKeyMap mirrors HashMap's get/iter/len/is_empty API.
On-disk keymap.bin format is unchanged: run_snapshot_job's persistence
loop only calls .iter()/.get(), and recover_v2.rs rebuilds the map
from the same on-disk entries via a plain insert loop.
Touches: src/vector/store.rs, src/vector/segment/holder.rs,
src/command/vector_search/{ft_search/execute,ft_search/response,
session,hybrid,hybrid_multi,tests}.rs, src/shard/spsc_handler.rs,
src/shard/scatter_hybrid.rs, src/vector/persistence/recover_v2.rs.
author: Tin Dang
SnapshotPool used an unbounded flume::unbounded::<SnapshotJob>() queue drained by a single worker. If compact/merge cadence outpaced the worker, queued jobs pinned every submitter's Arcs (now BucketedKeyMap buckets) indefinitely, growing unboundedly under sustained load. Replaced the channel with a coalescing slot: Mutex<HashMap<PathBuf, SnapshotJob>> + Condvar, keyed by the job's idx_dir (already unique per index and present on every SnapshotJob, so no new key type is needed). submit() inserts/overwrites the pending entry for that index and notifies one worker; a resubmit for an index whose prior job is still pending (not yet dequeued) replaces it in place, dropping the stale job's data immediately, so only the newest state per index is ever persisted. A job already dequeued and running is unaffected by a concurrent resubmit — the worker loop only removes an entry from the map at the moment it starts running that job. submit() never blocks the caller (map insert + notify, no waiting on capacity). SnapshotJob's three fields are now BucketedKeyMap<Bytes/u32/u64> (no Arc wrapper) to match VectorIndex's field types from the previous commit. run_snapshot_job's body is unchanged since it only calls .iter()/.get() on the map. 3 new tests: same-index submits under a slow worker execute far fewer than the number submitted (final persisted state is the newest job); jobs for distinct indexes all execute; an in-flight job survives a resubmit for the same index (the running job completes normally, the resubmit becomes the new pending entry). Also fixes 2 doc_lazy_continuation clippy lints in the new SnapshotPool doc comment. author: Tin Dang
…ueue fix Records the RSS/CPU wave 4 defects fixed in the previous three commits under [Unreleased]: BucketedKeyMap replacing the single Arc<HashMap> keymap fields, and the coalescing SnapshotPool replacing the unbounded flume queue. author: Tin Dang
…membership gate Two pre-existing, compounding bugs (both reproduce on main; surfaced while validating this branch against crash_recovery_vector_durability — the "wave-4 regression" was actually machine-load timing): 1. force_compact early-return left mutable residue unfrozen. When an explicit FT.COMPACT found a background auto-compaction in flight, it drained that build, installed it, persisted, and RETURNED — without compacting the docs inserted while the background build ran (the clone_suffix(frozen_len) mutable tail). Those docs stayed mutable-only with no durable segment until some future compact, breaking force_compact's full-drain contract (frozen == mutable on reply). Observed stuck durable state: segments live=1961, keymap entries=2000, converging never. force_compact now falls through to the inline drain loop after installing the in-flight build (a no-op when the tail is empty). 2. B3 recovery "verified" keymap entries with no backing segment doc. The durable keymap-<epoch>.bin covers EVERY indexed key (mutable + immutable) at snapshot-submit time, while the paired manifest.json lists only installed immutable segments — so after a kill -9 the on-disk keymap can be a strict SUPERSET of the on-disk segments (bug 1 makes that state persistent; even with bug 1 fixed, a crash between a segment install and its async snapshot commit still produces it transiently). The B3 dedup rescan treated a keymap checksum match as "verified unchanged" and never re-indexed those keys: their documents silently vanished from search (post-crash num_docs 1950/2000, "2000 key(s) verified unchanged, 0 re-indexed", wrong KNN top-1 — decoded from a preserved failing run's artifacts). load_segments_and_keymap now builds the union of live key_hashes across loaded segments (new ImmutableSegment::live_key_hashes, honoring install-time delete_lsn and steady-state tombstones) and drops any keymap entry not in it, so the rescan re-indexes those keys from the AOF — never wrong, only occasionally slower. A loud info line reports the dropped-phantom count. Tests (each red/green verified by toggling its fix): - unit: test_force_compact_drains_tail_inserted_during_inflight_bg_build stages bug 1 deterministically (in-flight bg build + 30 tail inserts, then force_compact; red: 30 docs left mutable, green: 0 and all 130 segment-resident). - unit: recover_reindexes_keymap_entries_not_backed_by_any_segment stages the window deterministically (rewrites the durable keymap with a checksum-matching phantom entry) and asserts the phantom is dropped at load, re-indexed on rescan, and searchable end-to-end. - integration S6 (crash_recovery_vector_durability): kills inside the async-snapshot window (manifest >= 2 segments gate, no full-durability wait) and asserts only the crash contract: num_docs == N and every key answers its own exact-match query. - S1/S2 flake fix: their re_indexed == 0 assertion silently assumed full durability at kill time; a new wait_for_durable_docs pre-kill gate (manifest'd segment live-counts AND keymap entries both == N) makes the precondition explicit — it is what exposed bug 1's never-converging state. Server stdout/stderr logs now open in append mode so the pre-crash generation's log survives restart for post-mortem diagnosis. author: Tin Dang
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds ChangesBucketed keymap migration and recovery/compaction fixes
CI and test hardening
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/crash_recovery_vector_durability.rs (1)
1-1611: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit this test file before it grows further
tests/crash_recovery_vector_durability.rsis at 1611 lines, over the 1500-line cap. Move the shared harness into a support module and split the six scenarios into separate test files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/crash_recovery_vector_durability.rs` around lines 1 - 1611, This test module exceeds the size cap and should be split into smaller files. Move the shared helpers and server/client harness (for example `ServerGuard`, `Client`, `spawn_moon_aof`, `wait_ready`, and the persistence polling utilities) into a reusable support module, then place each scenario test (`s1_...` through `s6_...`) into its own test file that imports that shared module. Keep the existing helper function names and scenario identifiers so the tests remain easy to locate and maintain after the split.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vector/persistence/manifest.rs`:
- Around line 655-657: The shutdown signal in the manifest worker path is being
updated outside the mutex guarding not_empty.wait(), which can let a worker miss
the notify and sleep indefinitely. In the relevant shutdown logic around pending
and not_empty, acquire the same mutex before setting shutdown and hold it while
updating the flag, then notify after the state change so the waiting worker
observes the shutdown reliably.
In `@src/vector/store.rs`:
- Around line 465-476: The manifest/keymap snapshot is being taken too early in
the force-compact flow, after installing the in-flight segment but before the
mutable tail is drained. In the `Store::force_compact` path, move
`persist_hook_after_install` so it runs only after `compact_segments` has fully
drained and persisted the tail, and keep the final hook as the single snapshot
point to avoid a keymap/segment_ids mismatch.
---
Outside diff comments:
In `@tests/crash_recovery_vector_durability.rs`:
- Around line 1-1611: This test module exceeds the size cap and should be split
into smaller files. Move the shared helpers and server/client harness (for
example `ServerGuard`, `Client`, `spawn_moon_aof`, `wait_ready`, and the
persistence polling utilities) into a reusable support module, then place each
scenario test (`s1_...` through `s6_...`) into its own test file that imports
that shared module. Keep the existing helper function names and scenario
identifiers so the tests remain easy to locate and maintain after the split.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84e3e1d7-39d6-49b0-a510-71edd95d082e
📒 Files selected for processing (17)
CHANGELOG.mdsrc/command/vector_search/ft_search/execute.rssrc/command/vector_search/ft_search/response.rssrc/command/vector_search/hybrid.rssrc/command/vector_search/hybrid_multi.rssrc/command/vector_search/session.rssrc/command/vector_search/tests.rssrc/shard/scatter_hybrid.rssrc/shard/spsc_handler.rssrc/vector/keymap.rssrc/vector/mod.rssrc/vector/persistence/manifest.rssrc/vector/persistence/recover_v2.rssrc/vector/segment/holder.rssrc/vector/segment/immutable.rssrc/vector/store.rstests/crash_recovery_vector_durability.rs
Comment on lines
+655
to
+657
| self.shutdown | ||
| .store(true, std::sync::atomic::Ordering::Release); | ||
| self.not_empty.notify_all(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Set shutdown under the condvar mutex.
shutdown is changed outside the mutex used by not_empty.wait(), so a worker can check shutdown == false, miss the notification, and sleep forever. Take pending before setting the flag.
Proposed fix
fn drop(&mut self) {
@@
- self.shutdown
- .store(true, std::sync::atomic::Ordering::Release);
+ let _guard = self.pending.lock();
+ self.shutdown
+ .store(true, std::sync::atomic::Ordering::Release);
self.not_empty.notify_all();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| self.shutdown | |
| .store(true, std::sync::atomic::Ordering::Release); | |
| self.not_empty.notify_all(); | |
| let _guard = self.pending.lock(); | |
| self.shutdown | |
| .store(true, std::sync::atomic::Ordering::Release); | |
| self.not_empty.notify_all(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vector/persistence/manifest.rs` around lines 655 - 657, The shutdown
signal in the manifest worker path is being updated outside the mutex guarding
not_empty.wait(), which can let a worker miss the notify and sleep indefinitely.
In the relevant shutdown logic around pending and not_empty, acquire the same
mutex before setting shutdown and hold it while updating the flag, then notify
after the state change so the waiting worker observes the shutdown reliably.
Comment on lines
465
to
+476
| self.persist_hook_after_install(); | ||
| // After draining we're done — the data is compacted. | ||
| // Compact additional fields inline (no in-flight for those). | ||
| for (_, fs) in &mut self.field_segments { | ||
| let dim = fs.collection.dimension; | ||
| let mut unused_id = 0u64; | ||
| Self::compact_segments( | ||
| &mut fs.segments, | ||
| &mut fs.scratch, | ||
| &fs.collection, | ||
| dim, | ||
| 0, | ||
| None, | ||
| &mut unused_id, | ||
| ); | ||
| } | ||
| return; | ||
| // Do NOT return here: inserts that landed WHILE the | ||
| // background build was in flight are still in the mutable | ||
| // tail (`clone_suffix(frozen_len)` above). An early return | ||
| // breaks force_compact's full-drain contract (FT.COMPACT: | ||
| // frozen == mutable on reply) and leaves those docs with no | ||
| // durable segment until some future compact fires — a kill | ||
| // -9 in that window relied on them being "verified | ||
| // unchanged" from a keymap that covers them while no | ||
| // segment does (the B3 phantom-entry hole). Fall through to | ||
| // the inline compact below, which is a no-op when the tail | ||
| // is empty and otherwise drains + persists it. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Delay the manifest/keymap snapshot until after the tail is drained.
Line 465 can submit a snapshot after installing the in-flight segment but before the fall-through compacts the mutable tail. That snapshot can persist a keymap containing tail keys while segment_ids only covers the in-flight segment; a crash before the later hook commits reopens the phantom-keymap window. Defer this hook and rely on the final hook after compact_segments.
Suggested change
- self.persist_hook_after_install();
+ // Defer the manifest/keymap snapshot until after the inline
+ // compact below drains the mutable tail. The worker already
+ // persisted the segment file; publishing the manifest early
+ // can expose keymap entries for tail docs that are not yet
+ // covered by any durable segment.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| self.persist_hook_after_install(); | |
| // After draining we're done — the data is compacted. | |
| // Compact additional fields inline (no in-flight for those). | |
| for (_, fs) in &mut self.field_segments { | |
| let dim = fs.collection.dimension; | |
| let mut unused_id = 0u64; | |
| Self::compact_segments( | |
| &mut fs.segments, | |
| &mut fs.scratch, | |
| &fs.collection, | |
| dim, | |
| 0, | |
| None, | |
| &mut unused_id, | |
| ); | |
| } | |
| return; | |
| // Do NOT return here: inserts that landed WHILE the | |
| // background build was in flight are still in the mutable | |
| // tail (`clone_suffix(frozen_len)` above). An early return | |
| // breaks force_compact's full-drain contract (FT.COMPACT: | |
| // frozen == mutable on reply) and leaves those docs with no | |
| // durable segment until some future compact fires — a kill | |
| // -9 in that window relied on them being "verified | |
| // unchanged" from a keymap that covers them while no | |
| // segment does (the B3 phantom-entry hole). Fall through to | |
| // the inline compact below, which is a no-op when the tail | |
| // is empty and otherwise drains + persists it. | |
| // Defer the manifest/keymap snapshot until after the inline | |
| // compact below drains the mutable tail. The worker already | |
| // persisted the segment file; publishing the manifest early | |
| // can expose keymap entries for tail docs that are not yet | |
| // covered by any durable segment. | |
| // Do NOT return here: inserts that landed WHILE the | |
| // background build was in flight are still in the mutable | |
| // tail (`clone_suffix(frozen_len)` above). An early return | |
| // breaks force_compact's full-drain contract (FT.COMPACT: | |
| // frozen == mutable on reply) and leaves those docs with no | |
| // durable segment until some future compact fires — a kill | |
| // -9 in that window relied on them being "verified | |
| // unchanged" from a keymap that covers them while no | |
| // segment does (the B3 phantom-entry hole). Fall through to | |
| // the inline compact below, which is a no-op when the tail | |
| // is empty and otherwise drains + persists it. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vector/store.rs` around lines 465 - 476, The manifest/keymap snapshot is
being taken too early in the force-compact flow, after installing the in-flight
segment but before the mutable tail is drained. In the `Store::force_compact`
path, move `persist_hook_after_install` so it runs only after `compact_segments`
has fully drained and persisted the tail, and keep the final hook as the single
snapshot point to avoid a keymap/segment_ids mismatch.
…for quickwins_red_api Main's post-merge CI runs for PR #217/#218/#219 are all red on two issues this branch's PR (#228) also inherits; both are test-side. 1. test_case_e_cross_db_copy_oom: 0/300 OOM on BOTH CI platforms while passing locally (76/300; earlier deflake N/10 -> N/30 saw 28/300). The single-shot statistical assert races GAP-1's elastic budget: one round of 300x4KB COPYs (~300KB/shard) fits inside the slack the elastic budget plus its 100ms-stale usage snapshots can grant, so the OOM share is runner-speed-dependent all the way down to zero. Rewritten as bounded escalation: up to 8 rounds of COPYs into fresh destination keys, stopping early once OOMs appear. The destination db's per-shard usage (~2.4MB after all rounds) provably exceeds the ABSOLUTE per-shard budget ceiling (compute_elastic_budget returns at most base + surplus <= maxmemory = 2MB, and the gate compares db.estimated_memory() against it under noeviction), so gate-checked COPYs MUST OOM on any runner speed. The RED floor stays exact and was re-verified: with the Gap A gate block neutered, 0 OOM through all 8 rounds (2400 COPYs) — the assert still discriminates. 2. tests/quickwins_red_api.rs broke the Windows CI leg at COMPILE time: qw1_accepted_socket_has_nodelay calls moon::server::socket_opts::apply_client_socket_opts, which is #[cfg(unix)] (takes AsFd). The test and its TcpListener/TcpStream imports are now unix-gated to match the helper. Validation: oom_bypass_closure 8/8 green on monoio AND tokio+jemalloc (MOON_NO_URING=1) locally; case E red/green toggled via the gate block. author: Tin Dang
test_recl_segment_stall_active_atomic_exists and test_is_segment_stall_active_helper both store/load the PROCESS-GLOBAL RECL_SEGMENT_STALL_ACTIVE atomic while the test harness runs them on parallel threads within the same binary — the atomic-exists test's `store(1); load()` observed the helper test's final `store(0)` interleaving (CI macOS: `left: 0, right: 1` at the read-back assert; PR #228 round-2 Check (macOS) leg). Pre-existing since MA1 (wave 1); purely a test-parallelism race, not a stall-gate defect. All mutation of the global now lives in ONE merged test (test_recl_segment_stall_active_atomic_and_helper) so there is nothing left to race with; the default-0 assert is also only valid under that single-writer arrangement. Ran 5x locally green. author: Tin Dang
…entry author: Tin Dang
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
2 participants
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
RSS/CPU wave 4 (task #23): eliminate the vector keymap CoW write-amplification + bound the durability snapshot queue — plus two pre-existing kill-9 data-loss bugs found (and fixed) while validating this branch against the
crash_recovery_vector_durabilitysuite.1.
BucketedKeyMap— bucket-scoped CoW for the vector keymaps (defect 1)VectorIndex's threekey_hash → Vmaps were each a singleArc<HashMap>. While an FT.SEARCH snapshot is alive, every write'sArc::make_mutfull-cloned the whole map synchronously on the shard event loop (~47MB @1m vectors). Replaced withBucketedKeyMap<V>(src/vector/keymap.rs): 256 fixed buckets, each independentlyArc<HashMap>, bucket =key_hash >> 56. Snapshot capture stays O(1)-ish (256 refcount bumps); a concurrent writer now clones at most 1/256th of the map. Property test asserts ≥255 buckets remainArc::ptr_eqafter one insert under a live snapshot. On-disk keymap format unchanged.2. Coalescing
SnapshotPool(defect 2)The durability snapshot queue was an unbounded
flumechannel: a burst of compact/merge installs queued jobs that pinned every submitter'sArcs until drained. Now aMutex<HashMap<idx_dir, SnapshotJob>>coalescing slot: a newer job for the same index replaces a pending one in place (itsArcs drop immediately); in-flight jobs are unaffected; submit never blocks. Single worker + per-index watermark keep ordering semantics identical.3. Pre-existing fix: FT.COMPACT left mutable residue behind (
mainbug)force_compactdraining an in-flight background build installed it and returned without compacting docs inserted during that build — they stayed mutable-only (no durable segment) indefinitely, breaking the full-drain contract. Load-dependent; this is what made S1/S2 flake by machine load (initially masquerading as a regression in this branch — bisect + artifact decoding cleared it). Now falls through to the inline drain loop. Unit regression red/green verified.4. Pre-existing fix: B3 recovery silently dropped docs for phantom keymap entries (
mainbug)The durable keymap covers every indexed key (mutable + immutable) at snapshot time; the manifest lists only installed segments. After a kill -9 inside the async-snapshot window the keymap is a strict superset of the segments, and the B3 dedup rescan "verified" the uncovered keys as unchanged (checksum matches AOF — exactly the crash-window signature) → their docs vanished from search (
num_docs1950/2000, wrong KNN top-1; decoded from a preserved failing run's on-disk artifacts). Recovery now gates keymap loading on live segment membership (ImmutableSegment::live_key_hashes); uncovered keys re-index from the AOF. Unit regression (staged phantom entry, red/green verified) + new integration scenario S6 + a full-durability pre-kill gate that de-flakes S1/S2.Validation
crash_recovery_vector_durability(6 scenarios): 6/6 × 4 runs under deliberate concurrent load + 2 quiet runsCloses #23.
5. CI fixes: main was red before this branch
Main's post-merge CI runs for #217/#218/#219 all failed on (a)
test_case_e_cross_db_copy_oom— 0/300 OOM on both platforms: the single-shot statistical assert races GAP-1's elastic budget + 100ms-stale usage snapshots; rewritten as bounded escalation past the ABSOLUTE budget ceiling (timing-independent; red floor re-verified exact by neutering the gate → 0 OOM through 2400 COPYs), and (b) the Windows leg failing to COMPILEtests/quickwins_red_api.rs(apply_client_socket_optsis#[cfg(unix)]; test now unix-gated).Summary by CodeRabbit
FT.COMPACT/force_compactto fully persist documents added during an in-progress background compaction.