feat(persistence): FTS term-dict + FST sidecar durability across kill-9 (kernel M4, task #50) - #313
Merged
Merged
Conversation
…-9 (kernel M4, task #50) Design note: FTS was the last plane not kill-9-lossless. Every restart rebuilt every text index from scratch by rescanning the keyspace and reassigning term ids by `DashTable` hash-iteration first-encounter order -- not reproducible across restarts. That's why a pre-existing `.fst` sidecar write path had its load path deliberately left unwired in production (`TextStore::load_fst_sidecars` had zero callers): loading it once (kernel-m2-brief-2026-07-12.md) corrupted FUZZY/PREFIX search silently -- the sidecar's baked-in term ids collided with whatever ids the fresh rescan happened to assign the same terms this time. Fix shape: persist the term dictionary itself alongside the FST, in one atomic sidecar per (shard, index) -- `{shard_dir}/{index_name}.tfst`, magic `TFS2`, version-stamped, written via the shared `atomic_write_durable` primitive (temp + fsync + rename + dir-fsync, same as every other text/vector sidecar). Per TEXT field it stores `next_id`, `fst_high_water_mark`, every `(term, id)` pair, and the optional FST bytes -- all four written together so they can never independently go stale relative to each other. `TermDictionary::from_pairs` reconstructs a dictionary whose ids are taken VERBATIM from the sidecar (never reassigned) and whose `next_id` continues the persisted high-water mark. Loading happens in `TextStore::load_term_fst_sidecars`, wired into shard boot (`src/shard/event_loop.rs`) immediately AFTER text index schemas are restored from `text-indexes.meta` but BEFORE the keyspace auto-reindex rescan runs. Seeding the term dict first is the whole trick: the rescan's `get_or_insert` calls resolve already-known terms to their PERSISTED ids (idempotent, not reassigned) and only hand out brand-new, non-colliding ids to terms that are genuinely new since the sidecar was written. That makes the loaded FST safe -- its baked-in ids and the live dictionary's ids are the same id-space by construction, not by hope. Staleness handling is fail-closed and per-index: missing sidecar (no FT.COMPACT ever ran, or index created after last compact) is a silent no-op -- today's full-rescan behavior, unchanged. Truncated/corrupt/ version-mismatched bytes, or a field-count mismatch (schema changed since the sidecar was written), aborts loading for that index entirely -- never partially applies a sidecar (e.g. seeding the dict but skipping a bad FST, or vice versa), which would silently reintroduce the exact id-space mismatch this closes. `FT.COMPACT` now persists via the combined saver (`save_term_fst_sidecar_for_index`) instead of the old FST-only one; the old FST-only save/load pair is kept `#[cfg(test)]` only, for its own pre-existing roundtrip tests. Multi-shard: the sidecar is written per shard_dir (each shard owns its own persist_dir), so it is naturally per-shard-keyed -- no cross-shard aliasing risk. Observability: new additive `FT.INFO` counters `sidecar_recovered_indexes` / `text_indexes_total` (same additive-merge convention as the vector engine's `graph_segments` / `segments_with_exact_rerank`), set via a new `TextIndex.recovered_from_sidecar` flag. Testing: - `TermDictionary::from_pairs`: roundtrip-equivalence to live `get_or_insert`, plus fail-closed cases (id >= next_id, duplicate id, duplicate term, hwm > next_id). - `index_persist`: serialize/deserialize roundtrip, disk save/load roundtrip, missing-sidecar, bad magic/version, too-short, truncated mid-payload, non-UTF8 term bytes -- all fail closed (Err), never panic. - `TextStore` equivalence gate (the core proof): build an index, compact (builds + persists the sidecar), then simulate a restart (fresh empty TextStore + schema-only index + `load_term_fst_sidecars` + re-index the same docs, exactly the boot sequence in event_loop.rs) and assert FUZZY and PREFIX `expand_terms` results -- and the BM25 search results built on top of them -- are byte-for-byte identical to the pre-crash index. Plus: missing-sidecar no-op, field-count-mismatch fail-closed. - New fuzz target `term_fst_sidecar` (fuzz/fuzz_targets/) covers the sidecar decoder against arbitrary bytes. - New default-GREEN crash-matrix cells `cross_plane_prod_{s1,s4}_text_fts_sidecar_isolated` (tests/crash_matrix_cross_plane): FT.CREATE, index docs, FT.COMPACT, index MORE docs post-compact (exercises id-continuation past the sidecar's high-water mark), kill -9, restart, assert a FUZZY query finds every doc and `FT.INFO sidecar_recovered_indexes >= 1`. Unlike the RED root-cause cells tracked elsewhere in this matrix, this one is expected to hold -- a regression here means the sidecar write, the boot wiring, or the id-continuation invariant broke. Gates: text-module unit tests green (index_persist, term_dict, store -- including the new equivalence/fail-closed tests), `cargo fmt --check` clean, `cargo clippy --lib --tests -- -D warnings` clean (default features), `cargo clippy --no-default-features --features runtime-tokio,jemalloc --lib --tests -- -D warnings` clean for every file touched by this change (grep-confirmed; that matrix currently has ~33 pre-existing, unrelated clippy errors in files this change never touches -- persistence_tick.rs / timers.rs / db_quota.rs / eviction.rs / vector/store.rs -- left untouched per "surgical changes only"), fuzz target compiles (`cargo check --bin term_fst_sidecar` inside fuzz/), and the crash-matrix integration crate compiles clean (`cargo test --test crash_matrix_cross_plane --no-run`). The binary-based GREEN run of the new crash-matrix cells (which needs a built release binary, and every cell in this suite is `#[ignore]`/"run explicitly" by convention) was still building under heavy concurrent-build contention on this machine at commit time -- run `cargo test --release --test crash_matrix_cross_plane -- --ignored text_fts_sidecar` to confirm before shipping. Confidence: completeness 0.9, clarity 0.92, practicality 0.9, optimization 0.85 (id-continuation avoids any FST rebuild cost on a sidecar-recovered boot), edge-cases 0.88 (schema-change and corruption paths covered; not covered: FST bytes present but term-dict absent from a half-written pre-K3 sidecar format -- not reachable since this is a new format, TFS2, distinct from the legacy TFST), self-evaluation 0.9. author: Tin Dang
…e (task #50 follow-up) The `text_fts_sidecar_isolated` scenario's pre-compact doc:1 body was "deep learning machinery". "machinery" stems to a different token than "machine" (Porter/Snowball -y handling), so it is NOT within edit-distance-1 of the FUZZY probe "machn" used by the test. That made the GREEN tripwire cell fail with a message that looked exactly like a real sidecar-recovery regression ("FUZZY 'machn' must find {txtiso}:txt:1") on the first binary run against the release build, but the failure was in the test fixture's corpus, not in `load_term_fst_sidecars` or the sidecar format -- the other 4 of 5 docs (which do contain "machine") passed correctly both before and after the fix. Fixed by changing doc:1's body to "deep learning machine" so every doc in the fixture actually contains a term that stems to "machin". Verified 3x consecutive GREEN at both shards=1 and shards=4 against the release binary (`cargo test --release --test crash_matrix_cross_plane -- --ignored text_fts_sidecar`). 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: 9 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 (12)
✨ 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 |
…848dcf8b4cc # Conflicts: # CHANGELOG.md
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
Kernel M4, task #50: FTS was the last plane without kill-9 durability — the inverted index was rebuilt by full keyspace rescan on every boot, and the FST sidecar's load path was deliberately unwired (loaded term-ids were stale id-space garbage vs the rebuilt dict → silent FUZZY/PREFIX corruption).
Design
Combined sidecar
{shard_dir}/{index_name}.tfst(magicTFS2, versioned, written viaatomic_write_durable) persists per TEXT field:next_id, FST high-water mark, every(term, id)pair, and the FST bytes — together, so none can go stale relative to the others. On boot,load_term_fst_sidecarsruns after index-schema restore but before the keyspace rescan: the rescan'sget_or_insertresolves known terms to persisted ids and hands fresh non-colliding ids only to new terms — the loaded FST's id-space matches the live dictionary by construction, closing the corruption that kept the load path unwired. Fail-closed per index (missing/corrupt/version-mismatch/schema-change → full rescan, never partial).FT.COMPACTwrites the sidecar; new additiveFT.INFOcounterssidecar_recovered_indexes/text_indexes_total.Verification
expand_terms+ BM25 results identical between sidecar-recovered restart and from-scratch rebuild.cross_plane_prod_{s1,s4}_text_fts_sidecar_isolated(create → docs → COMPACT → more docs → kill-9 → restart → FUZZY finds all + recovered-counter ≥1) — 3× green on macOS release AND Linux VM (46-cell suite green; second commit fixed a fixture false-negative the binary run caught — stemming edge, test-data bug not product bug).term_fst_sidecar; 24 index_persist unit tests; fmt + clippy both matrices clean for all touched files.With this, every plane (KV/vector/graph/text/WS/MQ/temporal) has kill-9 durability in prod config — the v0.8 headline invariant. Remaining RED cells are legacy-mode (
--disk-offload disable) graph only.Refs: task #50, K3 (#296), K4 (#297).