refactor(persist): extract shared durable-write + quarantine primitives#289
Merged
Conversation
Introduce `crate::persist`, the single-source home for the on-disk persistence primitives the durable stores share: atomic durable write (tmp → fsync → rename → dir fsync), corruption quarantine + rotation (rename aside as `.corrupt-<ts>`, preserve bytes), and the fixint bincode readers with an allocation cap. These lived as `pub(super)`/private helpers in `user_history` (persistence.rs / recovery.rs), where a second durable store (user_dict, LXUW) could not reach them — the gap that left user_dict with an unsynced write and the stray-`.tmp` bug LXUD already fixed. Centralizing them is the prerequisite for hardening user_dict without a second, drift-prone copy (CLAUDE.md: 同種処理は単一正準形に収束). Parity-only: the primitives move verbatim (quarantine generalized to return an outcome enum + an injected `ts`, so it stays clock-free and `persist` depends on no store's domain types), and the three consumers — persistence.rs, recovery.rs, and wal.rs — are redirected to the shared module. `now_epoch` stays in `user_history` (already the crate-wide time util, 34 call sites) and is injected into `quarantine`, keeping `persist` a leaf both stores depend on. Behavior is unchanged; the full suite (incl. the recovery matrix in tests_recovery.rs) is the proof. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
5 tasks
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
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.
What
Extract the on-disk persistence primitives that
user_history(LXUD) alreadyuses into a new crate-internal
crate::persistmodule, and redirect the threeconsumers (
persistence.rs,recovery.rs,wal.rs) to it:sync_all(F_FULLFSYNC) → rename → best-effort dir fsync<name>.corrupt-<ts>(preserve bytes, never delete), cap retained copiesWhy
This is the prerequisite for hardening the user dictionary (LXUW) without a
second, drift-prone copy of privacy/durability-critical code. Those primitives
were
pub(super)/private insideuser_history, unreachable by the otherdurable store — the gap that left
user_dict::savewith an unsynced write andthe stray-
.tmpbug that LXUD had already fixed (an unfixed copy of the samebug = the exact drift
CLAUDE.md's 「同種処理は単一の正準形に収束させる」 targets).Centralizing makes the durability/quarantine invariants hold by construction
for both stores.
Scope — parity-only
dict/corpus/hot-path touch.
user_dictis deliberately not wired tocrate::persisthere — that adoption (plus fsync/quarantine/with_limit) isthe follow-up PR3b.
behavior-preserving:
quarantinereturns an outcome enum (Renamed/Removed/Failed) and takesan injected
ts, so it is a clock-free leaf with no dependency on anystore's domain types or clock.
recovery.rskeeps a thin adapter mapping theoutcome back to its old
bool+ report-path-push (exact isomorphism).rotate_quarantined(path, keep)takes the retention count as a parameter;QUARANTINE_KEEP = 3stays the caller's policy inrecovery.rs.now_epochstays inuser_history(already the crate-wide time util, 34 callsites incl. the FFI crate) and is injected into
quarantine.Net:
crate::persist(+226) centralizes what the three consumers shed (−198).Why split (PR3a / PR3b)
Design-lens adjudicated this session (and passed
/lexime-plan-review):単一正準形 → extract; 束ね規律 → split the parity-only refactor of freshly-merged
crash-critical code (recovery/WAL from #281/#285) from the risk-bearing
user_dict hardening. PR3a carries one invariant set — did
user_historystaybyte-for-byte identical — whose whole proof is the green regression suite; PR3b
carries is the new user_dict recovery correct. Isolating them keeps either
independently bisectable/revertible.
Verification
cargo fmt --all --check,cargo clippy --workspace --all-features -- -D warningscargo test --workspace --all-features— 587 tests, 0 failed (incl. the51KB
tests_recovery.rsrecovery matrix, the authoritative parity oracle)cargo +1.88.0 check --workspace --locked; supply-chain screen(quarantine / build.rs baseline) +
cargo deny/vet/machete/simplify(4 agents) +/code-review high(correctness/lexime-review(Axis 3 structural + Axis 5 context) — all 0 findingsTest plan
-D warningsclean — everypub(crate)item live, no dead code / unused imports🤖 Generated with Claude Code