On Wed, Sep 16, 2026 at 12:10 PM Melanie Plageman <[email protected]> wrote: > > There is an interaction between CREATE DATABASE STRATEGY WAL_LOG and > this fix that causes data corruption (identified while investigating > [1]). The scenario is you have a heap page set PD_ALL_VISIBLE and a > corresponding VM page set all-visible, you do a CREATE DATABASE > STRATEGY WAL_LOG using that database as a template. The primary has > the heap page set PDV (PD_ALL_VISIBLE) set and VM page AV set but the > standby does NOT have the page set all-visible in the VM (it does have > the heap page set PDV). After failover you have a primary with heap > page PDV set and VM page not set. You modify a tuple on the page. > Because the VM was already clear, we do not register the VM block in > the WAL record. The standby replays this and clears the heap page > PD_ALL_VISIBLE but leaves the VM set. This is data corruption leading > to wrong results from index scans and worse if that standby is later > promoted back to primary. > > We didn't have this problem before ed62d26caca because it cleared the > VM during replay as long as PD_ALL_VISIBLE needed clearing. I mention > in the thread [2] that I think we have to fix > RelationCopyStorageUsingBuffer() to avoid producing this scenario > because tuple locking already caused data corruption before > ed62d26caca. However, for update/delete, etc, I'm wondering if we > should harden this a bit and go back to registering a vm clear > operation whenever PD_ALL_VISIBLE was set on the heap page. If this is > a no-op on the primary for the VM, we register it with > REBGUF_NO_CHANGE. Vacuum works very hard to never end up with > PD_ALL_VISIBLE cleared and the VM set. Perhaps we should do everything > we can to avoid this happening with page modifications as well.
We can't WAL-log all pages that might possibly be out-of-sync on the standby even if we didn't need to modify them on the primary. And the above approach would kind of set that precedent. As an alternative, the attached patch makes sure the VM is clear on the standby even when the VM block was not registered and emits a warning. It isn't resilient to torn pages since we won't have emitted an FPI on the primary, however, since it is making a best attempt to repair corruption, I think that's fine. This would only need to be backpatched to 19. - Melanie
From 9f69805bb6491bb911da0a104ddc88187c04be7d Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Mon, 21 Sep 2026 16:01:22 -0400 Subject: [PATCH] Clear divergent visibility map bits during heap redo When replaying a heap record that clears PD_ALL_VISIBLE but registered no VM buffer (its clear was a no-op on the origin), clear the VM bit anyway. The VM can diverge across a cluster, e.g. via CREATE DATABASE STRATEGY WAL_LOG, and a standby must not keep a set VM bit over a page whose PD_ALL_VISIBLE is clear. You can still get torn pages because you didn't log an FPI on the primary, however it's a corruption repair scenario, so that's fine. Only backpatch to 19 because <= 18 already have a fallback. --- src/backend/access/heap/heapam_xlog.c | 89 +++++++++++++++++++++++++ src/backend/access/heap/visibilitymap.c | 33 +++++++++ src/include/access/visibilitymap.h | 3 + 3 files changed, 125 insertions(+) diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c index 5fa1de09cfb..0b998fb5b5b 100644 --- a/src/backend/access/heap/heapam_xlog.c +++ b/src/backend/access/heap/heapam_xlog.c @@ -19,9 +19,56 @@ #include "access/visibilitymap.h" #include "access/xlog.h" #include "access/xlogutils.h" +#include "common/relpath.h" #include "storage/freespace.h" #include "storage/standby.h" +/* + * Clear visibility map bits for a heap block whose PD_ALL_VISIBLE flag the + * record being replayed cleared, but whose VM page the record did not + * register. + * + * The VM page is only registered if clearing the bits changed it, so the bits + * were already clear when the record was generated. They should be clear here + * too, since the VM is replicated through WAL. But the VM can be out of sync + * across a cluster (for instance, CREATE DATABASE STRATEGY WAL_LOG + * historically could produce out-of-sync VMs). It's incorrect for + * PD_ALL_VISIBLE to be clear and the VM to be set, so we must fix it. + * + * This is not fully resilient: the VM page is modified without a full-page + * image, so a torn write during a crash could leave it inconsistent until the + * page is next repaired. That is considered acceptable since this is an edge + * case in which we already have data corruption. + */ +static void +heap_xlog_vm_clear_unregistered(RelFileLocator rlocator, + BlockNumber heap_blkno, + uint8 flags, XLogRecPtr lsn) +{ + Buffer vmbuffer = InvalidBuffer; + + /* + * Read the VM page without extending the fork; a page that does not exist + * has no bits to clear. If the bits are already clear (the consistent + * case) there is nothing to do. + */ + if (xlog_visibilitymap_get_status(rlocator, heap_blkno, &vmbuffer) & flags) + { + LockBuffer(vmbuffer, BUFFER_LOCK_EXCLUSIVE); + if (visibilitymap_clear(rlocator, heap_blkno, vmbuffer, flags)) + { + PageSetLSN(BufferGetPage(vmbuffer), lsn); + ereport(WARNING, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("clearing out-of-sync visibility map bits for page %u of relation %s", + heap_blkno, relpathperm(rlocator, MAIN_FORKNUM).str))); + } + UnlockReleaseBuffer(vmbuffer); + } + else if (BufferIsValid(vmbuffer)) + ReleaseBuffer(vmbuffer); +} + /* * Clear visibility map bits for a single heap block during heap redo. * @@ -46,7 +93,11 @@ heap_xlog_vm_clear(XLogReaderState *record, Buffer vmbuffer = InvalidBuffer; if (!XLogRecHasBlockRef(record, wal_vm_block_id)) + { + heap_xlog_vm_clear_unregistered(target_locator, heap_blkno, flags, + lsn); return; + } /* * If the vmbuffer was registered, use the recovery-specific routines to @@ -764,6 +815,7 @@ heap_xlog_update(XLogReaderState *record, bool hot_update) npage; bool has_vm_old, has_vm_new; + bool old_vm_cleared_via_new = false; OffsetNumber offnum; ItemId lp; HeapTupleData oldtup; @@ -842,8 +894,29 @@ heap_xlog_update(XLogReaderState *record, bool hot_update) PageSetLSN(BufferGetPage(vmbuffer_new), lsn); } if (BufferIsValid(vmbuffer_new)) + { + /* + * Remember whether oldblk's VM bits live on this page. If so, + * they are now up to date, whether we cleared them above or the + * page had already been replayed past this record. + */ + old_vm_cleared_via_new = + (xlrec->flags & XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED) && + visibilitymap_pin_ok(oldblk, vmbuffer_new); UnlockReleaseBuffer(vmbuffer_new); + } } + else if (xlrec->flags & XLH_UPDATE_NEW_ALL_VISIBLE_CLEARED) + { + /* + * If PD_ALL_VISIBLE was cleared on the new heap page and its + * corresponding VM page was not registered, ensure it is already + * clear or clear it. + */ + heap_xlog_vm_clear_unregistered(rlocator, newblk, + VISIBILITYMAP_VALID_BITS, lsn); + } + if (has_vm_old) { Buffer vmbuffer_old = InvalidBuffer; @@ -860,6 +933,22 @@ heap_xlog_update(XLogReaderState *record, bool hot_update) if (BufferIsValid(vmbuffer_old)) UnlockReleaseBuffer(vmbuffer_old); } + else if ((xlrec->flags & XLH_UPDATE_OLD_ALL_VISIBLE_CLEARED) && + !old_vm_cleared_via_new) + { + /* + * VM_OLD is omitted when clearing the old page's VM bit was a no-op + * when the record was generated, because the bit was already clear. + * We get here in two such cases: either no VM_NEW was registered, or + * VM_NEW was registered but covers a different VM page than oldblk + * (the old and new heap pages map to different VM pages). + * + * If the VM has diverged on the standby, the bit may be set here, so + * clear it defensively. + */ + heap_xlog_vm_clear_unregistered(rlocator, oldblk, + VISIBILITYMAP_VALID_BITS, lsn); + } /* * In normal operation, it is important to lock the two pages in diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index fe5ce437e1b..6136dbab4b0 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -12,6 +12,7 @@ * * INTERFACE ROUTINES * visibilitymap_clear - clear bits for one page in the visibility map + * xlog_visibilitymap_get_status - get status of bits during WAL replay * visibilitymap_pin - pin a map page for setting a bit * visibilitymap_pin_ok - check whether correct map page is already pinned * visibilitymap_set - set bit(s) in a previously pinned page @@ -189,6 +190,38 @@ visibilitymap_clear(RelFileLocator rlocator, BlockNumber heapBlk, return cleared; } +/* + * Like visibilitymap_get_status(), but uses a RelFileLocator instead of a + * Relation, so it needs no relcache entry and can be used by redo routines. + * It never extends the fork: if the map page does not exist, there are no + * bits, so *vmbuf is set to InvalidBuffer and 0 is returned. + * + * On return *vmbuf holds the pinned (but unlocked) map page; the caller is + * responsible for releasing it. A caller that goes on to clear bits must lock + * it first. + */ +uint8 +xlog_visibilitymap_get_status(RelFileLocator rlocator, BlockNumber heapBlk, + Buffer *vmbuf) +{ + BlockNumber mapBlock = HEAPBLK_TO_MAPBLOCK(heapBlk); + uint32 mapByte = HEAPBLK_TO_MAPBYTE(heapBlk); + uint8 mapOffset = HEAPBLK_TO_OFFSET(heapBlk); + char *map; + + Assert(InRecovery); + + *vmbuf = XLogReadBufferExtended(rlocator, VISIBILITYMAP_FORKNUM, mapBlock, + RBM_NORMAL_NO_LOG, InvalidBuffer); + if (!BufferIsValid(*vmbuf)) + return 0; + + map = PageGetContents(BufferGetPage(*vmbuf)); + + /* A single byte read is atomic (see visibilitymap_get_status()). */ + return ((map[mapByte] >> mapOffset) & VISIBILITYMAP_VALID_BITS); +} + /* * visibilitymap_pin - pin a map page for setting a bit * diff --git a/src/include/access/visibilitymap.h b/src/include/access/visibilitymap.h index 165efd1c00e..8ed1e86778f 100644 --- a/src/include/access/visibilitymap.h +++ b/src/include/access/visibilitymap.h @@ -15,6 +15,7 @@ #define VISIBILITYMAP_H #include "access/visibilitymapdefs.h" +#include "access/xlogdefs.h" #include "storage/block.h" #include "storage/buf.h" #include "storage/relfilelocator.h" @@ -28,6 +29,8 @@ extern bool visibilitymap_clear(RelFileLocator rlocator, BlockNumber heapBlk, Buffer vmbuf, uint8 flags); +extern uint8 xlog_visibilitymap_get_status(RelFileLocator rlocator, + BlockNumber heapBlk, Buffer *vmbuf); extern void visibilitymap_pin(Relation rel, BlockNumber heapBlk, Buffer *vmbuf); extern bool visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf); -- 2.43.0
