On Fri, Nov 6, 2026 at 10:13 AM Robert Haas <[email protected]> wrote: > > On Mon, Oct 12, 2026 at 4:07 PM Melanie Plageman > <[email protected]> wrote: > > 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. > > The problem with this approach is that it also includes the hole data > when the page is standard but the space between pd_upper and pd_lower > contains leftover garbage rather than zero bytes. That case is common: > when we prune-and-defragment heap pages, we don't zero the hole. I > think it would be worth trying to pass down the AM OID and always use > page_std = true for main-fork heap and btree pages (and maybe other > in-core AMs with no known legacy problems). That would mitigate the > practical downsides of the patch considerably. Whether you do that or > not, it's probably worth mentioning the performance concern in the > commit message. With that change, the downside would only exist for > out-of-core AMs, which do exist, and they could consider vigorous > hole-zeroing as a workaround.
I was aware that prune/defrag doesn't zero the data and so that would not be included. I was mostly concerned that singling out heap tables and btree felt a little inexact. But I suppose since it's a pretty trivial diff to thread that information up from ScanSourceDatabasePgClassTuple() to RelationCopyStorageUsingBuffer(), it's worth doing. Attached v2 uses a hybrid approach that passes page_std for heap tables and btree indexes as well as when the data between pd_upper and pd_lower is all zeroes. That allows other AMs to zero out that area if they want to preserve the old behavior and avoid a regression (which I think was what you were suggesting). I also updated the test to use pg_visibilitymap_summary() instead of comparing the raw page, which should fix test stability concerns. The second commit in the patch set is targeting master only and would extend this approach to RelationCopyStorage(), which would reduce the WAL volume produced and works even more naturally there because they already know the relation AM and page layout. - Melanie
From 51e932eae4582c88783b0e66bc950599fb35ea9c Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Tue, 10 Nov 2026 14:47:58 -0500 Subject: [PATCH v2 1/2] Fix WAL-logged CREATE DATABASE with nonstandard pages 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 remove the apparent free-space region from WAL when copying a heap table or B-tree index, whose pages are known to use the standard layout. For other relation types, remove the region only when it contains all zeroes, ensuring that replay reconstructs the page without data loss. Use a shared bounds helper so that RelationCopyStorageUsingBuffer() checks the same region that XLogRecordAssemble() omits. This may increase WAL volume for access methods other than heap and B-tree when the space between pd_lower and pd_upper contains stale data that was previously omitted. Such access methods can preserve hole removal by zeroing that space. Reported-by: Matthias van de Meent <[email protected]> Reported-by: Rogers Wang <[email protected]> Reviewed-by: Robert Haas <[email protected]> Reviewed-by: surya poondla <[email protected]> Author: Melanie Plageman <[email protected]> Discussion: https://postgr.es/m/flat/CAEze2WgNjODiAiTU18E06_BrRK0nqwJ1kdS_bN61c2_DpFDSnQ%40mail.gmail.com Backpatch-through: 15 --- src/backend/access/transam/xloginsert.c | 13 +++---- src/backend/commands/dbcommands.c | 18 ++++++++- src/backend/storage/buffer/bufmgr.c | 37 +++++++++++++++---- src/include/storage/bufmgr.h | 3 +- src/include/storage/bufpage.h | 14 +++++++ src/test/recovery/Makefile | 1 + src/test/recovery/t/034_create_database.pl | 43 +++++++++++++++++++--- 7 files changed, 106 insertions(+), 23 deletions(-) diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 341deb7b80d..251688e37fd 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -740,16 +740,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/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 7e3fc59eafd..f514579e8fd 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -36,6 +36,7 @@ #include "catalog/dependency.h" #include "catalog/indexing.h" #include "catalog/objectaccess.h" +#include "catalog/pg_am_d.h" #include "catalog/pg_authid.h" #include "catalog/pg_collation.h" #include "catalog/pg_database.h" @@ -110,6 +111,7 @@ typedef struct CreateDBRelInfo RelFileLocator rlocator; /* physical relation identifier */ Oid reloid; /* relation oid */ bool permanent; /* relation is permanent or unlogged */ + bool mainfork_page_std; /* main fork has standard page layout */ } CreateDBRelInfo; @@ -217,7 +219,8 @@ CreateDatabaseUsingWalLog(Oid src_dboid, Oid dst_dboid, LockRelationId(&dstrelid, AccessShareLock); /* Copy relation storage from source to the destination. */ - CreateAndCopyRelationData(srcrlocator, dstrlocator, relinfo->permanent); + CreateAndCopyRelationData(srcrlocator, dstrlocator, relinfo->permanent, + relinfo->mainfork_page_std); /* Release the relation locks. */ UnlockRelationId(&srcrelid, AccessShareLock); @@ -449,6 +452,19 @@ ScanSourceDatabasePgClassTuple(HeapTupleData *tuple, Oid tbid, Oid dbid, relinfo->permanent = (classForm->relpersistence == RELPERSISTENCE_PERMANENT) ? true : false; + /* + * Heap tables and B-tree indexes are common cases whose main forks are + * known to use the standard page layout. Other access methods may do so + * as well, but some, such as GIN, have legacy page formats that differ + * from their current format. Keep the classification conservative by + * limiting it to heap and B-tree. + */ + relinfo->mainfork_page_std = + (RELKIND_HAS_TABLE_AM(classForm->relkind) && + classForm->relam == HEAP_TABLE_AM_OID) || + (classForm->relkind == RELKIND_INDEX && + classForm->relam == BTREE_AM_OID); + return relinfo; } diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index c17fc3590f0..4dbc25c6c50 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" @@ -666,7 +667,8 @@ static void FindAndDropRelationBuffers(RelFileLocator rlocator, BlockNumber firstDelBlock); static void RelationCopyStorageUsingBuffer(RelFileLocator srclocator, RelFileLocator dstlocator, - ForkNumber forkNum, bool permanent); + ForkNumber forkNum, bool permanent, + bool page_std); static void AtProcExit_Buffers(int code, Datum arg); static void CheckForBufferLeaks(void); #ifdef USE_ASSERT_CHECKING @@ -5376,7 +5378,8 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels) static void RelationCopyStorageUsingBuffer(RelFileLocator srclocator, RelFileLocator dstlocator, - ForkNumber forkNum, bool permanent) + ForkNumber forkNum, bool permanent, + bool page_std) { Buffer srcBuf; Buffer dstBuf; @@ -5462,7 +5465,24 @@ RelationCopyStorageUsingBuffer(RelFileLocator srclocator, /* WAL-log the copied page. */ if (use_wal) - log_newpage_buffer(dstBuf, true); + { + bool allow_page_std = page_std; + + /* + * For page layouts we don't recognize, hole removal is still safe + * if the exact range that WAL would omit contains only zeroes. + */ + if (!allow_page_std && 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(); @@ -5484,12 +5504,15 @@ RelationCopyStorageUsingBuffer(RelFileLocator srclocator, * * Pass permanent as true for permanent relations and false for * unlogged relations. Currently this API is not supported for - * temporary relations. + * temporary relations. mainfork_page_std indicates whether the main + * fork uses the standard page layout; all other forks are treated as + * non-standard. * -------------------------------------------------------------------- */ void CreateAndCopyRelationData(RelFileLocator src_rlocator, - RelFileLocator dst_rlocator, bool permanent) + RelFileLocator dst_rlocator, bool permanent, + bool mainfork_page_std) { char relpersistence; SMgrRelation src_rel; @@ -5512,7 +5535,7 @@ CreateAndCopyRelationData(RelFileLocator src_rlocator, /* copy main fork. */ RelationCopyStorageUsingBuffer(src_rlocator, dst_rlocator, MAIN_FORKNUM, - permanent); + permanent, mainfork_page_std); /* copy those extra forks that exist */ for (ForkNumber forkNum = MAIN_FORKNUM + 1; @@ -5531,7 +5554,7 @@ CreateAndCopyRelationData(RelFileLocator src_rlocator, /* Copy a fork's data, block by block. */ RelationCopyStorageUsingBuffer(src_rlocator, dst_rlocator, forkNum, - permanent); + permanent, false); } } } diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index 6837b35fc6d..b8e9b7f1093 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -297,7 +297,8 @@ extern void FlushRelationBuffers(Relation rel); extern void FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels); extern void CreateAndCopyRelationData(RelFileLocator src_rlocator, RelFileLocator dst_rlocator, - bool permanent); + bool permanent, + bool mainfork_page_std); extern void FlushDatabaseBuffers(Oid dbid); extern void DropRelationBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum, diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h index 713b6222286..2fe9085f881 100644 --- a/src/include/storage/bufpage.h +++ b/src/include/storage/bufpage.h @@ -241,6 +241,20 @@ 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 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..e90e0b1ad4c 100644 --- a/src/test/recovery/Makefile +++ b/src/test/recovery/Makefile @@ -11,6 +11,7 @@ EXTRA_INSTALL=contrib/pg_prewarm \ contrib/pg_stat_statements \ + contrib/pg_visibility \ contrib/test_decoding \ src/test/modules/injection_points \ src/test/modules/test_wait_lsn diff --git a/src/test/recovery/t/034_create_database.pl b/src/test/recovery/t/034_create_database.pl index 194d1837e23..a5c1ff228ba 100644 --- a/src/test/recovery/t/034_create_database.pl +++ b/src/test/recovery/t/034_create_database.pl @@ -10,19 +10,34 @@ 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 with a populated visibility map in the template database. +$node->safe_psql($db_template, q{ + CREATE TABLE tab_db_before_create_1 AS + SELECT generate_series(1, 10000) AS a; + VACUUM (FREEZE) 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 pg_visibility;"); +# This table should persist in the template database but not be present in the +# copy made above. $node->safe_psql($db_template, "CREATE TABLE tab_db_after_create_1 (a INT);"); # Flush the changes affecting the template database, then replay them. @@ -31,15 +46,31 @@ $node->safe_psql("postgres", "CHECKPOINT;"); $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 created after it was copied. $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" + "check that table created after database copy is absent after crash" ); +$node->wait_for_replay_catchup($standby); + +my $visibility_map_query = q{ + SELECT blkno, all_visible, all_frozen + FROM pg_visibility_map('tab_db_before_create_1') + ORDER BY blkno; +}; +my $primary_visibility_map = + $node->safe_psql($db_new, $visibility_map_query); +my $standby_visibility_map = + $standby->safe_psql($db_new, $visibility_map_query); + +isnt($primary_visibility_map, '', 'copied table has visibility map entries'); +is($standby_visibility_map, $primary_visibility_map, + "copied table visibility map matches standby"); + done_testing(); -- 2.43.0
From 264803abd3aa616edb90b418d98280373aa3c71d Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Tue, 10 Nov 2026 14:47:58 -0500 Subject: [PATCH v2 2/2] Reduce WAL generated when copying relation storage RelationCopyStorage() included the region between pd_lower and pd_upper in full-page images, even when that region was known to be free space, because it had no information about the page layout. It conservatively treated every page as nonstandard, so this was not a correctness bug, but it could generate unnecessary WAL. A recent commit fixed RelationCopyStorageUsingBuffer(), which previously treated every page as standard and could therefore omit meaningful data between pd_lower and pd_upper. It now omits that region only for heap and B-tree main-fork pages, which are known to use the standard page layout, or when the region contains only zeroes. Apply the same policy to RelationCopyStorage(). This reduces WAL volume for known standard page layouts while preserving the full contents of unknown layouts unless omission is demonstrably lossless. --- src/backend/access/heap/heapam_handler.c | 4 ++-- src/backend/catalog/storage.c | 19 ++++++++++++++----- src/backend/commands/tablecmds.c | 6 ++++-- src/include/catalog/storage.h | 3 ++- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 05a81e4557a..ff2345b9233 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -555,7 +555,7 @@ heapam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator) /* copy main fork */ RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM, - rel->rd_rel->relpersistence); + rel->rd_rel->relpersistence, true); /* copy those extra forks that exist */ for (ForkNumber forkNum = MAIN_FORKNUM + 1; @@ -574,7 +574,7 @@ heapam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator) forkNum == INIT_FORKNUM)) log_smgrcreate(newrlocator, forkNum); RelationCopyStorage(RelationGetSmgr(rel), dstrel, forkNum, - rel->rd_rel->relpersistence); + rel->rd_rel->relpersistence, false); } } diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c index e443a4993c5..1a4397d15da 100644 --- a/src/backend/catalog/storage.c +++ b/src/backend/catalog/storage.c @@ -476,7 +476,7 @@ RelationPreTruncate(Relation rel) */ void RelationCopyStorage(SMgrRelation src, SMgrRelation dst, - ForkNumber forkNum, char relpersistence) + ForkNumber forkNum, char relpersistence, bool page_std) { bool use_wal; bool copying_initfork; @@ -508,6 +508,7 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst, for (blkno = 0; blkno < nblocks; blkno++) { BulkWriteBuffer buf; + bool allow_page_std = page_std; int piv_flags; bool checksum_failure; bool verified; @@ -551,11 +552,19 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst, } /* - * Queue the page for WAL-logging and writing out. Unfortunately we - * don't know what kind of a page this is, so we have to log the full - * page including any unused space. + * For page layouts we don't recognize, hole removal is still safe if + * the exact range that WAL would omit contains only zeroes. */ - smgr_bulk_write(bulkstate, blkno, buf, false); + if (!allow_page_std && PageFreeSpaceBoundsAreValid((Page) buf)) + { + PageHeader phdr = (PageHeader) buf; + + allow_page_std = + pg_memory_is_all_zeros((char *) buf + phdr->pd_lower, + phdr->pd_upper - phdr->pd_lower); + } + + smgr_bulk_write(bulkstate, blkno, buf, allow_page_std); } smgr_bulk_finish(bulkstate); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index aa89c9a4637..cbd2bf0b2b7 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -36,6 +36,7 @@ #include "catalog/objectaccess.h" #include "catalog/partition.h" #include "catalog/pg_am.h" +#include "catalog/pg_am_d.h" #include "catalog/pg_attrdef.h" #include "catalog/pg_cast.h" #include "catalog/pg_collation.h" @@ -18527,7 +18528,8 @@ index_copy_data(Relation rel, RelFileLocator newrlocator) /* copy main fork */ RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM, - rel->rd_rel->relpersistence); + rel->rd_rel->relpersistence, + rel->rd_rel->relam == BTREE_AM_OID); /* copy those extra forks that exist */ for (ForkNumber forkNum = MAIN_FORKNUM + 1; @@ -18546,7 +18548,7 @@ index_copy_data(Relation rel, RelFileLocator newrlocator) forkNum == INIT_FORKNUM)) log_smgrcreate(&newrlocator, forkNum); RelationCopyStorage(RelationGetSmgr(rel), dstrel, forkNum, - rel->rd_rel->relpersistence); + rel->rd_rel->relpersistence, false); } } diff --git a/src/include/catalog/storage.h b/src/include/catalog/storage.h index 70f619a6d6f..68211e51c1e 100644 --- a/src/include/catalog/storage.h +++ b/src/include/catalog/storage.h @@ -30,7 +30,8 @@ extern void RelationPreserveStorage(RelFileLocator rlocator, bool atCommit); extern void RelationPreTruncate(Relation rel); extern void RelationTruncate(Relation rel, BlockNumber nblocks); extern void RelationCopyStorage(SMgrRelation src, SMgrRelation dst, - ForkNumber forkNum, char relpersistence); + ForkNumber forkNum, char relpersistence, + bool page_std); extern bool RelFileLocatorSkippingWAL(RelFileLocator rlocator); extern Size EstimatePendingSyncsSpace(void); extern void SerializePendingSyncs(Size maxSize, char *startAddress); -- 2.43.0
