On Tue, Apr 21, 2026 at 04:49:12PM +0300, Alexander Korotkov wrote: > I also think that usage of MarkBufferDirty() here is safe. If I > understood correctly. > 1) When wal_log_hints = on, should be completely safe. Even if we > have torn page after the crash, during recovery FPI from the primary > should come first.
I think this change (commit c06d1a4) is incorrect. Assume checksums and full-page writes are enabled, both defaults. Before this change, redo would transition FSM pages clean->dirty only via XLogReadBufferForRedo() of an FPI. At end of recovery, the FSM passed checksum validation. After $SUBJECT, nothing stops the following sequence of events: finish restartpoint; dirty FSM page via heap_xlog_*; crash tears write of that FSM page; resume recovery; end recovery without curing the torn page. The FSM code's own use of RBM_ZERO_ON_ERROR makes FSM code accept torn pages. Other callers assume normal reads will succeed on FSM_FORKNUM, e.g., the heapam_relation_copy_data() call to RelationCopyStorage(). Claude wrote a test of that, attached. To be fair, freespace/README itself incorrectly predicts the commit is okay, by saying things like, "we rely on a bunch of self-correcting measures to repair possible corruption." Long-term, we do have at least these alternatives: - Keep the pre-2026-05 invariant that FSM is free from torn pages at end of recovery. (We'll likely still have the property that heap_xlog_* will read torn FSM pages during recovery. That arises, I think, because the FPI that fixes the torn page may be later in the WAL stream. FSM is unlike other forks this way; other forks write before they read.) - Require all readers of FSM_FORKNUM to use RBM_ZERO_ON_ERROR or the equivalent. Side excursion that I'm dumping here in case it saves someone time: I was suspicious of the older MarkBufferDirty(), in FreeSpaceMapPrepareTruncateRel(). Subtle interactions make it harmless. It involves a WAL write via log_newpage_buffer(), so it almost follows the transam/README protocol. The odd part is that the protocol-required actions happen across two WAL records. The redo of XLOG_FPI comes first and dirties the buffer properly. Then redo of XLOG_SMGR_TRUNCATE calls FreeSpaceMapPrepareTruncateRel() and dirties it again. That would normally not be okay. However, the primary side used DELAY_CHKPT_COMPLETE to prevent a checkpoint or restartpoint between the two records. No problem behavior arises. > Should we push it to all supported branches? (The commit did get back-patched, even though its commit log doesn't say that.)
commit 0b07f8b (HEAD -> fsm-dirty-defect-tests) Author: Noah Misch <[email protected]> AuthorDate: Tue Sep 1 21:02:10 2026 +0000 Commit: Noah Misch <[email protected]> CommitDate: Tue Sep 1 14:07:06 2026 -0700 recovery: focused single-defect test for the c06d1a4 torn-FSM survival A minimal, standalone extract of the "D1 leg C" scenario already inside 057_fsm_dirty_torn.pl, for making one point without handing over the larger multi-defect file. On a default cluster (data checksums on, full_page_writes on, wal_log_hints off; no online checksum enabling and no injection points), a heap page is filled to under one FSM step of free space, then a crash tears the standby's unprotected FSM-page write. Recovery re-records "0 bytes free" onto the zeroed torn page, which is no change, so fsm_set_avail() never re-dirties the buffer and the torn page survives to end of recovery. After promotion, ALTER TABLE ... SET TABLESPACE copies the relation's forks through RelationCopyStorage(), whose verifying reads hit the surviving torn FSM page: ERROR: invalid page in block 2 of relation "base/5/<relfilenode>_fsm" Since c06d1a4, XLogRecordPageWithFreeSpace() uses MarkBufferDirty() instead of MarkBufferDirtyHint(); before it, a checksummed standby wrote no such page. 15 GUARD assertions prove the choreography (the standby wrote the page unprotected, insert #39 carries a full-page image and #40 does not, the crash tore the write, and the in-memory repair never reaches disk). Three CORRECT assertions fail on master and must pass after a fix: the two in-server signals (a startup-process WARNING and a pg_stat_database.checksum_failures bump on a fault-free cluster) and the SET TABLESPACE ERROR. This test avoids pg_basebackup and pg_checksums deliberately. The page self-corrects once any nonzero free-space change touches that FSM leaf, so a fix must keep the GUARDs passing (i.e. keep persisting replayed FSM updates). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_014bw5CScTPcbfh7xU82XLmN --- src/test/recovery/Makefile | 3 + src/test/recovery/meson.build | 1 + .../t/059_fsm_torn_page_survives_recovery.pl | 269 +++++++++++++++++++++ 3 files changed, 273 insertions(+) diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile index 9c4102b..07da006 100644 --- a/src/test/recovery/Makefile +++ b/src/test/recovery/Makefile @@ -12,6 +12,9 @@ EXTRA_INSTALL=contrib/pg_prewarm \ contrib/pg_stat_statements \ contrib/test_decoding \ + contrib/pg_freespacemap \ + contrib/pg_buffercache \ + contrib/pg_walinspect \ src/test/modules/injection_points \ src/test/modules/test_wait_lsn diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index b86fbff..8dcf200 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -67,6 +67,7 @@ tests += { 't/056_standby_snapshot_export.pl', 't/057_fsm_dirty_torn.pl', 't/058_fsm_enable_checksums.pl', + 't/059_fsm_torn_page_survives_recovery.pl', ], }, } diff --git a/src/test/recovery/t/059_fsm_torn_page_survives_recovery.pl b/src/test/recovery/t/059_fsm_torn_page_survives_recovery.pl new file mode 100644 index 0000000..55bf810 --- /dev/null +++ b/src/test/recovery/t/059_fsm_torn_page_survives_recovery.pl @@ -0,0 +1,269 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# A torn free-space-map page written during recovery can survive to the end of +# recovery, on a cluster with data checksums and full_page_writes -- both +# defaults -- and then break a normal SQL command. +# +# Since commit c06d1a4, heap-record replay marks FSM pages dirty with +# MarkBufferDirty() (was MarkBufferDirtyHint()). The FSM page is named in no +# WAL record, so the standby writes it with a fresh checksum, pd_lsn 0, and no +# full-page image anywhere. If a crash tears such an 8 KB write, recovery is +# expected to cure the page. It does -- unless the heap page the FSM entry +# describes is essentially full. Then replay re-records "0 bytes free" onto +# the freshly-zeroed torn page, which is no change, so fsm_set_avail() does not +# re-dirty the buffer, nothing is written back, and the torn page stays on +# disk past the end of recovery. +# +# After promotion, ALTER TABLE ... SET TABLESPACE copies every fork through +# RelationCopyStorage(), which reads with normal (verifying) reads -- unlike +# the FSM code's own RBM_ZERO_ON_ERROR -- so it hits the surviving torn FSM +# page and fails: ERROR: invalid page in block 2 of relation "...". No +# hardware faulted, and before c06d1a4 a checksummed standby wrote no such page. +# +# The tear is simulated (a TAP test cannot cut a physical write): while the +# server is stopped, block 2's second 4 KB (the FSM leaf nodes) is reverted to +# its pre-write content, leaving the first 4 KB -- header, checksum, and the +# FSM upper nodes -- as freshly written. That is byte-for-byte what a power +# loss can persist when one 8 KB write lands as two 4 KB writes. +# +# "GUARD:" assertions prove the choreography really happened; they pass today. +# "CORRECT:" assertions state what a non-defective server must do; each fails +# on an affected build, so a failure is exactly this defect and a post-fix pass +# is meaningful. The affected page self-corrects the moment any nonzero +# free-space change touches that FSM leaf; this test stops before that, and a +# fix must keep the GUARDs passing (i.e. keep persisting replayed FSM updates). + +use strict; +use warnings FATAL => 'all'; +use Fcntl qw(:seek); +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +use constant BLCKSZ => 8192; + +# ------------------------------------------------------------------ helpers + +# Return the 8 KB block $blkno of $file. +sub read_page +{ + my ($file, $blkno) = @_; + open my $fh, '<', $file or die "open $file: $!"; + binmode $fh; + sysseek($fh, $blkno * BLCKSZ, SEEK_SET) // die "sysseek $file: $!"; + my $n = sysread($fh, my $buf, BLCKSZ); + die "short read of $file block $blkno" unless defined $n && $n == BLCKSZ; + close $fh; + return $buf; +} + +# Simulate a torn write of block $blkno: keep the first 4 KB now on disk (the +# new write: header, pd_checksum, FSM upper nodes) and revert the second 4 KB +# (the FSM leaf nodes) to $old_page's second half (the pre-write content). +# The owning server must be stopped. Returns the bytes now on disk. +sub tear_page +{ + my ($file, $blkno, $old_page) = @_; + my $cur = read_page($file, $blkno); + my $torn = substr($cur, 0, BLCKSZ / 2) . substr($old_page, BLCKSZ / 2); + open my $fh, '+<', $file or die "open $file: $!"; + binmode $fh; + sysseek($fh, $blkno * BLCKSZ, SEEK_SET) // die "sysseek $file: $!"; + my $w = syswrite($fh, $torn, BLCKSZ); + die "short write of $file block $blkno" unless defined $w && $w == BLCKSZ; + close $fh or die "close $file: $!"; + return $torn; +} + +# FSM page field accessors (see src/backend/storage/freespace/fsm_internals.h): +# the leaf node for heap block N is at page byte 4123 + N; the root is byte 28. +sub fsm_leaf_byte { return ord(substr($_[0], 4123 + $_[1], 1)); } +sub fsm_root_byte { return ord(substr($_[0], 28, 1)); } + +# Force a restartpoint the standby cannot skip: replay a fresh primary +# checkpoint record first, then CHECKPOINT locally. +sub restartpoint +{ + my ($primary, $standby) = @_; + my $off = -s $standby->logfile; + $primary->safe_psql('postgres', 'CHECKPOINT'); + $primary->wait_for_catchup($standby); + $standby->safe_psql('postgres', 'CHECKPOINT'); + $standby->wait_for_log(qr/restartpoint complete/, $off); + return; +} + +# pg_stat_database.checksum_failures for 'postgres'. Stats flushes are async, +# so if the log since $off shows a verification failure, wait for the counter +# to catch up; that way the CORRECT comparison against 0 fails deterministically +# on an affected build instead of racing the flush. +sub settled_checksum_failures +{ + my ($node, $off) = @_; + if ($node->log_contains(qr/page verification failed/, $off)) + { + $node->poll_query_until('postgres', + q{SELECT coalesce(checksum_failures, 0) > 0 FROM pg_stat_database + WHERE datname = 'postgres'}); + } + return $node->safe_psql('postgres', + q{SELECT coalesce(checksum_failures, 0) FROM pg_stat_database + WHERE datname = 'postgres'}); +} + +# All corruption-alarm lines in $node's log since $off ('' if none). +sub corruption_log_lines +{ + my ($node, $off) = @_; + my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off); + return join("\n", + grep { /page verification failed|invalid page in block/ } + split(/\n/, $log)); +} + +# Evict FSM block 2 of $tbl; returns "buffer_evicted|buffer_flushed" +# ('t|t' = the buffer was dirty and written; 't|f' = it was clean). +sub evict_fsm +{ + my ($node, $tbl) = @_; + return $node->safe_psql('postgres', qq{ +SELECT buffer_evicted, buffer_flushed FROM pg_buffercache_evict( + (SELECT bufferid FROM pg_buffercache + WHERE relfilenode = pg_relation_filenode('$tbl') + AND reldatabase = (SELECT oid FROM pg_database + WHERE datname = current_database()) + AND relforknumber = 1 AND relblocknumber = 2))}); +} + +# -------------------------------------------------------------------- setup + +my $primary = PostgreSQL::Test::Cluster->new('primary'); +$primary->init(allows_streaming => 1); # data checksums on by default +$primary->append_conf('postgresql.conf', q{ +autovacuum = off +wal_log_hints = off +checkpoint_timeout = 1h +bgwriter_lru_maxpages = 0 +}); +$primary->start; +is($primary->safe_psql('postgres', 'SHOW data_checksums'), + 'on', 'GUARD: data checksums are on'); +is($primary->safe_psql('postgres', 'SHOW full_page_writes'), + 'on', 'GUARD: full_page_writes is on'); +$primary->safe_psql('postgres', + 'CREATE EXTENSION pg_freespacemap; CREATE EXTENSION pg_buffercache; ' + . 'CREATE EXTENSION pg_walinspect;'); +$primary->backup('bk'); + +# 32MB shared_buffers so nothing evicts a standby buffer except when we ask. +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($primary, 'bk', has_streaming => 1); +$standby->append_conf('postgresql.conf', 'shared_buffers = 32MB'); +$standby->start; + +# --------------------------------------------------------------------- body + +$primary->safe_psql('postgres', + 'CREATE TABLE t (a int, b text) WITH (autovacuum_enabled = off)'); +restartpoint($primary, $standby); + +# Fill heap block 0 to 992 bytes free (stored as the FSM byte 31). The primary +# inserter is never rejected, so the primary keeps no FSM and no WAL record +# names the FSM fork; only the standby's replay builds one. +$primary->safe_psql('postgres', q{DO $$ BEGIN FOR i IN 1..38 LOOP + INSERT INTO t VALUES (i, repeat('x', 150)); END LOOP; END $$;}); +$primary->wait_for_catchup($standby); +is($standby->safe_psql('postgres', + "SELECT avail FROM pg_freespace('t') WHERE blkno = 0"), + '992', 'GUARD: standby records 992 bytes free for heap block 0'); + +my $relpath = $primary->safe_psql('postgres', "SELECT pg_relation_filepath('t')"); +my $filenode = $primary->safe_psql('postgres', "SELECT pg_relation_filenode('t')"); +my $fsm = $standby->data_dir . "/${relpath}_fsm"; +note "standby FSM file: $fsm"; + +# Land that value on disk (clean), so the later tear reverts to it. +restartpoint($primary, $standby); +my $old = read_page($fsm, 2); +is(fsm_leaf_byte($old, 0), 31, + 'GUARD: the restartpoint wrote 31 (992 bytes free) to disk'); + +# Insert #39 is heap block 0's first touch after that checkpoint, so it carries +# a full-page image (restored unconditionally on replay: no FSM update). +# Insert #40 (812 bytes) leaves 28 bytes free, under one FSM step (32 bytes), +# so its replay lowers the FSM entry to 0 -- and carries no image. +my $lsn_a = $primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn()'); +$primary->safe_psql('postgres', "INSERT INTO t VALUES (39, repeat('x', 150))"); +$primary->wait_for_catchup($standby); +is($standby->safe_psql('postgres', + "SELECT avail FROM pg_freespace('t') WHERE blkno = 0"), + '992', 'GUARD: the FPI-carrying insert #39 left the FSM unchanged'); +$primary->safe_psql('postgres', "INSERT INTO t VALUES (40, repeat('x', 770))"); +my $lsn_b = $primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn()'); +is($primary->safe_psql('postgres', "SELECT pg_relation_size('t') / 8192"), + '1', 'GUARD: t is still a single heap block'); +is($primary->safe_psql('postgres', qq{ +SELECT count(*) FILTER (WHERE block_fpi_length > 0 + AND block_fpi_info::text LIKE '%APPLY%') + || '|' || + count(*) FILTER (WHERE coalesce(block_fpi_length, 0) = 0) + FROM pg_get_wal_block_info('$lsn_a', '$lsn_b') + WHERE relfilenode = $filenode AND relforknumber = 0}), + '1|1', 'GUARD: insert #39 carries a full-page image, insert #40 does not'); +$primary->wait_for_catchup($standby); +is($standby->safe_psql('postgres', + "SELECT avail FROM pg_freespace('t') WHERE blkno = 0"), + '0', 'GUARD: insert #40 replay lowered the standby FSM entry to 0'); + +# Write that 0 to disk the way any eviction/bgwriter would: no FPI, pd_lsn 0. +is(evict_fsm($standby, 't'), 't|t', + 'GUARD: eviction flushed the replay-dirtied FSM page'); +my $written = read_page($fsm, 2); +is(fsm_leaf_byte($written, 0), 0, 'GUARD: on-disk FSM leaf is now 0'); +is(fsm_root_byte($written), 0, 'GUARD: on-disk FSM root is now 0'); + +# Replay a fresh primary checkpoint before the crash, so post-crash +# restartpoints are permitted -- and shown to still not heal the page. +$primary->safe_psql('postgres', 'CHECKPOINT'); +$primary->wait_for_catchup($standby); + +# Crash, and tear the 8 KB write: new first half (root 0, checksum over the new +# page), stale second half (leaf 31, from the earlier restartpoint). +$standby->stop('immediate'); +my $torn = tear_page($fsm, 2, $old); +is(fsm_root_byte($torn), 0, 'GUARD: torn page root = 0 (new first half)'); +is(fsm_leaf_byte($torn, 0), 31, 'GUARD: torn page leaf = 31 (stale second half)'); + +my $off = -s $standby->logfile; +$standby->start; +$primary->wait_for_catchup($standby); + +# Replay re-reads the torn page (RBM_ZERO_ON_ERROR) and re-records 0, which is +# no change on the zeroed page, so the buffer is never re-dirtied. +is(corruption_log_lines($standby, $off), '', + 'CORRECT: crash-restart replay raises no checksum-corruption alarm'); +is(settled_checksum_failures($standby, $off), '0', + 'CORRECT: checksum_failures stays 0 (there is no real corruption)'); + +# A restartpoint now runs (the fresh primary checkpoint was re-replayed) ... +restartpoint($primary, $standby); + +# ... yet the torn page still cannot be healed: the in-memory page is clean +# (zeroed), so the repair never reaches disk. +is(evict_fsm($standby, 't'), 't|f', + 'GUARD: evicting the buffer writes nothing (the repair never reached disk)'); + +# Promote, then move the table to another tablespace. ALTER TABLE ... SET +# TABLESPACE copies every fork through RelationCopyStorage(), whose reads +# verify the checksum, so it hits the surviving torn FSM page and ERRORs. +$standby->promote; +ok($standby->poll_query_until('postgres', 'SELECT NOT pg_is_in_recovery()'), + 'GUARD: standby promoted to primary'); +$standby->safe_psql('postgres', + "SET allow_in_place_tablespaces = true; CREATE TABLESPACE ts LOCATION ''"); +my ($ret, $out, $err) = + $standby->psql('postgres', 'ALTER TABLE t SET TABLESPACE ts'); +is($ret, 0, 'CORRECT: ALTER TABLE ... SET TABLESPACE succeeds'); +note "ALTER TABLE SET TABLESPACE stderr: $err" if $err ne ''; + +done_testing();
