On Tue, Sep 22, 2026 at 7:12 PM Melanie Plageman <[email protected]> wrote: > > Andres suggested off-list having CreateDatabaseUsingWalLog() thread > information from the catalog about whether the page is a normal heap > page down into RelationCopyStorageUsingBuffer().
Upon further off-list discussion, instead of checking the AM, we can just check if the space between pd_upper and pd_lower is all zeroes. If it is, there is no harm in omitting it. It should always work, allows the optimization in more cases, and requires less code. Attached patch does that. I also added a test. It compares the pages of all forks of a table between a primary and standby after doing CREATE DATABASE USING STRATEGY WAL_LOG. I compare the whole page byte-by-byte including page header and LSN. Are those always guaranteed to be the same on the standby as well? - Melanie
From 68303b6ad5bb88835b5f55c779b0d196f0c791d4 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Mon, 12 Oct 2026 15:14:02 -0400 Subject: [PATCH v1] Preserve nonstandard pages in WAL-logged database copies RelationCopyStorageUsingBuffer() passed page_std=true when WAL-logging every copied relation fork. This allowed WAL insertion to omit the region between pd_lower and pd_upper from full-page images and restore it as zeroes during replay. That is safe only for pages using the standard page layout. On nonstandard pages, the apparent hole can contain meaningful data, causing the primary and standby to diverge. Known affected cases include free space maps, visibility maps, and old GIN indexes, though any access method using nonstandard pages could be affected. Losing VM contents can lead to data corruption after promotion. For example, a tuple lock on a promoted standby may find the all-frozen bit already clear and therefore emit no WAL to clear it. The old primary then replays the xmax change without clearing the all-frozen bit, retaining the stale bit. This can cause vacuum failures and make invisible data visible again. Only permit hole removal when the apparent free-space region is already all zeroes, making the omission lossless regardless of page layout. Use a shared bounds helper so RelationCopyStorageUsingBuffer() and XLogRecordAssemble() identify the region using the same conditions. Reported-by: Matthias van de Meent <[email protected]> Reported-by: Rogers Wang <[email protected]> Author: Melanie Plageman <[email protected]> Discussion: https://postgr.es/m/flat/202602021525.qv4m7oge4eca%40alvherre.pgsql#2dd58a8f7d87e15ef37199d5b8a05ac4 --- src/backend/access/transam/xloginsert.c | 13 +++---- src/backend/storage/buffer/bufmgr.c | 20 +++++++++-- src/include/storage/bufpage.h | 15 ++++++++ src/test/recovery/Makefile | 3 +- src/test/recovery/t/034_create_database.pl | 41 +++++++++++++++++++--- 5 files changed, 76 insertions(+), 16 deletions(-) diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 2195df85b10..8de52e3c653 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -730,16 +730,13 @@ XLogRecordAssemble(RmgrId rmid, uint8 info, */ if (regbuf->flags & REGBUF_STANDARD) { - /* Assume we can omit data between pd_lower and pd_upper */ - uint16 lower = ((const PageHeaderData *) page)->pd_lower; - uint16 upper = ((const PageHeaderData *) page)->pd_upper; + const PageHeaderData *phdr = (const PageHeaderData *) page; - if (lower >= SizeOfPageHeaderData && - upper > lower && - upper <= BLCKSZ) + /* Assume we can omit data between pd_lower and pd_upper */ + if (PageFreeSpaceBoundsAreValid(page)) { - bimg.hole_offset = lower; - cbimg.hole_length = upper - lower; + bimg.hole_offset = phdr->pd_lower; + cbimg.hole_length = phdr->pd_upper - phdr->pd_lower; } else { diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index f81c7732e68..e19e76deeb1 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -65,6 +65,7 @@ #include "storage/smgr.h" #include "storage/standby.h" #include "utils/memdebug.h" +#include "utils/memutils.h" #include "utils/ps_status.h" #include "utils/rel.h" #include "utils/resowner.h" @@ -5460,9 +5461,24 @@ RelationCopyStorageUsingBuffer(RelFileLocator srclocator, memcpy(dstPage, srcPage, BLCKSZ); MarkBufferDirty(dstBuf); - /* WAL-log the copied page. */ + /* + * WAL-log the copied page. It is safe to omit the apparent free space + * even if this is not a standard page, provided that the range + * contains only zeroes. + */ if (use_wal) - log_newpage_buffer(dstBuf, true); + { + bool allow_page_std = false; + + if (PageFreeSpaceBoundsAreValid(dstPage)) + { + PageHeader phdr = (PageHeader) dstPage; + + allow_page_std = pg_memory_is_all_zeros(dstPage + phdr->pd_lower, + phdr->pd_upper - phdr->pd_lower); + } + log_newpage_buffer(dstBuf, allow_page_std); + } END_CRIT_SECTION(); diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h index 713b6222286..593ad2ee9c6 100644 --- a/src/include/storage/bufpage.h +++ b/src/include/storage/bufpage.h @@ -241,6 +241,21 @@ typedef PageHeaderData *PageHeader; */ #define SizeOfPageHeaderData (offsetof(PageHeaderData, pd_linp)) +/* + * Returns true if the page header describes a non-empty, in-bounds free space + * range. This does not prove that the page actually uses the standard page + * layout. + */ +static inline bool +PageFreeSpaceBoundsAreValid(const PageData *page) +{ + const PageHeaderData *phdr = (const PageHeaderData *) page; + + return phdr->pd_lower >= SizeOfPageHeaderData && + phdr->pd_upper > phdr->pd_lower && + phdr->pd_upper <= BLCKSZ; +} + /* * PageIsEmpty * returns true iff no itemid has been allocated on the page 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..a14699743f4 100644 --- a/src/test/recovery/t/034_create_database.pl +++ b/src/test/recovery/t/034_create_database.pl @@ -10,36 +10,67 @@ use PostgreSQL::Test::Utils; use Test::More; my $node = PostgreSQL::Test::Cluster->new('node'); -$node->init; +$node->init(allows_streaming => 1); $node->start; +$node->backup('backup'); +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($node, 'backup', has_streaming => 1); +$standby->start; + # This checks that any DDLs run on the template database that modify pg_class # are persisted after creating a database from it using the WAL_LOG strategy, # as a direct copy of the template database's pg_class is used in this case. my $db_template = "template1"; my $db_new = "test_db_1"; -# Create table. It should persist on the template database. +# Create a table in the template database and add data then vacuum it, ensuring +# it will have both a VM and FSM. +$node->safe_psql($db_template, q{ +CREATE TABLE tab_db_before_create_1 AS + SELECT generate_series(1, 10000) AS a; +VACUUM (FREEZE, ANALYZE) tab_db_before_create_1; +}); + +# Copy the template database $node->safe_psql("postgres", "CREATE DATABASE $db_new STRATEGY WAL_LOG TEMPLATE $db_template;"); +$node->safe_psql($db_new, 'CREATE EXTENSION pageinspect;'); +# Create a table after copying it. This table should persist in the template +# database but not be in the newly created copy of it. $node->safe_psql($db_template, "CREATE TABLE tab_db_after_create_1 (a INT);"); # Flush the changes affecting the template database, then replay them. $node->safe_psql("postgres", "CHECKPOINT;"); +# Template database has the table we created after copying. $node->stop('immediate'); $node->start; my $result = $node->safe_psql($db_template, - "SELECT count(*) FROM pg_class WHERE relname LIKE 'tab_db_%';"); + "SELECT count(*) FROM pg_class WHERE relname = 'tab_db_after_create_1';"); is($result, "1", "check that table exists on template after crash, with checkpoint"); -# The new database should have no tables. +# The new database should not have the table we created after copying. $result = $node->safe_psql($db_new, - "SELECT count(*) FROM pg_class WHERE relname LIKE 'tab_db_%';"); + "SELECT count(*) FROM pg_class WHERE relname = 'tab_db_after_create_1';"); is($result, "0", "check that there are no tables from template on new database after crash" ); +$node->wait_for_replay_catchup($standby); + +# The new database should have the same data for all forks on the primary and +# standby for the table created before it was copied from the template. +for my $fork (qw(vm fsm)) +{ + my $query = + "SELECT encode(get_raw_page('tab_db_before_create_1', '$fork', 0), 'hex');"; + my $primary_page = $node->safe_psql($db_new, $query); + my $standby_page = $standby->safe_psql($db_new, $query); + + is($standby_page, $primary_page, "copied $fork page matches standby"); +} + done_testing(); -- 2.43.0
