Skip to main content
← Back to list
01Issue
BugShippedSwamp CLIPublic
Assigneesstack72

Relationships

#3506 v3 commit outbox: a discarded entry is removed without fsync, so after a power cut recovery can land a commit whose command reported failure

Opened by stack72 · 10/15/2026· Shipped 10/15/2026

Origin: a low finding from the adversarial review in verify-reviews run 002b0298-5d11-48bd-9be1-1da68a96527e, at commit cad9da678. It was left as a verdict in PR #3142 (merged 2026-10-15) and not fixed. Severity: low. The window is narrow, but it breaks a stated rule, and the fix touches only failure paths.

Background

Phase 4 F (#3490) added the v3 commit outbox, commits-outbox.db, which lives beside commits.db:

  1. A commit-log unit prepares each commit in the outbox just before it appends it.
  2. It then appends the commit and runs the post-commit chunk check.
  3. Finally it removes the entry (the settle).

A unit also removes entries when it discards them:

  • on a refusal, along with the unit's run entries (decision 7);
  • on an abandon;
  • when commit() ends with a commit still failing.

In each of those cases the failure was reported to the caller. The rule in the module docs and in design/enablers/datastores.md ("v3 commit outbox") is that a reported failure is never landed later. So only a process that dies should leave entries behind.

On the next writer hold, recovery (recoverCommitOutbox) lands any entry whose partition head still equals the entry's base.

The problem

To cut the per-commit cost (benchmarked against the merge-base), every removal runs with synchronous=NORMAL, so it is not fsynced. That is safe for a settle: if a settle is lost, the entry comes back after a power cut, recovery sees the txnId already landed, and it just settles it again.

It is not safe for a discard. The failure sequence:

  1. A unit's append fails with a transient error, such as "disk full", and the entry stays in the outbox while the unit is open.
  2. commit() ends with that commit still failing. The unit discards its entries, and the caller is told the command failed.
  3. Power is cut before the WAL frame of that relaxed removal reaches disk. The discarded entry is back.
  4. The next writer hold runs recovery. The partition head still equals the entry's base, so recovery lands a commit whose command reported failure.

The window is narrow: a power cut right after the discard of a failed commit, with the head unchanged since. SIGKILL and OOM are not affected, because WAL NORMAL survives a process crash and only a power loss drops it.

Where the code is (as merged in #3142)

  • src/infrastructure/datastore_v3/sqlite_commit_outbox.ts
    • remove() calls #write(..., { create: false, durable: false }), around line 195.
    • #write() takes the durable option, around line 349, and switches to PRAGMA synchronous=NORMAL when it is false, around line 356.
  • src/infrastructure/datastore_v3/commit_log_unit_of_work.ts
    • Settle after a landed commit: await outbox.remove([key]) in appendGroup, around line 932. This one may stay relaxed.

    • discardEntries(), around line 963, is called from:

      • the refusal path, around line 459;
      • abandon, around line 529;
      • a failed end, around line 552.

      These must be durable.

  • src/infrastructure/datastore_v3/outbox_commit_sink.ts: SinkOutbox.remove is shared by settles and discards, around line 91. It records failed removals as owed and retries them in retryOwedRemovals() when the handle closes. A retried discard must be durable too.
  • src/domain/datastore_v3/commit_outbox.ts: the CommitOutbox.remove port.

Suggested fix

  1. Split removal into two operations on the port and the sink:

    • settle(keys): relaxed (durable: false), used only after a commit landed;
    • discard(keys): durable (synchronous=FULL), used by discardEntries and the owed-removal retry.

    The smaller alternative is a durable flag on remove.

  2. Keep the settle relaxed. It is most of the saving the benchmark showed.

  3. Keep owed discards durable when retryOwedRemovals retries them.

  4. Update the comment in remove() ("A removal lost to a power cut costs one more settle or drop … never a commit"). It holds only for settles.

Tests to add

Red-check each test against the merged code first.

  • Unit (sqlite_commit_outbox_test.ts):

    • a discard runs with synchronous=FULL;
    • a settle runs with NORMAL.

    Probe this by reading PRAGMA synchronous inside the write, or with a test hook. There is no portable way to simulate a power cut in-process.

  • Unit (commit_log_unit_of_work_outbox_test.ts): the refusal, abandon and failed-end paths call the durable discard, never the relaxed settle. Extend OutboxMemorySink in commit_log_test_helpers.ts to record which one ran.

  • Model test (integration/datastore_v3_outbox_model_property_test.ts): optionally add a "power cut" command that resurrects entries removed by relaxed writes since the last durable one. The invariant "a commit whose failure was reported never lands later" should then hold for discards but may resurrect settles.

Cost to re-measure

Discards happen only on failure paths: refusals, abandons and failed commits. The common path (prepare, append, settle) is unchanged, so the per-commit cost should stay at about +0.3–0.4 ms in process. Use the same A/A benchmark against the merge-base to confirm.

  • A stalled writer on another host whose lease expired counts as gone. This is intended and documented.
  • A unit that is never ended keeps its entries. That only happens if the UnitOfWork contract is broken; runInCommitLogRoot always ends the unit.
  • A reused pid, or a failed chain record after the fence was raised. Both end in a reported drop, never a wrong landing.

Blocked by: nothing. Live for filesystem-v3 datastores today, so it doesn't wait for 4E.

02Bog Flow
✓OPEN✓TRIAGED✓IN PROGRESS✓SHIPPED+ 1 MOREASSIGNED+ 5 MOREREVIEW+ 11 MOREPR_MERGED+ 2 MORESESSION_SUMMARIZED

Shipped

10/15/2026, 8:27:06 PM

Click a lifecycle step above to view its details.

03Sludge Pulse
stack72 assigned stack7210/15/2026, 6:53:23 PM

Sign in to post a ripple.