Re: [PROPOSAL] Doublewrite Buffer as an alternative torn page protection to Full Page Write

Hi,

We have been working on the same problem in a PostgreSQL fork we
maintain, and we studied your patch closely along the way.

Three things came out of that which I think are worth sharing here: two
correctness questions about the posted design, numbers from hardware
that behaves differently from yours, and standby measurements.

A note on which code I am describing. The patches attached to Jakub's
reply and the tree at github.com/baotiao/postgres have diverged: the
repository has since grown checkpoint_start_pos, active_writers and
per-file generation counters, which the posted patches do not have. The
comments below are mostly against the posted patches.

1. Ring reclaim and data-file durability are not connected
----------------------------------------------------------

As I read the patch, a slot is freed in two places, and in neither of
them does anything track whether the data-file write it protects has
reached disk.

The first is in DWBufWritePage(): when the position returned by
fetch_add reaches num_slots, the code calls DWBufFlush() and wraps with
pos % num_slots. DWBufFlush() fsyncs the DWB files, which says nothing
about the data files, so the slot being overwritten can still be
protecting a page whose smgrwrite() is sitting in the kernel page
cache. The v1-0004 commit message describes this as "flush and wrap
instead of overwriting valid data"; if I follow the code correctly, the
flush covers a different file from the data at risk. I may well be
missing an ordering guarantee somewhere else in the patch.

The second is DWBufPostCheckpoint(), which zeroes write_pos and
flush_pos. It runs after SyncPostCheckpoint(), while the data-file
fsyncs come from ProcessSyncRequests() inside CheckPointGuts(), which
finished earlier. Whatever backends wrote between those two points is
durable in the DWB only, and its slots are the first ones handed out
afterwards.

The first path looks sufficient on its own, and it does not need a
checkpoint to happen:

1. A backend evicts dirty page P, takes DWB slot 500, and
smgrwrite(P) lands in the page cache. No fsync yet; md.c only
queued a sync request for the checkpointer.
2. The workload continues. After the ring's worth of writes (roughly
8000 slots at the 64 MB default) write_pos passes num_slots.
3. The writer holding pos = num_slots + 500 calls DWBufFlush(),
wraps, and overwrites slot 500.
4. The instance crashes before the checkpointer fsyncs P's segment.
5. Recovery: P fails verification (it was torn), DWBufRecoverPage()
finds nothing for it, and there is no FPI in the WAL either,
because that is the point of the mode.

In the repository version the shape is different but the question is
the same: DWBufPreCheckpoint() advances checkpoint_start_pos before
CheckPointGuts() runs, so before ProcessSyncRequests() has fsynced
anything, and the positions declared reusable again cover writes that
are only in the page cache. That version also looks like it can leave
BufferSync() phase 1 waiting indefinitely: DWBufWritePage() waits for
checkpoint_start_pos to advance, only the next DWBufPreCheckpoint()
advances it, and phase 1 runs inside the current CheckPointGuts(), so a
dirty set larger than num_slots would have nothing to wait for.

I guess, the common root is that reclaim is keyed to
the checkpoint, or in the wrap path to nothing that follows data-file
durability, while PostgreSQL's data-file durability is deferred and
asynchronous. InnoDB, as you know better than I do, frees a doublewrite
slot after the corresponding data-file write is durable rather than at a
checkpoint boundary, and that part seems to be what has to come across,
not only the buffer layout.

What worked for us was a slot lifecycle in which a slot is reused only
after the fsync covering its data-file write has completed, roughly
ALLOCATED -> WRITTEN -> DWB_FSYNCED -> DATA_WRITTEN -> DATA_FSYNCED ->
FREE, with the batch rather than the page as the unit of work.
Retirement then has to be driven by segment fsyncs, either piggybacked
on ProcessSyncRequests() or done by a dedicated worker, instead of by
the checkpoint. A pleasant side effect is that the ring no longer has
to be sized against the checkpoint window and can be sized against
concurrent writers instead, which is a much smaller number.

2. Two smaller things in the same code
--------------------------------------

The reset in DWBufPostCheckpoint() does not look fully synchronised
with writers. A writer takes its slot position with fetch_add before
the resetting flag is checked, and the reset then waits pg_usleep(1000)
for in-flight pwrites to land. The comment in the patch calls this
"conservative", and I think the window is still open: a writer
descheduled between fetch_add and pwrite for longer than a millisecond
would write into a ring that has already been declared empty.

The wrap path also looks expensive. write_pos only returns to zero at
the next DWBufPostCheckpoint(), so once the ring has wrapped, every
subsequent DWBufWritePage() sees pos >= num_slots and calls
DWBufFlush(), which fsyncs every file in the ring. With the default
64 MB ring and a write-heavy load that is one full-ring fsync per page
written for the rest of the checkpoint cycle. A lifecycle change would
remove this as well, and it also means the numbers posted so far were
measured with that cost in place, so there may be headroom above them.

3. The dependency on data_checksums = on is implicit
----------------------------------------------------

DWBufRecoverPage() is consulted only when PageIsVerified() fails.
Without data_checksums, verification is little more than a header
sanity check, so a torn write that leaves a plausible header passes and
the doublewrite copy is never consulted. So InnoDB requires checksums for
the same reason.

We ended up enforcing this at startup (FATAL when data_checksums is off
in double_writes mode); leaving it implicit did not feel safe enough.
One more thing we ran into: a 16-bit checksum still lets a torn page
through at roughly 2^-16. Repairing eagerly at startup by comparing the
on-disk page LSN against the slot LSN, rather than lazily on a failed
verification, covers that residual case, and it also covers torn pages
that redo never reads at all (hint-bit-only writes produce no WAL
record carrying an image once FPIs are off).

4. Minor, but likely to hit anyone testing the patch
----------------------------------------------------

In buffer_readv_complete_one(), the successful-repair path added by
v1-0004 jumps to page_verified without clearing the *failed_checksum
output parameter that the first PageIsVerified() call set. The jump
also skips the "else if (*failed_checksum) *ignored_checksum = true"
branch, so the report block just below runs with failed_checksum set
and zeroed, invalid and ignored all clear. buffer_readv_report() has no
case for that combination and falls through to pg_unreachable(), which
is abort() in assert-enabled builds. Unless I am misreading the flow,
the startup process then dies right after successfully repairing a
page, so recovery never finishes; in a production build it would be
undefined behaviour instead.

It is easy to fix. It also made us wary of doing the repair inside an
AIO completion callback at all.

5. Numbers from a box where FPI is not the bottleneck
-----------------------------------------------------

Since methodology came up upthread, here is a run with settings closer
to what was asked for, on hardware unlike yours.

104 cores / 1 TiB RAM, 10 NVMe in RAID0 (XFS), WAL and data on the
same array; shared_buffers = 192 GB, wal_compression = lz4,
max_wal_size = 512 GB, huge_pages = on, synchronous_commit = on;
pgbench tpcb-like, -M prepared, 2700 connections, 900 s per point,
client and server pinned to disjoint core sets. Vanilla side has
data_checksums = on so that both sides pay the same checksum cost.

checkpoint_timeout vanilla tps doublewrite tps vanilla/DWB
300 s 116 491 112 455 1.036
120 s 120 365 111 847 1.076
60 s 118 554 111 740 1.061

WAL per 900 s run: vanilla 114.2 GB / 103.7 M FPIs at 60 s and
116.5 GB / 104.7 M FPIs at 120 s;
doublewrite 39.6 GB with zero FPIs at every timeout (up to 2.9x less).

These come from our own implementation (but based on the same ideas
and very similar in shape), not from the posted patch.

On a wide NVMe array the FPI tax does not convert into throughput here:
vanilla is 3-8% ahead at every checkpoint interval, and the 2.9x WAL
reduction buys nothing locally. The win looks real but conditional. It
shows up where the WAL path is the constrained resource (replication
links, archiving, slower WAL storage, or a tight max_wal_size), rather
than on a box that can absorb 130 MB/s of extra WAL without noticing.

On this array vanilla does not suffer from frequent checkpoints; it
*benefits* from them (120 365 tps at 120 s against 116 491 at 300 s).
The checkpointer writes continuously, dirty buffers do not pile up,
backends always find clean victims, and the FPI waves are smaller and
more frequent. So the "this only helps badly-tuned servers" reading
does not quite fit our results either.

The property that survived on every box we measured is that throughput
in double_writes mode is nearly independent of checkpoint_timeout
(0.6% spread across 60/120/300 s), while vanilla's throughput and WAL
volume both move with it. For anyone tuning checkpoint frequency for
RTO rather than for throughput, that may be the more useful argument,
since unlike the raw tps ratio it does not depend on picking a
configuration where FPIs hurt.

On wal_compression: it has its own cost on the insert path, and in our
runs it does not change the shape of the comparison. The 2.9x WAL gap
above is *with* lz4 on both sides.

6. What this does to a standby
------------------------------

Jakub raised this in his first reply and expected prefetching to make
it close to free. We measured it, and on our stand it is not.

Primary with one synchronous standby, 10 GbE hop emulated with netem
(RTT 117 us), 192 GB shared_buffers on each side, SF75000, 900 s runs:

DWB 750 van 750 DWB 1500 van 1500
tps 81 013 79 549 73 933 68 319
WAL shipped 33.0 MB/s 83.1 MB/s 29.5 MB/s 71.2 MB/s
replay_lag median 63.6 s 3.1 s 57.3 s 0.014 s
standby backlog max 9.0 GB 2.0 GB 6.1 GB 0.04 GB
standby startup reads 43.5 M 34.6 K 43.5 M 37.7 K

Two things here. First, this is the only configuration where the
doublewrite side wins on tps (+1.8% and +8.2%): with a synchronous
standby the bottleneck moves into the commit-acknowledge pipeline and
the local doublewrite cost stops dominating.

Second, the standby does not keep up. The backlog grows roughly
linearly and does not recover. With no FPIs in the stream, replay has
to *read* almost every page it touches: 43.5 M reads per run, ~340 GB,
identical at both client counts, which points at the standby's own
ceiling rather than a load effect. recovery_prefetch does not appear to
be the limiter: 47 M prefetches issued, io_depth 35 of 64,
block_distance pinned at its 256 maximum. FPIs, whatever else they
cost, also act as a prefetch mechanism for the standby, and removing
them removes that.

One caveat, so I do not overstate this: "vanilla replays in real time"
is a property of that stand's WAL rate rather than of vanilla. On a
two-host pair where the primary drives far more WAL, vanilla falls
behind too, and ends the run *further* behind in bytes than the
doublewrite side (108.7 MB/s applied, 21.1 GB behind, versus 49.6 MB/s
applied and 9.7 GB behind). Time to drain after the load stops is a tie
at ~195 s, because vanilla's bytes are FPI-inflated; per *record* the
doublewrite side replays faster (773K vs 675K rec/s). Single-process
replay looks like the wall in both modes at a high enough WAL rate;
FPI volume is vanilla's shape of the cost and cold page reads are the
doublewrite mode's shape of it.

The practical consequence for this proposal is that "less WAL" does not
automatically mean an easier life for the standby, so standby numbers
probably belong next to the primary numbers for a change of this kind.
Where a deployment uses physical replication for HA, which is usually
why RTO matters in the first place, a replay lag that keeps growing
would work against the recovery-time argument that motivates the
feature.

Summary
-------

The idea looks good to me. Two things I would want
resolved before the posted patch could be adopted: the reclaim model,
which frees slots without reference to data-file durability, and the
repair path, which only runs when a checksum catches the tear. Freeing
a slot once its data-file write is durable, with retirement driven by
segment fsyncs, addresses the first, and it also makes the ring small
and independent of shared_buffers.

We have a working implementation of that batch-lifetime variant,
including startup repair, base backup, pg_rewind and pg_upgrade
handling. If there is interest in this direction, I can post it later
for comparison. Happy to go into any of the above in more detail, or to
be corrected where I have misread the patch.

Regards,
Vadim Ponomarev