Hi,
Reviving this thread: the bug reported here in 2024 is still present
on master, and has become more harmful since.
To recap the root cause: RelationCopyStorageUsingBuffer() copies every
fork page by page and WAL-logs each copied page with
log_newpage_buffer(buf, page_std = true). page_std=true punches the
[pd_lower, pd_upper) range out of the full-page image as a hole, to
save WAL. That works for heap and index pages, but VM and FSM pages
never maintain pd_lower/pd_upper - they keep whatever PageInit() left
(pd_lower = SizeOfPageHeaderData, pd_upper = BLCKSZ). So for those
forks the "hole" covers the whole content area, the FPI carries
nothing but the header, and replay zero-fills it. The primary lands
the real VM/FSM contents on disk, while the standby replays all-zero
pages. As far as I can see, this call site is the only one that logs
VM/FSM pages in standard mode - everywhere else they are logged with
page_std=false.
What changed since the last discussion
--------------------------------------
The divergence was assessed back then as "not actively harmful". That
no longer holds in v19: commit add323da40a added
Assert(BufferIsDirty(vmbuffer)) to heap_xlog_prune_freeze(), which
makes the divergence fatal on assert builds after a switchover:
1. The promoted standby has a zeroed VM, so VACUUM FREEZE sets the
bits again, emitting PRUNE records with no FPI for the VM buffer.
2. The old primary, rejoined as standby, still has the bits set with
an older page LSN, so replay takes the redo branch - but
visibilitymap_set() is a no-op there (bits already set) and the
buffer stays clean.
3. Assert(BufferIsDirty(vmbuffer)) fires, and the node crash-loops on
that record.
And even without assertions, the new database on the standby silently
loses all all-visible/all-frozen bits and all free space data of every
copied relation.
Repro
-----
With a streaming standby attached:
-- on the primary
CREATE DATABASE src_db;
\c src_db
CREATE EXTENSION pageinspect;
CREATE TABLE t(a int);
INSERT INTO t SELECT generate_series(1, 10000);
VACUUM FREEZE t;
-- back in postgres
CREATE DATABASE dst_db TEMPLATE src_db STRATEGY WAL_LOG;
-- then on both nodes, in dst_db:
SELECT encode(get_raw_page('t', 'vm', 0), 'hex');
SELECT encode(get_raw_page('t', 'fsm', 0), 'hex');
-- primary shows real content, standby is all zeros
For the assert crash: run with full_page_writes=off so the PRUNE
record carries no FPI for the VM page, promote the standby, rejoin the
old primary as its standby, then VACUUM (FREEZE, DISABLE_PAGE_SKIPPING)
t on the new primary. The old primary dies on the assert while
replaying the PRUNE records.
0002 adds a TAP test to src/test/recovery/t/034_create_database.pl
which automates the scenario above: all new checks fail without 0001
and pass with it (verified on REL_19_STABLE with --enable-cassert).
Fix
---
The v1 patch logged all copied pages with page_std=false, which was
objected to because of the WAL volume increase on the main fork: holes
would no longer be punched from heap/index pages. Attached v2 instead
makes the decision per fork, along the lines Michael suggested. The
VM and FSM forks are the only ones whose pages never maintain a
standard page layout, so only their pages are logged whole:
- /* WAL-log the copied page. */
+ /*
+ * WAL-log the copied page. VM/FSM pages never maintain
+ * pd_lower/pd_upper, so hole punching would omit their entire
+ * content from the FPI.
+ */
if (use_wal)
- log_newpage_buffer(dstBuf, true);
+ log_newpage_buffer(dstBuf,
+ forkNum != VISIBILITYMAP_FORKNUM &&
+ forkNum != FSM_FORKNUM);
Main and init forks keep hole punching, so their WAL volume is
unchanged; the extra WAL is limited to the VM/FSM forks, which are
small. The fork number is available locally in
RelationCopyStorageUsingBuffer() (it already branches on it for
use_wal), so no plumbing through dbcommands.c is needed.
The bug goes back to v15, where STRATEGY WAL_LOG was added
(9c08aea6a30); the assertion crash additionally requires v19
(add323da40a), so older branches see only the VM/FSM divergence.
This should be backpatched to all supported versions.
Regards,
Rogers Wang
diff --git a/src/backend/storage/buffer/bufmgr.c
b/src/backend/storage/buffer/bufmgr.c
index 169829eb020..6cee2e21954 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -5454,9 +5454,15 @@ RelationCopyStorageUsingBuffer(RelFileLocator srclocator,
memcpy(dstPage, srcPage, BLCKSZ);
MarkBufferDirty(dstBuf);
- /* WAL-log the copied page. */
+ /*
+ * WAL-log the copied page. VM/FSM pages never maintain
+ * pd_lower/pd_upper, so hole punching would omit their entire
+ * content from the FPI.
+ */
if (use_wal)
- log_newpage_buffer(dstBuf, true);
+ log_newpage_buffer(dstBuf,
+ forkNum !=
VISIBILITYMAP_FORKNUM &&
+ forkNum !=
FSM_FORKNUM);
END_CRIT_SECTION();
diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile
index 9c4102b6b2c..dfd3617c84c 100644
--- a/src/test/recovery/Makefile
+++ b/src/test/recovery/Makefile
@@ -9,7 +9,8 @@
#
#-------------------------------------------------------------------------
-EXTRA_INSTALL=contrib/pg_prewarm \
+EXTRA_INSTALL=contrib/pageinspect \
+ contrib/pg_prewarm \
contrib/pg_stat_statements \
contrib/test_decoding \
src/test/modules/injection_points \
diff --git a/src/test/recovery/t/034_create_database.pl
b/src/test/recovery/t/034_create_database.pl
index 194d1837e23..bff6c65a32b 100644
--- a/src/test/recovery/t/034_create_database.pl
+++ b/src/test/recovery/t/034_create_database.pl
@@ -42,4 +42,133 @@ is($result, "0",
"check that there are no tables from template on new database after
crash"
);
+# Check that the VM and FSM contents of relations copied with the
+# WAL_LOG strategy are preserved on a standby.
+#
+# The copy WAL-logs each page via log_newpage_buffer(). VM and FSM
+# pages never maintain pd_lower/pd_upper after PageInit(), so logging
+# them with page_std=true would omit their whole content area from the
+# FPI and replay would zero-fill it. When the promoted standby later
+# vacuums the table, replaying the VM-setting PRUNE records on the old
+# primary (where the bits are already set) would be a no-op that leaves
+# the VM buffer clean, tripping Assert(BufferIsDirty(vmbuffer)).
+
+# full_page_writes=off guarantees that the VM-setting record generated
+# below carries no FPI; the large checkpoint_timeout prevents WAL writes
+# while the old primary is demoted, so no rewind is needed then.
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->append_conf(
+ 'postgresql.conf', qq{
+full_page_writes = off
+autovacuum = off
+checkpoint_timeout = 1h
+});
+$node_primary->start;
+
+# Create a source database with a table whose VM bits are all set
+# (all-visible and all-frozen) and whose FSM is filled.
+$node_primary->safe_psql('postgres', "CREATE DATABASE src_db");
+$node_primary->safe_psql(
+ 'src_db', qq{
+CREATE EXTENSION pageinspect;
+CREATE TABLE test (a int);
+INSERT INTO test SELECT generate_series(1, 10000);
+VACUUM FREEZE test;
+});
+
+# Check that the VM page has real content beyond the 24-byte page
+# header, so that the comparison below is meaningful.
+my $vm_hex = $node_primary->safe_psql('src_db',
+ "SELECT encode(get_raw_page('test', 'vm', 0), 'hex')");
+like(substr($vm_hex, 48),
+ qr/[^0]/, 'VM bits are set on src_db before CREATE DATABASE');
+
+# Flush and take a base backup; the backup copies the VM/FSM files
+# verbatim.
+$node_primary->safe_psql('postgres', "CHECKPOINT");
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Create dst_db from src_db: the primary lands the copied forks on
+# disk, while the standby replays the FPIs.
+$node_primary->safe_psql('postgres',
+ "CREATE DATABASE dst_db TEMPLATE src_db STRATEGY WAL_LOG");
+$node_primary->wait_for_catchup($node_standby);
+
+# Compare the VM and FSM pages with pageinspect.
+my $p_vm = $node_primary->safe_psql('dst_db',
+ "SELECT encode(get_raw_page('test', 'vm', 0), 'hex')");
+my $s_vm = $node_standby->safe_psql('dst_db',
+ "SELECT encode(get_raw_page('test', 'vm', 0), 'hex')");
+is($s_vm, $p_vm, 'new database: VM page identical on both nodes');
+
+my $p_fsm = $node_primary->safe_psql('dst_db',
+ "SELECT encode(get_raw_page('test', 'fsm', 0), 'hex')");
+my $s_fsm = $node_standby->safe_psql('dst_db',
+ "SELECT encode(get_raw_page('test', 'fsm', 0), 'hex')");
+is($s_fsm, $p_fsm, 'new database: FSM page identical on both nodes');
+
+# Switch over and check that the old primary survives replaying the
+# PRUNE records generated by vacuuming the table on the new primary.
+
+# Move the redo point past the CREATE DATABASE records, so that crash
+# recovery of the old primary does not replay the CREATE DATABASE FPIs,
+# and wait until the standby has replayed the checkpoint.
+$node_primary->safe_psql('postgres', "CHECKPOINT");
+$node_primary->wait_for_catchup($node_standby);
+
+# Promote the standby, then demote the old primary to a standby of the
+# new primary: its WAL ends exactly at the promotion fork point, so no
+# rewind is needed. The slot preserves the WAL generated below.
+$node_standby->promote;
+
+$node_primary->stop('immediate');
+$node_standby->safe_psql('postgres',
+ "SELECT pg_create_physical_replication_slot('old_primary')");
+my $new_primary_connstr = $node_standby->connstr;
+$node_primary->append_conf(
+ 'postgresql.conf', qq{
+primary_conninfo = '$new_primary_connstr application_name=old_primary'
+primary_slot_name = 'old_primary'
+});
+$node_primary->set_standby_mode;
+$node_primary->start;
+
+# Wait until the new standby is fully caught up.
+$node_standby->wait_for_catchup($node_primary);
+
+# Note the log offset now: the crash can follow the VACUUM within
+# milliseconds.
+my $log_offset = -s $node_primary->logfile;
+
+# If the VM bitmap is all zero on the new primary, vacuum sets the bits
+# again, generating PRUNE records without an FPI for the VM buffer.
+$node_standby->safe_psql('dst_db',
+ "VACUUM (FREEZE, DISABLE_PAGE_SKIPPING) test");
+
+# Replay of the PRUNE records on the new standby would then be a no-op
+# that leaves the VM buffer clean and crashes on
+# Assert(BufferIsDirty(vmbuffer)), in which case the standby never
+# replays past the target LSN and this poll times out. The crash
+# follows the VACUUM within milliseconds, so a short timeout suffices.
+my $target_lsn = $node_standby->lsn('flush');
+my $saved_timeout = $PostgreSQL::Test::Utils::timeout_default;
+$PostgreSQL::Test::Utils::timeout_default = 15;
+my $caught_up = $node_standby->poll_query_until(
+ 'postgres',
+ "SELECT '$target_lsn' <= replay_lsn AND state = 'streaming'
+ FROM pg_catalog.pg_stat_replication
+ WHERE application_name = 'old_primary'");
+$PostgreSQL::Test::Utils::timeout_default = $saved_timeout;
+ok($caught_up, 'new standby catches up after replaying PRUNE records');
+ok(!$node_primary->log_contains(qr/TRAP:.*BufferIsDirty/, $log_offset),
+ 'no assertion crash during replay');
+
+$node_primary->stop;
+
done_testing();
