Skip to content

Durable path-level plane invalidation (invalidatePlane, format v7 latch) and the harper#2430 native delta - #1

Merged
kriszyp merged 5 commits into
mainfrom
feat/invalidate-plane-and-harper-2430-sync
Sep 2, 2026
Merged

Durable path-level plane invalidation (invalidatePlane, format v7 latch) and the harper#2430 native delta#1
kriszyp merged 5 commits into
mainfrom
feat/invalidate-plane-and-harper-2430-sync

Conversation

@kriszyp

@kriszyp kriszyp commented Sep 2, 2026

Copy link
Copy Markdown
Member

Brings the crate up to the reviewed native delta of HarperFast/harper#2430 (first-insert claim/join, search-side entry repair with a rotating bounded probe, predicate-drain fixes, 4-aligned neighbor arrays) and adds a package-owned, durable way to invalidate a plane file the host cannot delete, so Harper can replace its vendored native/hnsw-plane with a released @harperfast/hnsw. Recommended next version: 0.2.0 (format bump, new exports); the package pins are already moved but nothing is published by this PR.

Port. src/ and tests/ were byte-identical to the subtree at Harper 7661fd1b6 before this PR; the subtree diff to PR head a0f735d54 applied cleanly and, after it, they differed from the head subtree only in three doc-path pointers and the two corrected scrub claims — opened_clean is advisory: open() performs no scrub, and the orphaned "force persisted-odd seqlocks back to even" doc that had attached itself to slot_ptr is gone. The two design-doc hunks (slot pad, atomic slot payloads) landed in DESIGN.md so graph.rs's §10 citations resolve. The pre-push review then found three defects in the ported delta itself, fixed here rather than carried: a stale re-election could CAS over an equal-level entry a fresh first-insert claim had just installed, orphaning a node whose insert reported success (regression test a_stale_reelection_never_displaces_an_equal_level_entry_installed_meanwhile); insert's bounded entry-resolution loop left its published edgeless node behind on Err(Wedged); and a fully dead graph paid the 1024-read repair probe on every search forever, now stopped after one empty rotation, single-flight per handle, and re-armed by any node write through any handle, keyed on a header write epoch bumped after each slot publishes. These three diverge from Harper's vendored copy on purpose; the vendored copy is deleted when Harper adopts the package.

Invalidation. Harper's helper did this in JS with three defects: the .stale sidecar was never fsynced, a temporary handle was left to the garbage collector (on Windows that mapping is itself why the unlink failed), and both steps swallowed their errors. The planning review rejected the first design (a mutable watermark zero as tombstone) because a flushAsync already in flight re-stamps it; the adopted design is a one-way latch. invalidate_with always attempts both markers, in band first (an ordering test observes the latch on the file before the sidecar step runs), drops any temporary handle before the sidecar step, and errors only when neither is durable: in band, invalidate() sets the sticky H_INVALIDATED byte under which watermark() reads 0 on every handle whatever a racing flush stamps; the sidecar is created with create-new semantics and an existing one is re-synced through a no-follow, non-blocking open validated on the handle, so a planted symlink or FIFO is refused rather than followed; both are fsync'd with the directory entry on POSIX. The package enforces the markers: open() refuses a latched or sidecar'd file, re-checking the sidecar after mapping and treating any stat failure but absence as present and create() refuses a sidecar present before the create and latches the finished file when one landed during it, so losing that sidecar cannot make the failed create adoptable. Exposed as invalidatePlane(path) / invalidatePlaneAsync(path) and, through a handle the host already holds, plane.invalidateFile() — a method rather than an attached argument so a handle/path mismatch cannot be expressed. Look hardest at the latch's read-side rule: it is what makes in-band success sound against concurrent writers, and it is deliberately read-side only (no write is refused), which a_later_flush_or_stamp_cannot_revive_an_invalidated_plane pins.

For the human reviewer

  1. Format v7, not v6. The latch byte at header offset 57 is zero in every existing v6 file, so v6 could have been kept; bumping to v7 makes the semantics unambiguous at the cost of a one-time reindex of the v6 planes that exist only under unmerged harper#2430 on developer boxes. The review's ledger also flags that v5 → v7 lands in one release with no read-only tolerance of older files; there are no planes in the field (0.1.0 has no consumer yet), so a migration path was not built. Reversible before publishing by setting VERSION back. A "no" costs the ambiguity that an old crate ignores the latch on a file a new crate invalidated.
  2. Plane.open now refuses an invalidated plane (header latch or .stale sidecar — any directory entry at that name counts, fail-closed, so an unrelated file there bricks open and create for that path until removed) instead of returning it with watermark 0. This is the package-level enforcement the planning review asked for, and it covers standalone consumers that never read the watermark. Harper impact when it adopts the package: its attach path already removes the sidecar before opening, and an unopenable latched file goes down its existing "unopenable plane → delete and rebuild" branch (after PLANE_STALE_CREATE_MS, 60 s); its unit test that reopens an invalidated plane to check the watermark must assert the refusal instead. Alternative: keep open permissive and expose only invalidated(), leaving enforcement to hosts.
  3. Synchronous fsyncs on the calling thread for invalidatePlane and invalidateFile: three small fsyncs (header page, marker, directory) on a cold path that Harper calls from synchronous disable/reset code. invalidatePlaneAsync exists for callers that can await. No measured bound on slow storage; if the sync form is a concern for mass drops, the Harper call sites are where to switch.
  4. Windows directory fsync is skipped by cfg; the marker's own FlushFileBuffers is the durability point (documented to flush the metadata of the file's creation). No automated test proves power-loss durability on any platform; the claim is scoped to NTFS/ReFS-class local filesystems, not network shares.
  5. The probe futility latch costs every node write one relaxed fetch_add on a shared header word (the write epoch). The first version re-armed only on the writing handle's own writes, which the second review round showed leaves another process's revival invisible; the epoch makes the latch exact across processes at that per-write cost. Alternative: never stop probing, the 1024-read-per-search cost on a fully dead graph the first round measured.
  6. Inherited from harper#2430's review, unchanged here: search-side entry repair on the read path, REPAIR_PROBE_LIMIT = 1024 with per-handle rotation, re-election before the tombstone in delete_node, volatile reads rather than atomic slot fields (DESIGN.md §10), and invalidate() flushing the header page only. They were adjudicated in that PR and are ported verbatim; re-litigating them belongs there.
  7. getWatermark() reads 0 on an invalidated plane rather than the stored word, so the enforcement holds for hosts that only read the watermark; invalidated() is the diagnostic that says why. Alternative: report the stored word and leave the rule to hosts.
  8. create() refuses a leftover .stale rather than deleting it: a host that crashed mid-rebuild needs the operator (or its own attach path, as Harper's does) to remove the marker. Availability trade, one branch to flip.
  9. Left as best-effort: a sidecar landing after create()'s post-check hands the caller a live handle the next open will refuse (closing it needs an exclusive lock over create; Harper's wx create already serializes its own creates). Declined from the review: the Harper end-to-end proof (a mapping held by another worker, disable, mutate, re-enable, rebuild), which belongs to harper#2430's own suite. Plane.invalidate() (in-band only) stays exported because Harper's current helper calls it; its doc now points at invalidateFile(). Ported decisions the review re-raised — InsertError::Wedged for an unresolvable entry, NO_ID entries now repaired on read — are harper#2430's.

Verification

Route: the repo's own gates plus the package load smoke, executed locally on Linux x86_64 (the Windows leg of the new CI matrix is the first run of the cfg'd directory-fsync and no-follow reopen paths).

  • cargo test --release: 17 unit + 2 concurrent + 24 reopen tests pass, including the ported first-insert race, repair-probe, and predicate-drain tests, and the new coverage: both markers, in-band-only, sidecar-only, double failure deletes nothing, temporary handle released before return (registry tag reads dead, file deletable), attached handle claims no slot, idempotence, symlink at the sidecar path (first and repeat), open/create refusal, the equal-level re-election guard, the probe futility stop and cross-handle re-arm, the in-band-before-sidecar order, and the public-API path in tests/reopen.rs.
  • cargo clippy --release --features napi: no warnings in the new or changed code (one pre-existing identity_op in the ported create).
  • node build.mjs && node smoke.mjs (the package load smoke through index.js): passes, exercising invalidateFile, the latch surviving a later flush(900), the open refusal, invalidatePlane, invalidatePlaneAsync, and the double-failure throw.
  • Equivalence check: diff -r of src/ and tests/ against the harper PR-head subtree after the port showed only the intended doc-pointer and scrub-doc differences (recorded in the dispatch log); the three review fixes above are the only later divergences.

Refs HarperFast/harper#2430

Complexity: complicated

Review-Coverage: authored=claude; ran=codex; blocked=gemini(auth); declined=cursor-grok,cursor-composer,domain; rounds=3 @ 6d76ba4

Human-Review-Need: 3 @ 6d76ba4

kriszyp and others added 5 commits September 2, 2026 11:06
…..a0f735d54)

Applies the native/hnsw-plane subtree diff between the commit this crate already
matched (7661fd1b6) and the PR head (a0f735d54) verbatim: first-insert claim/join,
repair probe with per-plane rotation, predicate-drain fixes, format v6 (4-aligned
neighbor and upper id arrays, volatile field reads), Plane.invalidate(), and the
accompanying tests. The design doc receives the same two hunks (slot pad, atomic
slot payloads) so its section numbers still match the citations in graph.rs.

Also corrects two stale open-time scrub claims in format.rs: open() performs no
scrub, so opened_clean is advisory and the orphaned "force persisted-odd seqlocks
back to even" doc that had attached itself to slot_ptr is gone.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017edcNKY5AgYYJmyNfxmUaK
…'d .stale sidecar

A plane the host cannot delete (Windows sharing violation while another process maps it)
must never be adopted later at its nonzero watermark. Harper's helper did this in JS with
three defects: the .stale sidecar was never fsynced, a temporary handle was released by the
garbage collector (on Windows that mapping is itself why the unlink failed), and both steps
swallowed their errors.

invalidatePlane(path) / plane.invalidateFile() now own it: the in-band mark is a sticky
header byte (format v7) under which watermark() reads 0 on every handle — a flushAsync
already in flight can still stamp the word but cannot revive the plane — plus the sidecar,
created with create-new semantics (a planted symlink is never followed), fsync'd with its
directory entry on POSIX. Both markers are attempted every call; the temporary handle is
dropped before the sidecar step; the call throws only when neither marker is durable,
leaving the file exactly as found. open() refuses a file carrying either marker and
create() refuses a path with a leftover sidecar, so the markers are enforced by the
package rather than by each host's attach path.

Also: CI matrix gains windows-latest (the cfg'd directory-fsync path), package.json and its
platform pins move to 0.2.0, and Cargo.lock records the libc dependency it was missing.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017edcNKY5AgYYJmyNfxmUaK
…sidecar TOCTOU + no-follow reopen

- cas_entry_if_not_better aborts on an equal-level entry installed meanwhile (>=, not >):
  a stale re-election could otherwise CAS over a fresh claim_entry_if_empty winner with no
  in-edges, orphaning a node whose insert already reported success. Regression test.
- insert's bounded entry-resolution loop deletes the edgeless node it published when it
  falls out with Err(Wedged), instead of leaving a live-reading, edgeless slot for the
  repair probe or a re-election to root the graph at.
- probe_for_entry stops after `stride` consecutive empty rotations at one high-water and
  re-arms on any node write through the handle: a fully dead graph no longer pays 1024
  node reads per search forever. Unit test covers the stop and the re-arm.
- open() re-checks the sidecar after mapping and create() re-checks it before returning,
  closing the pre-map TOCTOU; an existing sidecar is re-synced through a no-follow,
  non-blocking open validated on the handle, so a marker swapped for a symlink or FIFO is
  refused rather than followed.
- smoke.mjs no longer unlinks a plane two handles still map (a Windows sharing violation).
- Contract text: a double failure deletes nothing; the in-band mark may still have landed
  in the shared mapping when its msync failed, which is the safe direction.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017edcNKY5AgYYJmyNfxmUaK
…hat raced an invalidation is latched

The probe futility latch was re-armed only by writes through the same handle, so another
process reviving a fully dead graph without an entry-point update stayed invisible to this
handle until its high-water changed — a regression against always probing. The header now
carries a write epoch (v7 field, offset 96) bumped by every node write through any handle;
the latch is keyed on (high-water, epoch) and any write anywhere re-arms the probe. The unit
test revives through a second handle on the same file.

create() finding a sidecar that landed during the create now latches the finished header
before returning Err, so a lost or removed sidecar cannot turn that failed create into an
adoptable empty plane.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017edcNKY5AgYYJmyNfxmUaK
… single-flight probe, ordering test

- The write epoch is bumped after the seqlock release that publishes the slot (Release on
  the bump, Acquire on the probe's load): a probe that consumed the bump while the slot was
  still invalid could otherwise latch a plane that holds a live node.
- stale_sidecar_present treats any stat failure other than NotFound as "present": a
  durability marker must fail closed, not vanish on a transient EIO/EACCES.
- One repair probe at a time per handle; concurrent searches return empty for that call
  rather than each paying the full walk before one publishes.
- invalidate_at gained a sidecar-writer seam so a test proves the in-band mark is on the
  file before the sidecar step runs, through a temporary handle and an attached one.
- create's post-check comment states its best-effort scope.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017edcNKY5AgYYJmyNfxmUaK
@kriszyp
kriszyp marked this pull request as ready for review September 2, 2026 22:56
@kriszyp
kriszyp merged commit 95c8076 into main Sep 2, 2026
3 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