fix(store): keep records-by-key index in sync on prefix delete

[cbenhagen]

Description

Fixes a secondary-index leak in the redb (fs-store) replica store.

The store keeps two tables that must stay 1:1:

Table	Key	Role
records-1	(namespace, author, key)	authoritative records
records-by-key-1	(namespace, key, author)	secondary index, value ()

StoreInstance::remove_prefix_filtered (src/store/fs.rs) is the production prefix-delete
path. It is called from Ranger::put whenever an entry supersedes its prefix children, which
is exactly what Replica::delete_prefix triggers. It removed rows from records only and never
removed the matching rows from records_by_key. Every prefix-deleted record therefore leaked an
orphan index row that was never reclaimed.

The only code that removed from both tables was entry_remove, which is #[cfg(test)] and never
runs in production, so there was no live code path that kept the index in sync on delete.

Consequences:

• Unbounded store growth: orphan index rows in records-by-key-1 are never reclaimed and
  accumulate on every delete-heavy namespace.
• Degraded / incorrect key-range queries: RecordsByKeyRange walks records-by-key-1 and
  dereferences into records-1; orphans are dangling pointers that, at best, waste work and, at
  worst, surface as lookup misses.

Changes

• remove_prefix_filtered now drains the extract_from_if iterator into the set of removed keys
  (which performs the deletion from records) and then removes the corresponding row from
  records_by_key for each one, keeping the index 1:1 with records. The collect-first step
  releases the mutable borrow of tables.records before tables.records_by_key is mutated.
• Added two regression tests asserting records_by_key.len() == records.len() after a delete:
  • test_remove_prefix_filtered_cleans_by_key_index exercises remove_prefix_filtered
    directly and confirms records under unrelated prefixes are untouched.
  • test_delete_prefix_keeps_by_key_index_in_sync drives the same path end-to-end through
    Replica::delete_prefix.

Breaking Changes

None. This is a behavioural bug fix internal to the fs-store backend; no public API changes.

Notes & open questions

• This fix prevents new orphans. It does not retroactively prune orphan rows that already
  accumulated in existing stores. Because records-by-key-1 is a pure derived index, an existing
  store can be repaired offline (with the scheduler stopped) by clearing records-by-key-1 and
  re-inserting (namespace, key, author) -> () for every row in records-1. A migration could be
  added if we want this to self-heal on open; left out of this PR to keep it focused.
• entry_remove remains #[cfg(test)]. The single-entry remove is only used by tests, and the
  prefix path is the one that runs in production. Worth considering whether both deletes should
  route through one shared helper so the index-cleanup can't drift out of the live path again.
• Detection for the fleet: on any docs.redb, count(records-by-key-1) > count(records-1) means
  the leak is present and the delta is the orphan count.

Change checklist

• Self-review.
• Documentation updates following the style guide, if relevant.
• Tests if relevant.
• All breaking changes documented.

Summary by CodeRabbit

• Bug Fixes

  • Improved reliability of prefix-based record deletion to ensure proper data consistency across internal storage structures, preventing potential data inconsistencies.

• Tests

  • Added tests verifying data integrity and proper cleanup during prefix-based deletion operations.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bcf8b012-3238-473f-b4a4-1470b63d68ba

📥 Commits

Reviewing files that changed from the base of the PR and between fc89461 and 38e0b90.

📒 Files selected for processing (1)

• src/store/fs.rs

---

📝 Walkthrough

Walkthrough

remove_prefix_filtered in src/store/fs.rs is fixed to collect deleted records before removing their corresponding (namespace, key, author) rows from the records_by_key index, keeping both tables in sync. Two new tests verify this invariant via direct and indirect (Replica::delete_prefix) deletion paths.

Changes

records_by_key Index Sync Fix

Layer / File(s)	Summary
Fix records_by_key sync in remove_prefix_filtered src/store/fs.rs	remove_prefix_filtered now collects all records extracted by records.extract_from_if into a Vec before iterating to remove matching rows from records_by_key, avoiding a simultaneous borrow of records while mutating records_by_key. The returned count comes from the collected Vec length.
Index consistency tests src/store/fs.rs	test_remove_prefix_filtered_cleans_by_key_index and test_delete_prefix_keeps_by_key_index_in_sync insert prefixed and unrelated records, trigger deletion, assert records_by_key row count equals records row count, and confirm surviving keys are accessible.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 Hop hop, the index was astray,
Two tables drifted far away.
Collect first, then remove each row—
Now records_by_key stays in tow!
The tests confirm: all counts align,
Every key and index: fine. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main fix: keeping the records-by-key secondary index synchronized during prefix deletions.
Description check	✅ Passed	The description is comprehensive, addressing the bug context, consequences, implementation changes, testing, and notes on future improvements with all checklist items marked complete.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/records-by-key-index-leak

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}