v34-0003-Move-VM-assert-into-prune-freeze-code-and-simpli.patch

From e591bf061ee673f3750d1180673e1ab48be43bb8 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 27 Jan 2026 16:53:11 -0500
Subject: [PATCH v34 03/14] Move VM assert into prune/freeze code and simplify
 returned values

After pruning and freezing, we do an assert-only validatation that the
page's visibility status matches what we found during the pruning and
freezing pass over the page.

There's no reason to wait until lazy_scan_prune() to do this validation,
as all of the VM setting logic has already been moved to
heap_page_prune_and_freeze().

Doing so also allows us to remove some fields of PruneFreezeResult,
narrowing the scope of values the caller has to think about.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/7ib3sa55sapwjlaz4sijbiq7iezna27kjvvvar4dpgkmadml6t%40gfpkkwmdnepx
---
 src/backend/access/heap/pruneheap.c  | 65 +++++++++++++++++++---------
 src/backend/access/heap/vacuumlazy.c | 35 +--------------
 src/include/access/heapam.h          | 26 ++++-------
 3 files changed, 54 insertions(+), 72 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 014c3c92d6c..192df9a2218 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -991,6 +991,7 @@ heap_page_prune_and_freeze(PruneFreezeParams *params,
 	Buffer		vmbuffer = params->vmbuffer;
 	Page		page = BufferGetPage(buffer);
 	BlockNumber blockno = BufferGetBlockNumber(buffer);
+	TransactionId vm_conflict_horizon = InvalidTransactionId;
 	PruneState	prstate;
 	bool		do_freeze;
 	bool		do_prune;
@@ -1149,23 +1150,8 @@ heap_page_prune_and_freeze(PruneFreezeParams *params,
 	presult->nfrozen = prstate.nfrozen;
 	presult->live_tuples = prstate.live_tuples;
 	presult->recently_dead_tuples = prstate.recently_dead_tuples;
-	presult->all_visible = prstate.all_visible;
-	presult->all_frozen = prstate.all_frozen;
 	presult->hastup = prstate.hastup;
 
-	/*
-	 * For callers planning to update the visibility map, the conflict horizon
-	 * for that record must be the newest xmin on the page.  However, if the
-	 * page is completely frozen, there can be no conflict and the
-	 * vm_conflict_horizon should remain InvalidTransactionId.  This includes
-	 * the case that we just froze all the tuples; the prune-freeze record
-	 * included the conflict XID already so the caller doesn't need it.
-	 */
-	if (presult->all_frozen)
-		presult->vm_conflict_horizon = InvalidTransactionId;
-	else
-		presult->vm_conflict_horizon = prstate.visibility_cutoff_xid;
-
 	presult->lpdead_items = prstate.lpdead_items;
 	/* the presult->deadoffsets array was already filled in */
 
@@ -1183,6 +1169,46 @@ heap_page_prune_and_freeze(PruneFreezeParams *params,
 		}
 	}
 
+	/*
+	 * If updating the visibility map, the conflict horizon for that record
+	 * must be the newest xmin on the page.  However, if the page is
+	 * completely frozen, there can be no conflict and the vm_conflict_horizon
+	 * should remain InvalidTransactionId.  This includes the case that we
+	 * just froze all the tuples; the prune-freeze record included the
+	 * conflict XID already so we don't need to again.
+	 */
+	if (prstate.all_frozen)
+		vm_conflict_horizon = InvalidTransactionId;
+	else
+		vm_conflict_horizon = prstate.visibility_cutoff_xid;
+
+	/*
+	 * During its second pass over the heap, VACUUM calls
+	 * heap_page_would_be_all_visible() to determine whether a page is
+	 * all-visible and all-frozen. The logic here is similar. After completing
+	 * pruning and freezing, use an assertion to verify that our results
+	 * remain consistent with heap_page_would_be_all_visible().
+	 */
+#ifdef USE_ASSERT_CHECKING
+	if (prstate.all_visible)
+	{
+		TransactionId debug_cutoff;
+		bool		debug_all_frozen;
+
+		Assert(presult->lpdead_items == 0);
+
+		Assert(heap_page_is_all_visible(params->relation, buffer,
+										prstate.cutoffs->OldestXmin,
+										&debug_all_frozen,
+										&debug_cutoff, off_loc));
+
+		Assert(prstate.all_frozen == debug_all_frozen);
+
+		Assert(!TransactionIdIsValid(debug_cutoff) ||
+			   debug_cutoff == vm_conflict_horizon);
+	}
+#endif
+
 	/* Now update the visibility map and PD_ALL_VISIBLE hint */
 	Assert(!prstate.all_visible || (prstate.lpdead_items == 0));
 
@@ -1229,22 +1255,21 @@ heap_page_prune_and_freeze(PruneFreezeParams *params,
 		 * make everything safe for REDO was logged when the page's tuples
 		 * were frozen.
 		 */
-		Assert(!prstate.all_frozen ||
-			   !TransactionIdIsValid(presult->vm_conflict_horizon));
+		Assert(!prstate.all_frozen || !TransactionIdIsValid(vm_conflict_horizon));
 
 		visibilitymap_set(params->relation, blockno, buffer,
 						  InvalidXLogRecPtr,
-						  vmbuffer, presult->vm_conflict_horizon,
+						  vmbuffer, vm_conflict_horizon,
 						  new_vmbits);
 
 		if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0)
 		{
 			presult->new_all_visible_pages = 1;
-			if (presult->all_frozen)
+			if (prstate.all_frozen)
 				presult->new_all_visible_frozen_pages = 1;
 		}
 		else if ((old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0 &&
-				 presult->all_frozen)
+				 prstate.all_frozen)
 			presult->new_all_frozen_pages = 1;
 	}
 }
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 323b8e3dde3..90e94b2ac3f 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -456,13 +456,6 @@ static void dead_items_add(LVRelState *vacrel, BlockNumber blkno, OffsetNumber *
 static void dead_items_reset(LVRelState *vacrel);
 static void dead_items_cleanup(LVRelState *vacrel);
 
-#ifdef USE_ASSERT_CHECKING
-static bool heap_page_is_all_visible(Relation rel, Buffer buf,
-									 TransactionId OldestXmin,
-									 bool *all_frozen,
-									 TransactionId *visibility_cutoff_xid,
-									 OffsetNumber *logging_offnum);
-#endif
 static bool heap_page_would_be_all_visible(Relation rel, Buffer buf,
 										   TransactionId OldestXmin,
 										   OffsetNumber *deadoffsets,
@@ -2032,32 +2025,6 @@ lazy_scan_prune(LVRelState *vacrel,
 		vacrel->new_frozen_tuple_pages++;
 	}
 
-	/*
-	 * VACUUM will call heap_page_is_all_visible() during the second pass over
-	 * the heap to determine all_visible and all_frozen for the page -- this
-	 * is a specialized version of the logic from this function.  Now that
-	 * we've finished pruning and freezing, make sure that we're in total
-	 * agreement with heap_page_is_all_visible() using an assertion.
-	 */
-#ifdef USE_ASSERT_CHECKING
-	if (presult.all_visible)
-	{
-		TransactionId debug_cutoff;
-		bool		debug_all_frozen;
-
-		Assert(presult.lpdead_items == 0);
-
-		Assert(heap_page_is_all_visible(vacrel->rel, buf,
-										vacrel->cutoffs.OldestXmin, &debug_all_frozen,
-										&debug_cutoff, &vacrel->offnum));
-
-		Assert(presult.all_frozen == debug_all_frozen);
-
-		Assert(!TransactionIdIsValid(debug_cutoff) ||
-			   debug_cutoff == presult.vm_conflict_horizon);
-	}
-#endif
-
 	/*
 	 * Now save details of the LP_DEAD items from the page in vacrel
 	 */
@@ -3511,7 +3478,7 @@ dead_items_cleanup(LVRelState *vacrel)
  * that expect no LP_DEAD on the page. Currently assert-only, but there is no
  * reason not to use it outside of asserts.
  */
-static bool
+bool
 heap_page_is_all_visible(Relation rel, Buffer buf,
 						 TransactionId OldestXmin,
 						 bool *all_frozen,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index df9fa17a6f9..a0f7974942e 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -257,8 +257,7 @@ typedef struct PruneFreezeParams
 	 * HEAP_PAGE_PRUNE_MARK_UNUSED_NOW indicates that dead items can be set
 	 * LP_UNUSED during pruning.
 	 *
-	 * HEAP_PAGE_PRUNE_FREEZE indicates that we will also freeze tuples, and
-	 * will return 'all_visible', 'all_frozen' flags to the caller.
+	 * HEAP_PAGE_PRUNE_FREEZE indicates that we will also freeze tuples.
 	 *
 	 * HEAP_PAGE_PRUNE_UPDATE_VM indicates that we will set the page's status
 	 * in the VM.
@@ -294,21 +293,6 @@ typedef struct PruneFreezeResult
 	int			live_tuples;
 	int			recently_dead_tuples;
 
-	/*
-	 * all_visible and all_frozen indicate if the all-visible and all-frozen
-	 * bits in the visibility map can be set for this page, after pruning.
-	 *
-	 * vm_conflict_horizon is the newest xmin of live tuples on the page.  The
-	 * caller can use it as the conflict horizon when setting the VM bits.  It
-	 * is only valid if we froze some tuples (nfrozen > 0), and all_frozen is
-	 * true.
-	 *
-	 * These are only set if the HEAP_PAGE_PRUNE_FREEZE option is set.
-	 */
-	bool		all_visible;
-	bool		all_frozen;
-	TransactionId vm_conflict_horizon;
-
 	/*
 	 * Whether or not the page was newly set all-visible and all-frozen during
 	 * phase I of vacuuming.
@@ -453,7 +437,13 @@ extern void log_heap_prune_and_freeze(Relation relation, Buffer buffer,
 /* in heap/vacuumlazy.c */
 extern void heap_vacuum_rel(Relation rel,
 							const VacuumParams params, BufferAccessStrategy bstrategy);
-
+#ifdef USE_ASSERT_CHECKING
+extern bool heap_page_is_all_visible(Relation rel, Buffer buf,
+									 TransactionId OldestXmin,
+									 bool *all_frozen,
+									 TransactionId *visibility_cutoff_xid,
+									 OffsetNumber *logging_offnum);
+#endif
 /* in heap/heapam_visibility.c */
 extern bool HeapTupleSatisfiesVisibility(HeapTuple htup, Snapshot snapshot,
 										 Buffer buffer);
-- 
2.43.0