From 9f69805bb6491bb911da0a104ddc88187c04be7d Mon Sep 17 00:00:00 2001 From: Melanie Plageman 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