Skip to content

engine: MN-REQ-05.5 post-merge fixes (lost write on evicting victim, fsync, claim leak, counter, serve_status) - #221

Merged
chouswei merged 4 commits into
masterfrom
engine/mn-req-05.5-evict-fixes
Oct 21, 2026
Merged

chouswei merged 4 commits into
masterfrom
engine/mn-req-05.5-evict-fixes

Conversation

@chouswei

@chouswei chouswei commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner

Follow-up to #220, covering Elon's post-merge review rounds. Do not merge without owner review. No release. Head: 60314cd.

Round 1 (818e9c3): post-merge blockers

  • B1, lost write:
    • Bug: at cap 1, a mutate that took the victim's handle before the eviction dropped it exited 0, and its write was lost.
    • Fix: SessionStore.lock() re-checks, after taking the lock, that the registry entry is the same one and isn't evicting. If not, it refuses with the existing session-miss code (session_expired|snap_available, or session_not_found) and writes nothing.
    • CLI: lock users, including import_slice, go through _SessionLock, so the refusal shows as @ERR, not internal.
    • Test: a deterministic test pins the refusal path.
  • (a) Dir fsync: a directory fsync failure after the rename now raises for real I/O errors, so the victim stays live and the save is snapshot_unsaveable. EINVAL, ENOTSUP and EOPNOTSUPP are tolerated for filesystems that can't fsync a directory.
  • (b) Claim leak: a claimed slot is released on any error between claim and register.
  • (c) Counter: the unsaveable mark is cleared on a successful save, so sessions_unsaveable_held no longer overcounts.
  • (d) serve_status: MCP serve_status reports "config_error": "config_conflict", with no paths.
  • (e) import_absorb: no separate surface exists; import_slice is a write that already touches the session. Elon accepted this.

Round 2 (eecdd17): expire race

  • Fix: every expiry drop goes through one helper. It tries the session lock without waiting and skips the session until the next sweep if it's busy, including when the calling thread holds it. Under the lock it re-checks that the entry is the same one and still expired, then saves and drops. An unsaveable session keeps its RAM.
  • Coverage: the purge sweep, get_session and the save path, plus CLI session save and session load.
  • Tests (tests/test_expire_race.py):
    • The holder's write lands.
    • Reads don't block on a held lock.
    • A TTL renewed between the scan and the lock is kept.
    • A session the calling thread holds is skipped.
    • The CLI load and save paths are covered.
    • An unsaveable session is kept.
  • Docs: crash safety holds only where the filesystem supports directory fsync.

Round 3 (60314cd): renewed-TTL fall-through

  • Fix: the expiry helper now returns a distinct outcome (DROPPED, RENEWED, BUSY, UNSAVEABLE or GONE). On RENEWED, get_session, the save path and CLI session_load fall through to the live path and return or save the live session. Busy and unsaveable behave as before.
  • Tests: Elon's probe is now a test covering all three paths, plus guards for busy and unsaveable.
  • Docs:
    • Added: "Expiry never blocks reads; a read of a session another thread holds waits for that holder, as today."
    • Removed the stale "Proposed, not shipped" opener from docs/operations/one-session-per-document.md.

Tests: 720 passed, 1 skipped (the AgensGraph live test, which needs MEMNET_AGENSGRAPH_URL), 0 failed, 0 xpassed. ruff is clean. Built by Claude haiku; Claude sonnet reviewed each round, and the round-3 review was CORRECT.

LATER:

  • Fold the _is_owned() call into gap 7.
  • session_evicted refusal.
  • Gaps 2–6 and 8.
  • A dir-fsync-unsupported signal or warning.
  • Same-session reads waiting on a writer.

The CLI session_load path still refuses a GONE session with overdue rather than trying the snapshot, as it did before this PR.

…m, dir fsync errors, claim release, unsaveable mark clear, serve_status config_conflict
…te, reads never block); narrow crash-safety doc to dir-fsync filesystems
@chouswei
chouswei merged commit f1dc1ad into master Oct 21, 2026
2 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