Subject: [PATCH v2 05/17] Prepare freeze tuples in heap_page_prune()

From: Melanie Plageman <[email-masked]>
Date: Sun, 7 Jan 2024 11:18:52 -0500

In order to combine the freeze and prune records, we must determine
which tuples are freezable before actually executing pruning. All of the
page modifications should be made in the same critical section along
with emitting the combined WAL. Determine whether or not tuples should
or must be frozen and whether or not the page will be all frozen as a
consequence during pruning.
---
 src/backend/access/heap/pruneheap.c  | 78 ++++++++++++++++++++++++++--
 src/backend/access/heap/vacuumlazy.c | 68 ++++++------------------
 src/include/access/heapam.h          | 13 +++++
 3 files changed, 102 insertions(+), 57 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index b3a7ce06699..44a5c0a917b 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -62,6 +62,9 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
 static int	heap_prune_chain(Buffer buffer,
 							 OffsetNumber rootoffnum,
 							 PruneState *prstate, PruneResult *presult);
+
+static void prune_prepare_freeze_tuple(Page page, OffsetNumber offnum,
+									   HeapPageFreeze *pagefrz, PruneResult *presult);
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
 									   OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -155,7 +158,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
 			 * not the relation has indexes, since we cannot safely determine
 			 * that during on-access pruning with the current implementation.
 			 */
-			heap_page_prune(relation, buffer, vistest, false,
+			heap_page_prune(relation, buffer, vistest, false, NULL,
 							&presult, NULL);
 
 			/*
@@ -204,6 +207,9 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
  * mark_unused_now indicates whether or not dead items can be set LP_UNUSED during
  * pruning.
  *
+ * pagefrz contains both input and output parameters used if the caller is
+ * interested in potentially freezing tuples on the page.
+ *
  * off_loc is the offset location required by the caller to use in error
  * callback.
  *
@@ -215,6 +221,7 @@ void
 heap_page_prune(Relation relation, Buffer buffer,
 				GlobalVisState *vistest,
 				bool mark_unused_now,
+				HeapPageFreeze *pagefrz,
 				PruneResult *presult,
 				OffsetNumber *off_loc)
 {
@@ -250,6 +257,7 @@ heap_page_prune(Relation relation, Buffer buffer,
 	 */
 	presult->ndeleted = 0;
 	presult->nnewlpdead = 0;
+	presult->nfrozen = 0;
 
 	/*
 	 * Keep track of whether or not the page is all_visible in case the caller
@@ -396,6 +404,15 @@ heap_page_prune(Relation relation, Buffer buffer,
 	 */
 	presult->all_visible_except_removable = presult->all_visible;
 
+	/*
+	 * We will update the VM after pruning, collecting LP_DEAD items, and
+	 * freezing tuples. Keep track of whether or not the page is all_visible
+	 * and all_frozen and use this information to update the VM. all_visible
+	 * implies lpdead_items == 0, but don't trust all_frozen result unless
+	 * all_visible is also set to true.
+	 */
+	presult->all_frozen = true;
+
 	/* Scan the page */
 	for (offnum = FirstOffsetNumber;
 		 offnum <= maxoff;
@@ -403,14 +420,18 @@ heap_page_prune(Relation relation, Buffer buffer,
 	{
 		ItemId		itemid;
 
-		/* Ignore items already processed as part of an earlier chain */
-		if (prstate.marked[offnum])
-			continue;
-
 		/* see preceding loop */
 		if (off_loc)
 			*off_loc = offnum;
 
+		if (pagefrz)
+			prune_prepare_freeze_tuple(page, offnum,
+									   pagefrz, presult);
+
+		/* Ignore items already processed as part of an earlier chain */
+		if (prstate.marked[offnum])
+			continue;
+
 		/* Nothing to do if slot is empty */
 		itemid = PageGetItemId(page, offnum);
 		if (!ItemIdIsUsed(itemid))
@@ -853,6 +874,53 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 	return ndeleted;
 }
 
+/*
+ * While pruning, before actually executing pruning and updating the line
+ * pointers, we may consider freezing tuples referred to by LP_NORMAL line
+ * pointers whose visibility status is not HEAPTUPLE_DEAD. That is to say, we
+ * want to consider freezing normal tuples which will not be removed.
+*/
+static void
+prune_prepare_freeze_tuple(Page page, OffsetNumber offnum,
+						   HeapPageFreeze *pagefrz,
+						   PruneResult *presult)
+{
+	bool		totally_frozen;
+	HeapTupleHeader htup;
+	ItemId		itemid;
+
+	Assert(pagefrz);
+
+	itemid = PageGetItemId(page, offnum);
+
+	if (!ItemIdIsNormal(itemid))
+		return;
+
+	/* We do not consider freezing tuples which will be removed. */
+	if (presult->htsv[offnum] == HEAPTUPLE_DEAD ||
+		presult->htsv[offnum] == -1)
+		return;
+
+	htup = (HeapTupleHeader) PageGetItem(page, itemid);
+
+	/* Tuple with storage -- consider need to freeze */
+	if ((heap_prepare_freeze_tuple(htup, pagefrz,
+								   &presult->frozen[presult->nfrozen],
+								   &totally_frozen)))
+	{
+		/* Save prepared freeze plan for later */
+		presult->frozen[presult->nfrozen++].offset = offnum;
+	}
+
+	/*
+	 * If any tuple isn't either totally frozen already or eligible to become
+	 * totally frozen (according to its freeze plan), then the page definitely
+	 * cannot be set all-frozen in the visibility map later on
+	 */
+	if (!totally_frozen)
+		presult->all_frozen = false;
+}
+
 /* Record lowest soon-prunable XID */
 static void
 heap_prune_record_prunable(PruneState *prstate, TransactionId xid)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 06e0e841582..4187c998d25 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1416,16 +1416,13 @@ lazy_scan_prune(LVRelState *vacrel,
 				maxoff;
 	ItemId		itemid;
 	PruneResult presult;
-	int			tuples_frozen,
-				lpdead_items,
+	int			lpdead_items,
 				live_tuples,
 				recently_dead_tuples;
 	HeapPageFreeze pagefrz;
 	bool		hastup = false;
-	bool		all_frozen;
 	int64		fpi_before = pgWalUsage.wal_fpi;
 	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
-	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
 
 	Assert(BufferGetBlockNumber(buf) == blkno);
 
@@ -1443,7 +1440,6 @@ lazy_scan_prune(LVRelState *vacrel,
 	pagefrz.NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
 	pagefrz.NoFreezePageRelminMxid = vacrel->NewRelminMxid;
 	pagefrz.cutoffs = &vacrel->cutoffs;
-	tuples_frozen = 0;
 	lpdead_items = 0;
 	live_tuples = 0;
 	recently_dead_tuples = 0;
@@ -1461,31 +1457,20 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * false otherwise.
 	 */
 	heap_page_prune(rel, buf, vacrel->vistest, vacrel->nindexes == 0,
-					&presult, &vacrel->offnum);
+					&pagefrz, &presult, &vacrel->offnum);
 
 	/*
 	 * Now scan the page to collect LP_DEAD items and check for tuples
 	 * requiring freezing among remaining tuples with storage. We will update
 	 * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
-	 * have determined whether or not the page is all_visible. Keep track of
-	 * whether or not the page is all_frozen and use this information to
-	 * update the VM. all_visible implies lpdead_items == 0, but don't trust
-	 * all_frozen result unless all_visible is also set to true.
+	 * have determined whether or not the page is all_visible and able to
+	 * become all_frozen.
 	 *
 	 */
-	all_frozen = true;
-
-	/*
-	 * Now scan the page to collect LP_DEAD items and update the variables set
-	 * just above.
-	 */
 	for (offnum = FirstOffsetNumber;
 		 offnum <= maxoff;
 		 offnum = OffsetNumberNext(offnum))
 	{
-		HeapTupleHeader htup;
-		bool		totally_frozen;
-
 		/*
 		 * Set the offset number so that we can display it along with any
 		 * error that occurred while processing this tuple.
@@ -1521,8 +1506,6 @@ lazy_scan_prune(LVRelState *vacrel,
 
 		Assert(ItemIdIsNormal(itemid));
 
-		htup = (HeapTupleHeader) PageGetItem(page, itemid);
-
 		/*
 		 * The criteria for counting a tuple as live in this block need to
 		 * match what analyze.c's acquire_sample_rows() does, otherwise VACUUM
@@ -1587,29 +1570,8 @@ lazy_scan_prune(LVRelState *vacrel,
 
 		hastup = true;			/* page makes rel truncation unsafe */
 
-		/* Tuple with storage -- consider need to freeze */
-		if (heap_prepare_freeze_tuple(htup, &pagefrz,
-									  &frozen[tuples_frozen], &totally_frozen))
-		{
-			/* Save prepared freeze plan for later */
-			frozen[tuples_frozen++].offset = offnum;
-		}
-
-		/*
-		 * If any tuple isn't either totally frozen already or eligible to
-		 * become totally frozen (according to its freeze plan), then the page
-		 * definitely cannot be set all-frozen in the visibility map later on
-		 */
-		if (!totally_frozen)
-			all_frozen = false;
 	}
 
-	/*
-	 * We have now divided every item on the page into either an LP_DEAD item
-	 * that will need to be vacuumed in indexes later, or a LP_NORMAL tuple
-	 * that remains and needs to be considered for freezing now (LP_UNUSED and
-	 * LP_REDIRECT items also remain, but are of no further interest to us).
-	 */
 	vacrel->offnum = InvalidOffsetNumber;
 
 	/*
@@ -1618,8 +1580,8 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * freeze when pruning generated an FPI, if doing so means that we set the
 	 * page all-frozen afterwards (might not happen until final heap pass).
 	 */
-	if (pagefrz.freeze_required || tuples_frozen == 0 ||
-		(presult.all_visible_except_removable && all_frozen &&
+	if (pagefrz.freeze_required || presult.nfrozen == 0 ||
+		(presult.all_visible_except_removable && presult.all_frozen &&
 		 fpi_before != pgWalUsage.wal_fpi))
 	{
 		/*
@@ -1629,7 +1591,7 @@ lazy_scan_prune(LVRelState *vacrel,
 		vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid;
 		vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid;
 
-		if (tuples_frozen == 0)
+		if (presult.nfrozen == 0)
 		{
 			/*
 			 * We have no freeze plans to execute, so there's no added cost
@@ -1657,7 +1619,7 @@ lazy_scan_prune(LVRelState *vacrel,
 			 * once we're done with it.  Otherwise we generate a conservative
 			 * cutoff by stepping back from OldestXmin.
 			 */
-			if (presult.all_visible_except_removable && all_frozen)
+			if (presult.all_visible_except_removable && presult.all_frozen)
 			{
 				/* Using same cutoff when setting VM is now unnecessary */
 				snapshotConflictHorizon = presult.frz_conflict_horizon;
@@ -1673,7 +1635,7 @@ lazy_scan_prune(LVRelState *vacrel,
 			/* Execute all freeze plans for page as a single atomic action */
 			heap_freeze_execute_prepared(vacrel->rel, buf,
 										 snapshotConflictHorizon,
-										 frozen, tuples_frozen);
+										 presult.frozen, presult.nfrozen);
 		}
 	}
 	else
@@ -1684,8 +1646,8 @@ lazy_scan_prune(LVRelState *vacrel,
 		 */
 		vacrel->NewRelfrozenXid = pagefrz.NoFreezePageRelfrozenXid;
 		vacrel->NewRelminMxid = pagefrz.NoFreezePageRelminMxid;
-		all_frozen = false;
-		tuples_frozen = 0;		/* avoid miscounts in instrumentation */
+		presult.all_frozen = false;
+		presult.nfrozen = 0;	/* avoid miscounts in instrumentation */
 	}
 
 	/*
@@ -1708,6 +1670,8 @@ lazy_scan_prune(LVRelState *vacrel,
 									  &debug_cutoff, &debug_all_frozen))
 			Assert(false);
 
+		Assert(presult.all_frozen == debug_all_frozen);
+
 		Assert(!TransactionIdIsValid(debug_cutoff) ||
 			   debug_cutoff == presult.frz_conflict_horizon);
 	}
@@ -1738,7 +1702,7 @@ lazy_scan_prune(LVRelState *vacrel,
 
 	/* Finally, add page-local counts to whole-VACUUM counts */
 	vacrel->tuples_deleted += presult.ndeleted;
-	vacrel->tuples_frozen += tuples_frozen;
+	vacrel->tuples_frozen += presult.nfrozen;
 	vacrel->lpdead_items += lpdead_items;
 	vacrel->live_tuples += live_tuples;
 	vacrel->recently_dead_tuples += recently_dead_tuples;
@@ -1761,7 +1725,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	{
 		uint8		flags = VISIBILITYMAP_ALL_VISIBLE;
 
-		if (all_frozen)
+		if (presult.all_frozen)
 		{
 			Assert(!TransactionIdIsValid(presult.frz_conflict_horizon));
 			flags |= VISIBILITYMAP_ALL_FROZEN;
@@ -1832,7 +1796,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * true, so we must check both all_visible and all_frozen.
 	 */
 	else if (all_visible_according_to_vm && presult.all_visible &&
-			 all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
+			 presult.all_frozen && !VM_ALL_FROZEN(vacrel->rel, blkno, &vmbuffer))
 	{
 		/*
 		 * Avoid relying on all_visible_according_to_vm as a proxy for the
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 6823ab8b658..bea35afc4bd 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -212,7 +212,19 @@ typedef struct PruneResult
 	 * 1. Otherwise every access would need to subtract 1.
 	 */
 	int8		htsv[MaxHeapTuplesPerPage + 1];
+
 	bool		all_visible_except_removable;
+
+	/* Whether or not the page can be set all frozen in the VM */
+	bool		all_frozen;
+
+	/* Number of newly frozen tuples */
+	int			nfrozen;
+
+	/*
+	 * One entry for every tuple that we may freeze.
+	 */
+	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
 } PruneResult;
 
 /*
@@ -324,6 +336,7 @@ extern void heap_page_prune_opt(Relation relation, Buffer buffer);
 extern void heap_page_prune(Relation relation, Buffer buffer,
 							struct GlobalVisState *vistest,
 							bool mark_unused_now,
+							HeapPageFreeze *pagefrz,
 							PruneResult *presult,
 							OffsetNumber *off_loc);
 extern void heap_page_prune_execute(Buffer buffer,
-- 
2.40.1

--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0006-lazy_scan_prune-reorder-freeze-execution-logic.patch"