Skip to content

Protect Harper-backed Tantivy readers during reclamation - #27

Merged
kylebernhardy merged 9 commits into
mainfrom
codex/issue-11-reader-pins
Sep 10, 2026
Merged

Protect Harper-backed Tantivy readers during reclamation#27
kylebernhardy merged 9 commits into
mainfrom
codex/issue-11-reader-pins

Conversation

@kylebernhardy

@kylebernhardy kylebernhardy commented Sep 10, 2026

Copy link
Copy Markdown
Member

Adds process-local object/revision reader pins to the Harper-backed Tantivy Directory so a retained file handle remains visible to the upcoming bounded reclamation consumer. Open-read registration and deletion publication are ordered through a fixed hash-sharded fence without adding storage reads, and the benchmark now measures retained, churned, distinct-file, and shared-file reader behavior.

Progresses #11.

For the human reviewer

  1. The design uses a blocking 256-way path-hashed registration fence instead of an epoch/quiescence protocol. The fence gives a direct proof that a reader either registers before deletion commits or observes the missing binding; its accepted cost is that a stalled host read or write can delay a hash-colliding path. This is reversible before cleanup is enabled, and the plan requires measuring the shard count against Harper host-read p99; rejecting it means implementing and proving a quiescence protocol before the reclamation consumer.
  2. Pins are process-local and shared by every Directory with the same storage identity and namespace rather than persisted in RocksDB. This exactly follows Tantivy file-handle lifetime without a read-time storage mutation, but cleanup remains gated on Harper proving one active generation owner and shared native state across its workers. Rejecting it requires a cross-process reader-lifetime protocol owned by Harper.
  3. This PR intentionally lands the pin producer before the physical cleanup consumer. That keeps the new concurrency and lifetime rules inert and independently testable; no payload bytes are deleted by this change. Combining the consumer now would remove the isolation boundary and substantially increase the data-loss blast radius of review.

Verification

  • npm run check passed: formatting, TypeScript, Clippy, 92 Rust tests, 35 Node tests, and the packed-package test.
  • cargo bench --locked --features phase0 --bench kv_directory -- --smoke --revision reader-pin-final passed and emitted the new retained, churned, distinct-file -rw-, and shared-file -rw- reader cases.
  • The real Tantivy lifecycle and crash/reopen tests exercise the Directory route, and a deterministic registration/deletion race test proves the ordering invariant. End-to-end prevention of physical reclamation is intentionally not observable until the next consumer slice.

Comment generated by kAIle (GPT-5)

Complexity: complicated

Review-Coverage: authored=codex; ran=claude; declined=gemini,cursor-grok,cursor-composer,domain; rounds=7 @ 008e2d2

Human-Review-Need: 3 @ 008e2d2

@kylebernhardy kylebernhardy added this to the v5.3 milestone Sep 10, 2026
@kylebernhardy
kylebernhardy marked this pull request as ready for review September 10, 2026 05:37
@kylebernhardy
kylebernhardy merged commit 15045c9 into main Sep 10, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant