writeonce/docs/superpowers/specs/2026-08-28-wal-group-commit-design.md
shoney.arickathil 026919762b docs(plan): WAL group commit — 6 tasks, databasev2 4 part A
Plan for the approved spec. Code-free per the repo convention
(docs/plan/discarded.md:54); the executor writes the code.

- T1 a failed barrier is detected and fatal — one entry point that names
  the operation, errno, WAL path and batch size, then exits. The abort
  path itself stays unexercised and the task says so rather than buying
  coverage with a fault-injection switch
- T2 the barrier moves to the drain point and replies are held; the
  request path stops committing per append. Riskiest task, and its risk
  is one place: the crash legs. Plan says STOP if they fail, do not
  adjust the test
- T3 the inline path takes the same fatal rule but keeps its own barrier,
  with a comment explaining the asymmetry so the next reader does not
  "fix" it. Looks like a no-op; without it the two paths disagree, which
  is the unevenness the spec exists to remove
- T4 prove batches actually form BEFORE measuring the payoff — otherwise
  a win gets attributed to the wrong cause. Also records peak staged
  bytes, settling the no-cap decision with a number
- T5 measure, gate, write it down. If the payoff is absent, say so and
  stop: part B must not start on an unproven premise
- T6 closeout, including the error catalogue — WO_T_IO leaving the write
  path is language-visible and must be written down

Spec corrected while planning: it pointed at durable.s1.seed as the
payoff. Wrong, structurally — worker shards hold no WAL, so a queue only
exists when other shards write, and a serial writer has nothing to batch
with. The real target is durable.sN.mixwrite: 480 ops/s at p99 5888us
against s1's 1023 at p99 664, so adding shards currently makes durable
writing WORSE. That inversion is a better argument for the iteration than
the one the story recorded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-28 09:06:09 +02:00

208 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# WAL group commit — design
> databasev2 [4](../../stories/databasev2/04-io-uring-commit.md), part A.
> Brainstormed and approved 2026-08-28.
>
> **This spec covers batching only.** The iteration was split during the
> brainstorm: part A amortises one durability barrier across many statements,
> part B (io_uring submission) is deferred until A's measurement says whether
> the blocking boundary is still the bottleneck. That split matches the
> iteration's own fork 1 — "drop-in behind `wo_wal_commit` first, an async
> variant only if the scheduler proves the blocking boundary is the
> bottleneck" — and it means the throughput win arrives behind a much smaller
> correctness surface.
## Decisions taken (the brainstorm's forks, settled)
| Fork | Decision |
| --- | --- |
| Scope | **Batching first, io_uring later.** Two independent wins were being carried as one; only the first needs a new syscall interface, and it is where most of the number lives |
| Batch boundary | **Queue-drain.** Shard 0 stages every pending write request, then commits once. No timer, no tunable |
| Failure | **Fatal, diagnosed abort.** Any failure between "RAM mutated" and "record durable" ends the process |
| Batch cap | **None initially.** Measure peak staged bytes; add a cap only if the queue's existing upstream bound proves insufficient |
| Abort coverage | Unit-test the failure *return*; the abort path itself stays covered by inspection, and that gap is disclosed |
## The problem, read off the engine
The story says "replace fsync-per-commit with io_uring group-commit". Read
against the code, the premise needed correcting: the engine does not commit per
*commit*, it commits per **statement**. `db.c` calls `wo_wal_commit`
immediately after every append, at all six sites — insert, update and remove,
each on both the inline and the DB-actor path. Every single row change is one
`pwrite` plus one `fdatasync`.
That is what the numbers say too. Iteration 22's baseline records durable writes
at **4460 ops/s** single-shard and mixed writes at **1023 ops/s**, p50 **430 µs**,
p99 **664 µs** — against **1.28M ops/s** for durable reads. Writes are roughly
290× slower than reads, and the barrier is the whole of it.
**The batching machinery already exists and is simply never used.**
`wo_wal_commit` writes `w->buf` for `w->len` bytes — a staged buffer that can
hold any number of records. Today it never holds more than one, because the
caller commits immediately after staging. So part A is closer to removing calls
than to adding a mechanism.
## The design
### The commit path
The six `wo_wal_commit` calls come out of `db.c`. Applying to RAM and staging
the record stay exactly where they are; only the barrier moves, up to the point
where shard 0 runs out of work.
Shard 0 owns the WAL — DB statements from other shards arrive as marshaled
request envelopes and are executed on shard 0's thread, serialized, and a reply
envelope unparks the requester. The change is that **the reply is held rather
than sent**: shard 0 executes and stages each queued request, keeps draining
while requests remain, then issues one barrier, and only then releases every
held reply.
Each requester therefore unparks having been acknowledged after the barrier that
carried *its* record — the ack contract the story states, which today is true
only because every batch has one member.
A statement executing inline on shard 0 (rather than arriving as a request)
stages and commits before returning, as it does now. It has no reply to hold —
it returns into its own fiber — and because the drain always commits before it
ends, nothing uncommitted is ever left staged when the inline path runs.
### Why queue-drain, and what it costs
The batch boundary is the queue going empty, not a tick and not a timer. Two
properties follow, and they are the reason to prefer it:
- **A lone writer pays nothing.** One queued request means a batch of one, which
is today's path at today's latency. Batching engages only under genuine
contention, so an idle system is not taxed to serve a busy one.
- **The batch self-tunes.** Its size is whatever actually accumulated between
drains, so it grows with load rather than with a configured number. There is
nothing to set and nothing to set wrong.
The rejected alternative was the iteration's recorded leaning, the shard tick.
That leaning was recorded when the batch was assumed to ride an io_uring
submission; with batching landing first, a tick boundary would add up to one
quantum of latency even to a lone writer — paying the cost of batching when
there is nothing to batch with.
**No batch cap ships initially, and that is a decision rather than an
oversight.** databasev2 1 established that unbounded growth is precisely how
this engine dies without warning, so the instinct to bound it is right. But the
request queue is already bounded upstream by iteration 24's mailbox caps, and a
second bound on the same quantity is a knob that can only be wrong. The proof
plan measures peak staged bytes so the question is settled by a number.
## Failure: one rule, replacing three behaviours
Today's rollback is uneven, and the code says so. An `insert` whose commit fails
removes the row again, under a comment claiming RAM never claims what disk has
not acknowledged. An `update` or a `delete` whose commit fails does **not** roll
back — its comment admits the state plainly: RAM ahead of disk, trap, do not
ack. Nothing acknowledged is lost, but the process continues with divergent
state, and batching would multiply that from one row to as many as the batch
held.
The rule that replaces it: **once a statement has mutated RAM, the only outcomes
are durable or process death.** It covers both failure points identically —
a staging failure and a barrier failure have the same consequence, RAM ahead of
disk with no way back, and only one of the three verbs can undo itself.
Retrying is not an alternative worth designing for. On Linux a failed `fsync`
may already have discarded the dirty pages, so a second call can report success
having written nothing; the recovery that actually works is replay, which
returns exactly the last durable state. That is what the log is for.
**This removes `WO_T_IO` from the write path.** A program can no longer catch a
disk failure on a write. The removal is deliberate — there was never a
recovery a program could meaningfully perform with its RAM ahead of its disk —
but it is language-visible and must be stated in the story banner and the error
catalogue, not slipped in.
The diagnostic has to earn the abort: the failing operation, the `errno` text,
the WAL path, and the number of records in the batch, on stderr, then exit with
a status of its own. Exit 1 is a trap and exit 2 is a refusal, so a durability
failure takes a third. `abort()` is rejected — a core dump on a full disk is
noise, not evidence.
## What will improve, and what will not
**Corrected 2026-08-28, after reading the baseline properly.** The spec first
pointed at `durable.s1.seed` as the payoff metric. That was wrong, and the
reason is structural rather than a matter of degree.
Worker shards hold no WAL at all — the runtime asserts it — so every DB
statement on a worker marshals to shard 0 and parks, while a statement already
on shard 0 executes inline. **A queue of write requests therefore exists only
when other shards are writing.** Batches form where there is a queue:
| Workload | Today | Batching |
| --- | --- | --- |
| `durable.sN.mixwrite` — concurrent writers across shards | **480 ops/s, p99 5888 µs** | **the target.** N shards marshal N writes and shard 0 pays N barriers serially; one barrier replaces them |
| `durable.s1.mixwrite` — concurrent writers, one shard | 1023 ops/s, p99 664 µs | **no change.** Every write is inline with no queue, so no batch forms |
| `durable.*.seed` — one serial writer | ~4460 ops/s | **no change**, under any batching scheme. There is nothing to batch with |
The inversion in those numbers is the finding worth keeping: **multi-shard
concurrent writes are currently 2× slower than single-shard with a 9× worse
p99.** Adding shards makes durable writing worse today, because every marshaled
statement still buys its own barrier on the owner. That is the pathology group
commit exists to remove, and it is a better argument for this iteration than the
one the story recorded.
**Single-shard concurrent batching is deliberately out of part A.** It would
need the inline path to park its fiber on the barrier rather than commit
synchronously — the same parking machinery part B needs anyway. Deferring it
keeps A to one mechanism, and B inherits the reason to build it.
So the acceptance criterion is scoped: **`durable.sN.mixwrite` throughput up and
its p99 down; `durable.s1.*` and both `seed` legs must not regress.** A plan
that reported "no improvement" against the s1 seed number would be measuring a
workload this change cannot help.
## Proof plan
| Claim | How it is proven |
| --- | --- |
| The payoff is real | **`durable.sN.mixwrite`** before and after on one machine, recorded in `perf-targets.md`. Today 480 ops/s, p99 5888 µs. `durable.s1.*` and both `seed` legs are regression guards, not targets — see the section above |
| Durability is unchanged | Iteration 22's crash battery, unaltered: concurrent writers, `kill -9` mid-stream, replay. **The critical test** — a kill between staging and the barrier must lose only unacknowledged writes |
| Batches actually form | New metrics for mean and peak batch size under contention. If batches are always one, the feature is inert and any throughput change came from somewhere else |
| No idle tax | Single-writer p99 must not regress against the current baseline |
| The cap question is answered | Peak staged bytes recorded per run |
| A failure is detected | `test_wal.c` asserts `wo_wal_commit` reports failure on a bad descriptor |
**One disclosed gap.** Forcing a genuine `fdatasync` failure needs a full or
read-only filesystem, which the gate cannot arrange without mount privileges.
The unit test proves the error is *detected*; the abort that follows it stays
covered by inspection. The alternative — a fault-injection switch — means
shipping a binary that can be told to kill itself, which is a worse trade. This
gap is recorded rather than hidden, because iteration 40 was exactly a fatal
path that nothing exercised.
## Out of scope
- **io_uring submission.** Part B, and it only earns its complexity if A's
measurement shows the blocking boundary still dominating. A's parking and ack
machinery is what B would build on, so nothing here is wasted either way.
- **`transaction { }`** — language iteration 18. A transaction already *is* a
staged batch, so the two compose without either knowing about the other; that
is a reason not to entangle them now.
- **Checkpoint and compaction** — databasev2 3. This changes when the barrier
runs, never what the log contains.
- **The read path.** databasev2 1 measured that appending under memory pressure
costs about 1% while random reads cost 273×, so the pressure is on reads —
but that is iteration 2's `resident: keys` question, not this one.
- **Rollback with pre-images.** Rejected above: it would add per-write cost on
every statement to serve a path that ends the process anyway.
## Alternatives rejected
**Tick-boundary batching** — the iteration's recorded leaning, superseded by
the split. It taxes an idle system to serve a busy one.
**Count-or-timer batching** — two tunables, and the timer reintroduces the tick
problem with extra configuration.
**Full rollback with an undo log** — keeps `WO_T_IO` catchable, at the price of
capturing pre-images for every update and delete, paid on every write, to
support continuing in a state the engine cannot trust.
**Keeping today's per-verb behaviour** — turns a rare one-row divergence into a
routine N-row one, silently.