v15-0011-Remove-heap-buffer-from-XLOG_HEAP2_VISIBLE-WAL-c.patch

From f14db744ecb79b121d8d0d3489384a93bd6abf07 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 16 Sep 2025 11:05:30 -0400
Subject: [PATCH v15 11/23] Remove heap buffer from XLOG_HEAP2_VISIBLE WAL
 chain

Now that all users of visibilitymap_set() include setting PD_ALL_VISIBLE
in the WAL record capturing other changes to the heap page, we no longer
need to include the heap buffer in the WAL chain for setting the VM.
---
 src/backend/access/heap/heapam.c        | 16 +-----
 src/backend/access/heap/heapam_xlog.c   | 76 +++----------------------
 src/backend/access/heap/vacuumlazy.c    |  6 +-
 src/backend/access/heap/visibilitymap.c | 31 +---------
 src/include/access/heapam_xlog.h        |  3 +-
 src/include/access/visibilitymap.h      |  2 +-
 6 files changed, 16 insertions(+), 118 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7f354caec31..d4d83a6f9fe 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8806,21 +8806,14 @@ bottomup_sort_and_shrink(TM_IndexDeleteOp *delstate)
  *
  * snapshotConflictHorizon comes from the largest xmin on the page being
  * marked all-visible.  REDO routine uses it to generate recovery conflicts.
- *
- * If checksums or wal_log_hints are enabled, we may also generate a full-page
- * image of heap_buffer. Otherwise, we optimize away the FPI (by specifying
- * REGBUF_NO_IMAGE for the heap buffer), in which case the caller should *not*
- * update the heap page's LSN.
  */
 XLogRecPtr
-log_heap_visible(Relation rel, Buffer heap_buffer, Buffer vm_buffer,
+log_heap_visible(Relation rel, Buffer vm_buffer,
 				 TransactionId snapshotConflictHorizon, uint8 vmflags)
 {
 	xl_heap_visible xlrec;
 	XLogRecPtr	recptr;
-	uint8		flags;
 
-	Assert(BufferIsValid(heap_buffer));
 	Assert(BufferIsValid(vm_buffer));
 
 	xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
@@ -8829,14 +8822,7 @@ log_heap_visible(Relation rel, Buffer heap_buffer, Buffer vm_buffer,
 		xlrec.flags |= VISIBILITYMAP_XLOG_CATALOG_REL;
 	XLogBeginInsert();
 	XLogRegisterData(&xlrec, SizeOfHeapVisible);
-
 	XLogRegisterBuffer(0, vm_buffer, 0);
-
-	flags = REGBUF_STANDARD;
-	if (!XLogHintBitIsNeeded())
-		flags |= REGBUF_NO_IMAGE;
-	XLogRegisterBuffer(1, heap_buffer, flags);
-
 	recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_VISIBLE);
 
 	return recptr;
diff --git a/src/backend/access/heap/heapam_xlog.c b/src/backend/access/heap/heapam_xlog.c
index 4ea1a186c98..2e9fda0a9bf 100644
--- a/src/backend/access/heap/heapam_xlog.c
+++ b/src/backend/access/heap/heapam_xlog.c
@@ -229,15 +229,12 @@ heap_xlog_visible(XLogReaderState *record)
 	XLogRecPtr	lsn = record->EndRecPtr;
 	xl_heap_visible *xlrec = (xl_heap_visible *) XLogRecGetData(record);
 	Buffer		vmbuffer = InvalidBuffer;
-	Buffer		buffer;
-	Page		page;
 	RelFileLocator rlocator;
 	BlockNumber blkno;
-	XLogRedoAction action;
 
 	Assert((xlrec->flags & VISIBILITYMAP_XLOG_VALID_BITS) == xlrec->flags);
 
-	XLogRecGetBlockTag(record, 1, &rlocator, NULL, &blkno);
+	XLogRecGetBlockTag(record, 0, &rlocator, NULL, &blkno);
 
 	/*
 	 * If there are any Hot Standby transactions running that have an xmin
@@ -254,70 +251,11 @@ heap_xlog_visible(XLogReaderState *record)
 											rlocator);
 
 	/*
-	 * Read the heap page, if it still exists. If the heap file has dropped or
-	 * truncated later in recovery, we don't need to update the page, but we'd
-	 * better still update the visibility map.
-	 */
-	action = XLogReadBufferForRedo(record, 1, &buffer);
-	if (action == BLK_NEEDS_REDO)
-	{
-		/*
-		 * We don't bump the LSN of the heap page when setting the visibility
-		 * map bit (unless checksums or wal_hint_bits is enabled, in which
-		 * case we must). This exposes us to torn page hazards, but since
-		 * we're not inspecting the existing page contents in any way, we
-		 * don't care.
-		 */
-		page = BufferGetPage(buffer);
-
-		PageSetAllVisible(page);
-
-		if (XLogHintBitIsNeeded())
-			PageSetLSN(page, lsn);
-
-		MarkBufferDirty(buffer);
-	}
-	else if (action == BLK_RESTORED)
-	{
-		/*
-		 * If heap block was backed up, we already restored it and there's
-		 * nothing more to do. (This can only happen with checksums or
-		 * wal_log_hints enabled.)
-		 */
-	}
-
-	if (BufferIsValid(buffer))
-	{
-		Size		space = PageGetFreeSpace(BufferGetPage(buffer));
-
-		UnlockReleaseBuffer(buffer);
-
-		/*
-		 * Since FSM is not WAL-logged and only updated heuristically, it
-		 * easily becomes stale in standbys.  If the standby is later promoted
-		 * and runs VACUUM, it will skip updating individual free space
-		 * figures for pages that became all-visible (or all-frozen, depending
-		 * on the vacuum mode,) which is troublesome when FreeSpaceMapVacuum
-		 * propagates too optimistic free space values to upper FSM layers;
-		 * later inserters try to use such pages only to find out that they
-		 * are unusable.  This can cause long stalls when there are many such
-		 * pages.
-		 *
-		 * Forestall those problems by updating FSM's idea about a page that
-		 * is becoming all-visible or all-frozen.
-		 *
-		 * Do this regardless of a full-page image being applied, since the
-		 * FSM data is not in the page anyway.
-		 */
-		if (xlrec->flags & VISIBILITYMAP_VALID_BITS)
-			XLogRecordPageWithFreeSpace(rlocator, blkno, space);
-	}
-
-	/*
-	 * Even if we skipped the heap page update due to the LSN interlock, it's
-	 * still safe to update the visibility map.  Any WAL record that clears
-	 * the visibility map bit does so before checking the page LSN, so any
-	 * bits that need to be cleared will still be cleared.
+	 * Even if the heap relation was dropped or truncated and the previously
+	 * emitted record skipped the heap page update due to this LSN interlock,
+	 * it's still safe to update the visibility map.  Any WAL record that
+	 * clears the visibility map bit does so before checking the page LSN, so
+	 * any bits that need to be cleared will still be cleared.
 	 */
 	if (XLogReadBufferForRedoExtended(record, 0, RBM_ZERO_ON_ERROR, false,
 									  &vmbuffer) == BLK_NEEDS_REDO)
@@ -341,7 +279,7 @@ heap_xlog_visible(XLogReaderState *record)
 
 		reln = CreateFakeRelcacheEntry(rlocator);
 
-		visibilitymap_set(reln, blkno, InvalidBuffer, lsn, vmbuffer,
+		visibilitymap_set(reln, blkno, lsn, vmbuffer,
 						  xlrec->snapshotConflictHorizon, vmbits);
 
 		ReleaseBuffer(vmbuffer);
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index c016f8f7c25..735f1e7501e 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1911,7 +1911,7 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
 											  NULL, 0);
 			}
 
-			visibilitymap_set(vacrel->rel, blkno, buf,
+			visibilitymap_set(vacrel->rel, blkno,
 							  InvalidXLogRecPtr,
 							  vmbuffer, InvalidTransactionId,
 							  VISIBILITYMAP_ALL_VISIBLE |
@@ -2100,7 +2100,7 @@ lazy_scan_prune(LVRelState *vacrel,
 			flags |= VISIBILITYMAP_ALL_FROZEN;
 		}
 
-		old_vmbits = visibilitymap_set(vacrel->rel, blkno, buf,
+		old_vmbits = visibilitymap_set(vacrel->rel, blkno,
 									   InvalidXLogRecPtr,
 									   vmbuffer, presult.vm_conflict_horizon,
 									   flags);
@@ -2898,7 +2898,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
 	 */
 	if ((vmflags & VISIBILITYMAP_ALL_VISIBLE) != 0)
 	{
-		visibilitymap_set(vacrel->rel, blkno, buffer,
+		visibilitymap_set(vacrel->rel, blkno,
 						  InvalidXLogRecPtr,
 						  vmbuffer, visibility_cutoff_xid,
 						  vmflags);
diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index b28460392b7..33541e36674 100644
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -233,9 +233,7 @@ visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf)
  * when a page that is already all-visible is being marked all-frozen.
  *
  * Caller is expected to set the heap page's PD_ALL_VISIBLE bit before calling
- * this function. Except in recovery, caller should also pass the heap
- * buffer. When checksums are enabled and we're not in recovery, we must add
- * the heap buffer to the WAL chain to protect it from being torn.
+ * this function.
  *
  * You must pass a buffer containing the correct map page to this function.
  * Call visibilitymap_pin first to pin the right one. This function doesn't do
@@ -244,7 +242,7 @@ visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf)
  * Returns the state of the page's VM bits before setting flags.
  */
 uint8
-visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
+visibilitymap_set(Relation rel, BlockNumber heapBlk,
 				  XLogRecPtr recptr, Buffer vmBuf, TransactionId cutoff_xid,
 				  uint8 flags)
 {
@@ -261,18 +259,11 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
 #endif
 
 	Assert(InRecovery || XLogRecPtrIsInvalid(recptr));
-	Assert(InRecovery || PageIsAllVisible(BufferGetPage(heapBuf)));
 	Assert((flags & VISIBILITYMAP_VALID_BITS) == flags);
 
 	/* Must never set all_frozen bit without also setting all_visible bit */
 	Assert(flags != VISIBILITYMAP_ALL_FROZEN);
 
-	/* Check that we have the right heap page pinned, if present */
-	if (BufferIsValid(heapBuf) && BufferGetBlockNumber(heapBuf) != heapBlk)
-		elog(ERROR, "wrong heap buffer passed to visibilitymap_set");
-
-	Assert(!BufferIsValid(heapBuf) || BufferIsExclusiveLocked(heapBuf));
-
 	/* Check that we have the right VM page pinned */
 	if (!BufferIsValid(vmBuf) || BufferGetBlockNumber(vmBuf) != mapBlock)
 		elog(ERROR, "wrong VM buffer passed to visibilitymap_set");
@@ -294,23 +285,7 @@ visibilitymap_set(Relation rel, BlockNumber heapBlk, Buffer heapBuf,
 			if (XLogRecPtrIsInvalid(recptr))
 			{
 				Assert(!InRecovery);
-				recptr = log_heap_visible(rel, heapBuf, vmBuf, cutoff_xid, flags);
-
-				/*
-				 * If data checksums are enabled (or wal_log_hints=on), we
-				 * need to protect the heap page from being torn.
-				 *
-				 * If not, then we must *not* update the heap page's LSN. In
-				 * this case, the FPI for the heap page was omitted from the
-				 * WAL record inserted above, so it would be incorrect to
-				 * update the heap page's LSN.
-				 */
-				if (XLogHintBitIsNeeded())
-				{
-					Page		heapPage = BufferGetPage(heapBuf);
-
-					PageSetLSN(heapPage, recptr);
-				}
+				recptr = log_heap_visible(rel, vmBuf, cutoff_xid, flags);
 			}
 			PageSetLSN(page, recptr);
 		}
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 7d3fb75dda7..82b8f7f2bbc 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -440,7 +440,6 @@ typedef struct xl_heap_inplace
  * This is what we need to know about setting a visibility map bit
  *
  * Backup blk 0: visibility map buffer
- * Backup blk 1: heap buffer
  */
 typedef struct xl_heap_visible
 {
@@ -493,7 +492,7 @@ extern void heap2_desc(StringInfo buf, XLogReaderState *record);
 extern const char *heap2_identify(uint8 info);
 extern void heap_xlog_logical_rewrite(XLogReaderState *r);
 
-extern XLogRecPtr log_heap_visible(Relation rel, Buffer heap_buffer,
+extern XLogRecPtr log_heap_visible(Relation rel,
 								   Buffer vm_buffer,
 								   TransactionId snapshotConflictHorizon,
 								   uint8 vmflags);
diff --git a/src/include/access/visibilitymap.h b/src/include/access/visibilitymap.h
index 3dcf37ba03f..fbc69604d57 100644
--- a/src/include/access/visibilitymap.h
+++ b/src/include/access/visibilitymap.h
@@ -32,7 +32,7 @@ extern void visibilitymap_pin(Relation rel, BlockNumber heapBlk,
 							  Buffer *vmbuf);
 extern bool visibilitymap_pin_ok(BlockNumber heapBlk, Buffer vmbuf);
 extern uint8 visibilitymap_set(Relation rel,
-							   BlockNumber heapBlk, Buffer heapBuf,
+							   BlockNumber heapBlk,
 							   XLogRecPtr recptr,
 							   Buffer vmBuf,
 							   TransactionId cutoff_xid,
-- 
2.43.0