Conversation
…rier per durable commit)
Implements ADR-0019 (Accepted 2026-10-01), LMDB me_mfd parity:
- zerodb-io: every writable non-WRITE_MAP env opens the data file a second
time O_WRONLY|O_DSYNC|O_CLOEXEC (MmapBacking::meta_sync) — even under
NO_SYNC/NO_META_SYNC, as the fork does; READ_ONLY/WRITE_MAP open none.
MmapBacking::write_page_durable pwrites the meta through it, durable on
return. All platforms, macOS included (maintainer decision; the weaker
macOS O_DSYNC guarantee is stated in SPEC 06).
- zerodb-core: Backing::write_page_durable (default = write_at_page +
sync_data, byte-identical to the old C4+C5 for test backings). The commit
pipeline fuses C4+C5 through it when sync_meta && !write_map (LMDB's
routing mask); C5's separate fdatasync is gone in default mode. WRITE_MAP
keeps the explicit C4 map write + C5 msync (REC-12 untouched).
- LMDB's failed-write scrub (REC-13 as amended): on a failed/short durable
meta write the slot's previous bytes are rewritten through the plain path
best-effort, then the env is poisoned — a clean reopen before power loss
cannot read back an unacknowledged commit.
- FaultBacking models the durable-on-return write: journal pending →
in-flight observer window ({absent, torn, intact}) → live-view write →
fold-SELF (never fold-all). Knobs: broken_dsync (mutation self-test, trips
REC-18.4 on default-mode cuts), fail_durable_writes (scrub tests),
set_durable_write_observer (the harness's in-flight H3 capture).
- Harness: image H3 cuts in default mode capture mid-durable-write; SIGKILL
H3 assertion TIGHTENS to 'recovers to N' for default mode (other modes keep
{N-1, N}); crash_mutation gains the broken_dsync tripwire test.
- Tests: zerodb-io fd-policy/routing/fallback units, fault-model units,
durability_barriers rewritten for the new call shapes (default = one C3
barrier + one durable write) plus scrub/poison coverage, and the
crash_dsync integration test (routing per mode, scrub end-to-end with a
real-file reopen seeing only the acked state).
- SPEC 04 TXN-61 C4/C5/H3 rows + TXN-64, SPEC 06 REC-6/REC-7 (+ macOS
platform note)/REC-9/REC-13/REC-17/REC-20, SPEC 01 S6 fd-routing
realization; ADR-0019 dated implementation note (OQ4: full-page write
kept; OQ5: the fd lives in MmapBacking). No DIVERGENCES entry: parity.
- short_txn_census: CENSUS_SYNC=1 opens both engines without NO_SYNC for the
strace syscall census.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (18)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe commit pipeline now writes default-mode metadata through a durable backing operation. The file-backed implementation can use an ChangesDurable metadata commit path
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant CommitPipeline
participant MmapBacking
participant MetaSyncFD as O_DSYNC meta-sync fd
CommitPipeline->>MmapBacking: sync_data for data pages
CommitPipeline->>MmapBacking: write_page_durable for the meta page
MmapBacking->>MetaSyncFD: write meta page
MetaSyncFD-->>MmapBacking: durable write returns
MmapBacking-->>CommitPipeline: write_page_durable returns
CommitPipeline->>CommitPipeline: run H3 and H4, then publish snapshot
Merge Risk: ⚪ Minimal · up to No actionable merge-blocking defect is established. Default commits use durable metadata writes while other modes retain their documented behavior; target-device performance measurements are still pending. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The new metadata write handle is not checked against the database handle. If the database pathname is replaced during opening, commits could write to a different file and report success without persisting metadata to the intended database. Exposure depends on who can modify the storage directory. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 61.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 73 functions across 12 files. (6 skipped: 6 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Parked — no measured performance benefit. The change is correct and the gate is green (incl. crash-test-full 10k cycles, miri, loom, fuzz), and the mechanism provably fires — an strace census of 2000 durable single-put commits on EBS io2 (Graviton) shows durability barriers drop from 2 → 1 fdatasync per commit (4002 → 2001), with the meta write moving onto the O_DSYNC fd exactly as designed. But it yields no latency improvement on any device we could measure:
Root cause: on the io2 volume reachable here, Not verified: the ADR's named decisive workload — rust-storage-bench YCSB B Branch |
1 participant
What this is
Implements ADR-0019 (Accepted by Quentin 2026-10-01): replace the two
fdatasyncs a default-mode commit issues (C3 data + C5 meta) with onedurable meta write through an
O_DSYNCdescriptor — LMDB'sme_mfdscheme.One barrier syscall per durable commit instead of two.
Builds on the campaign already merged in #83; branches cleanly off current
main(2 commits, 0 behind).Mechanism (LMDB parity, clean-room from
mdb.c)WRITE_MAPenv opens the data file a second timeO_WRONLY|O_DSYNC|O_CLOEXEC(file::open_meta_sync), stored onMmapBacking::meta_sync. Commit step C4 writes the meta page through it; thewrite returns only when the page is durable, so C5's separate
fdatasyncdisappears in default mode.
Backing::write_page_durable(pgno, psize, data)— default =write_at_page+sync_data(byte-identical to the old C4+C5 for backendswithout a synchronized write);
MmapBackingoverrides with the dsync-fdpwrite. Routing mirrors LMDB's mask: fused C4+C5 whensync_meta && !write_map;NO_META_SYNC/NO_SYNCroute the meta through the plain fdunchanged;
WRITE_MAPkeeps msync C3/C5 (REC-12 untouched).previous mapped bytes are rewritten through the plain fd before the env is
poisoned (REC-13), closing the "reopen before power loss reads an
unacknowledged
N" window.bench-gated ledger candidate, not folded in.
On macOS
O_DSYNCdoes not force the device cache, so the macOS metabarrier is weaker than today's
sync_data— macOS is a dev platform, not adurability target; stated plainly in SPEC 06. (Reviewers: confirm the macOS
path matches the approved decision.)
Crash ordering / safety
REC-7 intact: C3 (data fdatasync) still completes before the meta write begins,
and the meta write returning implies durability — so the old H3→H4 "meta
written, not yet durable" window ceases to exist in default mode (strictly
fewer reachable crash states). The H3 assertion gets stricter (
== N) indefault mode; the old
{N−1, N}outcome stays fully exercised via fault captureduring the C4 write and under the relaxed modes.
New crash coverage:
FaultBacking::write_page_durable(journal → observerwindow → fold-self), a
broken_dsyncmutation self-test that trips REC-18within 4 default-mode cuts (
crash_mutation.rs), andcrash_dsync.rs.No on-disk format change. No API change.
Spec surface to review
docs/SPEC/04-txn-mvcc.md(TXN-61 C4/C5, H3/H4),docs/SPEC/06-recovery.md(REC-6/7/9/13/17),
docs/SPEC/01-flags.md(fd-routing note),docs/adr/0019-meta-write-dsync.md, and the ADR-0004 cross-reference.Gate
fmt --checkclippy --workspace --all-targets -D warningstest --workspacedurability_barriers,crash_dsync,crash_mutation)miri -p zerodb-coredurability_barriers)loomstress(180 s)crash-test-fullfuzz-quickdiff_ops22,705 runs / 684 s 0 divergences;fuzz_image_openokThis is a durable-commit latency change whose win is device-dependent (FUA vs
flush-emulated). Per the ADR bench plan, the three-column LMDB / before / after
numbers on Graviton4 + EBS io2 (the 0.91× / 1.51 ms-p99 gap this targets)
are pending and must land before merge. The correctness gate below stands on
its own; the perf justification does not yet.
Approved by Quentin 2026-10-01 (ADR-0019 Accepted).
🤖 Generated with Claude Code
Summary by CodeRabbit
WRITE_MAPretains its separate sync step;NO_SYNCandNO_META_SYNCretain reduced durability.