Repository navigation
perf: WAL fsync-per-write batching/relaxed-durability escape hatch (#302) - #353
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY
Add test_normal_sync_mode_survives_checkpoint_and_reopen to verify that facts written with OpenOptions::synchronous(SyncMode::Normal) survive a checkpoint and reopen cycle. This confirms that the WAL sync mode plumbing (threaded by Task 2) works end-to-end: opening with SyncMode::Normal, writing facts, checkpointing, and reopening all preserve data correctly. Task 2 already wired both WalWriter::open_or_create call sites in db.rs to pass opts.synchronous, so this test now passes without further changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY
Adds a regression test that fails if either WalWriter call site (open_with_options initial-open, or wal_write_stamped_batch lazy-reopen) reverts to a hardcoded SyncMode::Full instead of honoring opts.synchronous — closing the gap that let this exact regression slip through once already. Also fixes five doc/wording issues from review: a misattributed re-export doc comment in lib.rs, and expanded/corrected doc comments on SyncMode::Normal and OpenOptions::synchronous covering the batching durability lever, the usize::MAX checkpoint-threshold composition, in-memory inapplicability, and precise checkpoint-flush semantics (the main file is fsynced directly; the WAL is deleted, not itself flushed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Summary
WalWriter::append_entry's per-writesync_all()(fsync) forsync_data()(fdatasync) — same durability guarantee for a pure-append file, one fewer metadata flush, unconditional in both modes.SyncMode { Full, Normal }andOpenOptions::synchronous(defaultFull, preserving current behavior exactly).Normalskips the per-write flush entirely;checkpoint()still fsyncs the main file unconditionally in both modes, remaining the hard durability boundary.SyncModeat the crate root.begin_write()/commit()batching pattern that already collapses N facts into one WAL write/fsync.Architecture.md,Performance-Tuning.md) updated and pushed separately ahead of this PR.Design:
docs/superpowers/specs/2026-08-24-wal-sync-mode-design.mdPlan:
docs/superpowers/plans/2026-08-25-wal-sync-mode.mdImplemented via subagent-driven development: fresh implementer per task, task-scoped review after each, one whole-branch review at the end with one fix wave. Notable mid-flight catch: an early task needed to touch
src/db.rsoutside its stated scope to keep the crate compiling after a signature change, and initially hardcodedSyncMode::Fullthere — silently making the new option a no-op. Caught in task review, fixed to useopts.synchronous, and the final whole-branch review added a dedicated regression test (test_normal_sync_mode_reaches_both_wal_writer_call_sites) so that specific defect can't reappear silently.Test plan
cargo test— 706 lib tests + all integration suites, 0 failurescargo fmt --check— cleancargo clippy --lib -- -D warnings— clean (--all-targetsfails on pre-existing, unrelated lints in benches/other test files, not touched by this branch)cargo doc --no-deps— no new warnings, intra-doc links resolvecargo test --test crash_kill_test— 2/2, SIGKILL during an active transact has a ~26% chance of permanently corrupting the entire database (header checksum, no recovery path) #308 fsync/crash-safety semantics unaffected🤖 Generated with Claude Code
https://claude.ai/code/session_01Nsh3ZnmoK6PTaUbp4HcqHY