8000 perf: WAL fsync-per-write is a full fsync() with no batching/relaxed-durability escape hatch · Issue #302 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

Skip to content

perf: WAL fsync-per-write is a full fsync() with no batching/relaxed-durability escape hatch #302

Closed

Description

@adityamukho

Context

Found while diagnosing why temporal-reasoning's git-history ingestion pipeline was blocking its host process's event loop. Every commit ingested there issues on the order of 20-25 separate db.execute()/checkpoint() calls (one per fact for :contains/:depends-on edges, per the workaround for #287), and each one turns into a full, synchronous fsync in WalWriter::append_entry (src/wal.rs:153-158):

pub fn append_entry(&mut self, tx_count: u64, facts: &[Fact]) -> Result<()> {
    let entry_bytes = serialize_entry(tx_count, facts)?;
    self.file.write_all(&entry_bytes)?;
    self.file.sync_all()?;
    Ok(())
}

For a bulk-load / high-write-volume caller (ingestion, migration, seeding), this is the dominant cost — dozens of synchronous disk round-trips per logical unit of work, each one a full fsync.

The consuming application already worked around the blocking symptom by moving these calls off its async event-loop thread onto a dedicated worker (confirmed safe: minigraf-ffi's UniFFI/ctypes bindings release the GIL for the duration of the call, and Arc<Mutex<Minigraf>> in minigraf-ffi/src/lib.rs serializes cross-thread access correctly). That fixes unresponsiveness but does nothing about the raw fsync volume/cost, which is a minigraf-side concern.

Proposed improvements (independent, increasing risk/leverage)

  1. sync_data() instead of sync_all() — File::sync_data() (fdatasync) skips flushing inode metadata (mtimes etc.) that sync_all() (fsync) also flushes, while still guaranteeing the appended data + the file-length metadata needed to read it back are durable. Since the WAL is pure append, there's no metadata caller here depends on beyond size. Low risk, no format change, should be a straight swap in wal.rs:156.

  2. Let non-colliding facts share one commit()/one fsync today — WriteTransaction::commit() (db.rs:1012-1059) already does exactly one WAL write for however many facts were staged via execute(), since it re-stamps everything with a single tx_count before applying. That's unrelated to the bug: EavtKey and AevtKey missing value bytes causes silent data loss when batching multi-valued facts #287 bug (which only bites when multiple facts share entity+attribute+valid_from+valid_to). So call sites that today issue one bare db.execute() per fact for unrelated entities/attributes (e.g. module-level facts + parent-commit edges + watermark update in one ingestion step) could switch to begin_write() + N execute() + one commit() and collapse N fsyncs into 1, with zero format change and no interaction with bug: EavtKey and AevtKey missing value bytes causes silent data loss when batching multi-valued facts #287. Might be worth calling out explicitly in the docs/README as the recommended pattern for bulk writers, since it's easy to miss that commit() already batches.

  3. Opt-in relaxed durability mode (biggest lever, real tradeoff) — expose something like SQLite's PRAGMA synchronous=NORMAL: skip the per-write fsync and rely on the OS page cache + a periodic/explicit checkpoint() (which already does a heavier flush) for durability. Risk window is an OS crash/power loss between writes and the next checkpoint (not an ordinary process crash — the WAL bytes and the extended file length are still in the OS's write-back cache and become durable on next fsync regardless of which process wrote them). For bulk-loaders that can safely re-run from a watermark/checkpoint on failure (like git ingestion), this could be a large throughput win. Would need to be an explicit, well-documented opt-in given Minigraf's current default is "every transact/retract is fsync-durable immediately."

Suggested priority

(1) is a near-zero-risk quick win. (2) is mostly a docs/call-site change, not a library change — maybe just needs clearer documentation. (3) is the real design decision and probably deserves its own issue/RFC if there's interest — flagging it here for visibility since it fell out of the same investigation.

Happy to provide more detail on the ingestion-side call pattern that motivated this if useful.

Activity

  1. modified the milestones: , v1.3.0 on Aug 24, 2026
  2. adityamukho commented on Aug 25, 2026

    @adityamukho
    CollaboratorAuthor

    Fixed by #353 (squash commit e2f4912). Full writeup in the PR description and docs/superpowers/specs/2026-08-24-wal-sync-mode-design.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    durabilityData durability concernperformanceBenchmark performance regressions

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      0