On Mon, Aug 31, 2026 at 11:46 AM Melanie Plageman <[email protected]> wrote: > > This part > > > > 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. > > is true completely independently of my commit. Only the assert is new > and would require changes to v19. I didn't think of this way of ending > up with a discrepancy between primary and standby VM. I assumed that > if the VM bit was not set on the primary it would not be set on the > standby. Even if we commit something to eliminate this corner case > (with RelationCopyStorageUsingBuffer()), you could probably end up > with a lost or truncated VM on the primary and the VM being set on the > standby in some other way, so we shouldn't assume that the > visibilitymap_set() will dirty the buffer. We can't set the page LSN > unless we marked the buffer dirty, so for 19, I think I could just do > if (BufferIsDirty()) -> PageSetLSN() > In master, it probably makes sense to change back the signature of > visibilitymap_set() to report if it did actually set the bit(s) and > then use that to decide whether to set the page LSN. Unless we don't > think it is too late in 19 to change the visibilitymap_set() API. It > does mean all the other callers of visibilitymap_set() are going to > discard the return value, but maybe that's okay?
Since we are putting out a beta 4, I feel fine about changing the visiblitymap_set() API back. Here is a patch that does that. - Melanie
From 4781d34b85a55fca467ebe6ae46d11f790be5bb8 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Fri, 11 Sep 2026 16:33:56 -0400 Subject: [PATCH v1] Handle no-op visibility map setting during redo add323da40a consolidated visibility map updates into XLOG_HEAP2_PRUNE_FREEZE records. It assumed that a record containing a visibility map update would always modify the VM page during replay. That is not true when the requested bits are already set on the standby. Primary and standby visibility maps can diverge for several reasons. This issue was exposed by a CREATE DATABASE ... STRATEGY WAL_LOG bug. After a standby with an out-of-date VM is promoted, VACUUM may set bits that were already present on the former primary. When the former primary later replays that record as a standby, visibilitymap_set() is a no-op. Restore visibilitymap_set()'s former API, which returns the state of the VM bits before setting the requested flags. Use that result to set the VM page LSN only when the operation actually modifies the page. --- src/backend/access/heap/heapam.c | 10 +++++----- src/backend/access/heap/heapam_xlog.c | 18 ++++++------------ src/backend/access/heap/pruneheap.c | 5 +++-- src/backend/access/heap/vacuumlazy.c | 16 ++++++++-------- src/backend/access/heap/visibilitymap.c | 6 +++++- src/include/access/visibilitymap.h | 6 +++--- 6 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 10766d330a9..b9bf47fd798 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2484,11 +2484,11 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, { PageSetAllVisible(page); PageClearPrunable(page); - visibilitymap_set(BufferGetBlockNumber(buffer), - vmbuffer, - VISIBILITYMAP_ALL_VISIBLE | - VISIBILITYMAP_ALL_FROZEN, - relation->rd_locator); + (void) visibilitymap_set(BufferGetBlockNumber(buffer), + vmbuffer, + VISIBILITYMAP_ALL_VISIBLE | + VISIBILITYMAP_ALL_FROZEN, + relation->rd_locator); } /* diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c index 7a7bc7ea740..c66b30f4c58 100644 --- a/src/backend/access/heap/heapam_xlog.c +++ b/src/backend/access/heap/heapam_xlog.c @@ -283,10 +283,8 @@ heap_xlog_prune_freeze(XLogReaderState *record) if (PageIsNew(vmpage)) PageInit(vmpage, BLCKSZ, 0); - visibilitymap_set(blkno, vmbuffer, vmflags, rlocator); - - Assert(BufferIsDirty(vmbuffer)); - PageSetLSN(vmpage, lsn); + if (visibilitymap_set(blkno, vmbuffer, vmflags, rlocator) != vmflags) + PageSetLSN(vmpage, lsn); } if (BufferIsValid(vmbuffer)) @@ -692,19 +690,15 @@ heap_xlog_multi_insert(XLogReaderState *record) &vmbuffer) == BLK_NEEDS_REDO) { Page vmpage = BufferGetPage(vmbuffer); + uint8 vmflags = VISIBILITYMAP_ALL_VISIBLE | + VISIBILITYMAP_ALL_FROZEN; /* initialize the page if it was read as zeros */ if (PageIsNew(vmpage)) PageInit(vmpage, BLCKSZ, 0); - visibilitymap_set(blkno, - vmbuffer, - VISIBILITYMAP_ALL_VISIBLE | - VISIBILITYMAP_ALL_FROZEN, - rlocator); - - Assert(BufferIsDirty(vmbuffer)); - PageSetLSN(vmpage, lsn); + if (visibilitymap_set(blkno, vmbuffer, vmflags, rlocator) != vmflags) + PageSetLSN(vmpage, lsn); } if (BufferIsValid(vmbuffer)) diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c index 29f4722b02d..6f6253d7f66 100644 --- a/src/backend/access/heap/pruneheap.c +++ b/src/backend/access/heap/pruneheap.c @@ -1310,8 +1310,9 @@ heap_page_prune_and_freeze(PruneFreezeParams *params, */ PageSetAllVisible(prstate.page); PageClearPrunable(prstate.page); - visibilitymap_set(prstate.block, prstate.vmbuffer, prstate.new_vmbits, - prstate.relation->rd_locator); + (void) visibilitymap_set(prstate.block, prstate.vmbuffer, + prstate.new_vmbits, + prstate.relation->rd_locator); } MarkBufferDirty(prstate.buffer); diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 063ef2208de..8e1f660bc2f 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -1964,11 +1964,11 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno, PageSetAllVisible(page); PageClearPrunable(page); - visibilitymap_set(blkno, - vmbuffer, - VISIBILITYMAP_ALL_VISIBLE | - VISIBILITYMAP_ALL_FROZEN, - vacrel->rel->rd_locator); + (void) visibilitymap_set(blkno, + vmbuffer, + VISIBILITYMAP_ALL_VISIBLE | + VISIBILITYMAP_ALL_FROZEN, + vacrel->rel->rd_locator); /* * Emit WAL for setting PD_ALL_VISIBLE on the heap page and @@ -2846,9 +2846,9 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer, */ PageSetAllVisible(page); PageClearPrunable(page); - visibilitymap_set(blkno, - vmbuffer, vmflags, - vacrel->rel->rd_locator); + (void) visibilitymap_set(blkno, + vmbuffer, vmflags, + vacrel->rel->rd_locator); conflict_xid = newest_live_xid; } diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index 15bd5cf717c..fe5ce437e1b 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -255,8 +255,10 @@ visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf) * containing heapBlk. * * rlocator is used only for debugging messages. + * + * Returns the state of the page's VM bits before setting flags. */ -void +uint8 visibilitymap_set(BlockNumber heapBlk, Buffer vmBuf, uint8 flags, RelFileLocator rlocator) @@ -299,6 +301,8 @@ visibilitymap_set(BlockNumber heapBlk, map[mapByte] |= (flags << mapOffset); MarkBufferDirty(vmBuf); } + + return status; } /* diff --git a/src/include/access/visibilitymap.h b/src/include/access/visibilitymap.h index b860c4ef3ca..165efd1c00e 100644 --- a/src/include/access/visibilitymap.h +++ b/src/include/access/visibilitymap.h @@ -31,9 +31,9 @@ extern bool visibilitymap_clear(RelFileLocator rlocator, BlockNumber heapBlk, extern void visibilitymap_pin(Relation rel, BlockNumber heapBlk, Buffer *vmbuf); extern bool visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf); -extern void visibilitymap_set(BlockNumber heapBlk, - Buffer vmBuf, uint8 flags, - RelFileLocator rlocator); +extern uint8 visibilitymap_set(BlockNumber heapBlk, + Buffer vmBuf, uint8 flags, + RelFileLocator rlocator); extern uint8 visibilitymap_get_status(Relation rel, BlockNumber heapBlk, Buffer *vmbuf); extern void visibilitymap_count(Relation rel, BlockNumber *all_visible, BlockNumber *all_frozen); extern BlockNumber visibilitymap_prepare_truncate(Relation rel, -- 2.43.0
