v15-0009-Vacuum-phase-III-set-PD_ALL_VISIBLE-in-vacuum-WA.patch

From e5fba63482e7d3bd44a991773ac3da50d2402781 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 16 Sep 2025 10:39:31 -0400
Subject: [PATCH v15 09/23] Vacuum phase III set PD_ALL_VISIBLE in vacuum WAL
 record

Instead of setting PD_ALL_VISIBLE on the heap page when setting bits in
the VM, set it when flipping the line pointers on the page to LP_UNUSED.
This will allow us to omit the heap page from the VM WAL chain.

To do this, we must check if the page will be all-visible once we flip
the line pointers before we actually do so.

One functional change is that a single critical section surrounds both
the VM update and the heap update. Previously they were each in a
critical section, so we could crash and have set PD_ALL_VISIBLE but not
set bits in the VM.
---
 src/backend/access/heap/vacuumlazy.c | 140 ++++++++++++++++++++-------
 1 file changed, 105 insertions(+), 35 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 5a6bbbd97f2..9bfcd67a61b 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -465,6 +465,11 @@ static void dead_items_reset(LVRelState *vacrel);
 static void dead_items_cleanup(LVRelState *vacrel);
 static bool heap_page_is_all_visible(LVRelState *vacrel, Buffer buf,
 									 TransactionId *visibility_cutoff_xid, bool *all_frozen);
+static bool heap_page_would_be_all_visible(LVRelState *vacrel, Buffer buf,
+										   OffsetNumber *deadoffsets,
+										   int ndeadoffsets,
+										   bool *all_frozen,
+										   TransactionId *visibility_cutoff_xid);
 static void update_relstats_all_indexes(LVRelState *vacrel);
 static void vacuum_error_callback(void *arg);
 static void update_vacuum_error_info(LVRelState *vacrel,
@@ -2793,6 +2798,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
 	TransactionId visibility_cutoff_xid;
 	bool		all_frozen;
 	LVSavedErrInfo saved_err_info;
+	uint8		vmflags = 0;
 
 	Assert(vacrel->do_index_vacuuming);
 
@@ -2803,6 +2809,18 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
 							 VACUUM_ERRCB_PHASE_VACUUM_HEAP, blkno,
 							 InvalidOffsetNumber);
 
+	if (heap_page_would_be_all_visible(vacrel, buffer,
+									   deadoffsets, num_offsets,
+									   &all_frozen, &visibility_cutoff_xid))
+	{
+		vmflags |= VISIBILITYMAP_ALL_VISIBLE;
+		if (all_frozen)
+		{
+			vmflags |= VISIBILITYMAP_ALL_FROZEN;
+			Assert(!TransactionIdIsValid(visibility_cutoff_xid));
+		}
+	}
+
 	START_CRIT_SECTION();
 
 	for (int i = 0; i < num_offsets; i++)
@@ -2822,6 +2840,13 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
 	/* Attempt to truncate line pointer array now */
 	PageTruncateLinePointerArray(page);
 
+	/*
+	 * The page will never have PD_ALL_VISIBLE already set, so if we are
+	 * setting the VM, we must set PD_ALL_VISIBLE as well.
+	 */
+	if ((vmflags & VISIBILITYMAP_VALID_BITS) != 0)
+		PageSetAllVisible(page);
+
 	/*
 	 * Mark buffer dirty before we write WAL.
 	 */
@@ -2833,7 +2858,7 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
 		log_heap_prune_and_freeze(vacrel->rel, buffer,
 								  InvalidTransactionId,
 								  false,	/* no cleanup lock required */
-								  false,
+								  (vmflags & VISIBILITYMAP_VALID_BITS) != 0,
 								  PRUNE_VACUUM_CLEANUP,
 								  NULL, 0,	/* frozen */
 								  NULL, 0,	/* redirected */
@@ -2842,36 +2867,26 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
 	}
 
 	/*
-	 * End critical section, so we safely can do visibility tests (which
-	 * possibly need to perform IO and allocate memory!). If we crash now the
-	 * page (including the corresponding vm bit) might not be marked all
-	 * visible, but that's fine. A later vacuum will fix that.
+	 * Note that we don't end the critical section until after emitting the VM
+	 * record. This ensures both PD_ALL_VISIBLE and the VM bits are set or
+	 * unset in the event of a crash. While it is correct for PD_ALL_VISIBLE
+	 * to be set and the VM to be clear, we should do our best to keep these
+	 * in sync. This does mean that we will take a lock on the VM buffer
+	 * inside of a critical section, which is generally discouraged. There is
+	 * precedent for this in other callers of visibilitymap_set(), though.
 	 */
-	END_CRIT_SECTION();
 
 	/*
-	 * Now that we have removed the LP_DEAD items from the page, once again
-	 * check if the page has become all-visible.  The page is already marked
-	 * dirty, exclusively locked, and, if needed, a full page image has been
-	 * emitted.
+	 * Now that we have removed the LP_DEAD items from the page, set the
+	 * visibility map if the page became all-visible/all-frozen. Changes to
+	 * the heap page have already been logged.
 	 */
-	Assert(!PageIsAllVisible(page));
-	if (heap_page_is_all_visible(vacrel, buffer, &visibility_cutoff_xid,
-								 &all_frozen))
+	if ((vmflags & VISIBILITYMAP_ALL_VISIBLE) != 0)
 	{
-		uint8		flags = VISIBILITYMAP_ALL_VISIBLE;
-
-		if (all_frozen)
-		{
-			Assert(!TransactionIdIsValid(visibility_cutoff_xid));
-			flags |= VISIBILITYMAP_ALL_FROZEN;
-		}
-
-		PageSetAllVisible(page);
 		visibilitymap_set(vacrel->rel, blkno, buffer,
 						  InvalidXLogRecPtr,
 						  vmbuffer, visibility_cutoff_xid,
-						  flags);
+						  vmflags);
 
 		/* Count the newly set VM page for logging */
 		vacrel->vm_new_visible_pages++;
@@ -2879,6 +2894,8 @@ lazy_vacuum_heap_page(LVRelState *vacrel, BlockNumber blkno, Buffer buffer,
 			vacrel->vm_new_visible_frozen_pages++;
 	}
 
+	END_CRIT_SECTION();
+
 	/* Revert to the previous phase information for error traceback */
 	restore_vacuum_error_info(vacrel, &saved_err_info);
 }
@@ -3540,30 +3557,77 @@ dead_items_cleanup(LVRelState *vacrel)
 }
 
 /*
- * Check if every tuple in the given page is visible to all current and future
- * transactions. Also return the visibility_cutoff_xid which is the highest
- * xmin amongst the visible tuples.  Set *all_frozen to true if every tuple
- * on this page is frozen.
- *
- * This is a stripped down version of lazy_scan_prune().  If you change
- * anything here, make sure that everything stays in sync.  Note that an
- * assertion calls us to verify that everybody still agrees.  Be sure to avoid
- * introducing new side-effects here.
+ * Wrapper for heap_page_would_be_all_visible() which can be used for
+ * callers that expect no LP_DEAD on the page.
  */
 static bool
 heap_page_is_all_visible(LVRelState *vacrel, Buffer buf,
 						 TransactionId *visibility_cutoff_xid,
 						 bool *all_frozen)
 {
+
+	return heap_page_would_be_all_visible(vacrel, buf,
+										  NULL, 0,
+										  all_frozen,
+										  visibility_cutoff_xid);
+}
+
+/*
+ * Determines whether or not the heap page in buf is all-visible other than
+ * the dead line pointers referred to by the provided deadoffsets array.
+ *
+ * deadoffsets are the offsets the caller knows about and already removed
+ * associated index entries. Vacuum will call this before setting those line
+ * pointers LP_UNUSED. So, if there are no new LP_DEAD items, then the page
+ * can be set all-visible in the VM by the caller.
+ *
+ * Returns true if the page is all-visible other than the provided
+ * deadoffsets and false otherwise.
+ *
+ * vacrel->cutoffs.OldestXmin is used to determine visibility.
+ *
+ * *all_frozen is an output parameter indicating to the caller if every tuple
+ * on the page is frozen.
+ *
+ * *visibility_cutoff_xid is an output parameter with the highest xmin amongst the
+ * visible tuples. It is only valid if the page is all-visible.
+ *
+ * Callers looking to verify that the page is already all-visible can call
+ * heap_page_is_all_visible().
+ *
+ * This is similar logic to that in heap_prune_record_unchanged_lp_normal() If
+ * you change anything here, make sure that everything stays in sync.  Note
+ * that an assertion calls us to verify that everybody still agrees.  Be sure
+ * to avoid introducing new side-effects here.
+ */
+static bool
+heap_page_would_be_all_visible(LVRelState *vacrel, Buffer buf,
+							   OffsetNumber *deadoffsets,
+							   int ndeadoffsets,
+							   bool *all_frozen,
+							   TransactionId *visibility_cutoff_xid)
+{
 	Page		page = BufferGetPage(buf);
 	BlockNumber blockno = BufferGetBlockNumber(buf);
 	OffsetNumber offnum,
 				maxoff;
 	bool		all_visible = true;
+	int			matched_dead_count = 0;
 
 	*visibility_cutoff_xid = InvalidTransactionId;
 	*all_frozen = true;
 
+	Assert(ndeadoffsets == 0 || deadoffsets);
+
+#ifdef USE_ASSERT_CHECKING
+	/* Confirm input deadoffsets[] is strictly sorted */
+	if (ndeadoffsets > 1)
+	{
+		for (int i = 1; i < ndeadoffsets; i++)
+			Assert(deadoffsets[i - 1] < deadoffsets[i]);
+	}
+#endif
+
 	maxoff = PageGetMaxOffsetNumber(page);
 	for (offnum = FirstOffsetNumber;
 		 offnum <= maxoff && all_visible;
@@ -3591,9 +3655,15 @@ heap_page_is_all_visible(LVRelState *vacrel, Buffer buf,
 		 */
 		if (ItemIdIsDead(itemid))
 		{
-			all_visible = false;
-			*all_frozen = false;
-			break;
+			if (!deadoffsets ||
+				matched_dead_count >= ndeadoffsets ||
+				deadoffsets[matched_dead_count] != offnum)
+			{
+				*all_frozen = all_visible = false;
+				break;
+			}
+			matched_dead_count++;
+			continue;
 		}
 
 		Assert(ItemIdIsNormal(itemid));
-- 
2.43.0