From caf2b10b524b79e71c9bf96ea626df7da8daaf0b Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 29 Mar 2024 21:22:14 -0400
Subject: [PATCH v10 07/10] Prepare freeze tuples in heap_page_prune()

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/heapam.c     |  6 +--
 src/backend/access/heap/pruneheap.c  | 64 +++++++++++++++++++++-----
 src/backend/access/heap/vacuumlazy.c | 67 ++++++++++------------------
 src/include/access/heapam.h          | 25 ++++++++++-
 4 files changed, 103 insertions(+), 59 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 2f6527df0d..f8fddce03b 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6475,10 +6475,10 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
  */
 bool
 heap_prepare_freeze_tuple(HeapTupleHeader tuple,
-						  const struct VacuumCutoffs *cutoffs,
 						  HeapPageFreeze *pagefrz,
 						  HeapTupleFreeze *frz, bool *totally_frozen)
 {
+	const struct VacuumCutoffs *cutoffs = pagefrz->cutoffs;
 	bool		xmin_already_frozen = false,
 				xmax_already_frozen = false;
 	bool		freeze_xmin = false,
@@ -6889,9 +6889,9 @@ heap_freeze_tuple(HeapTupleHeader tuple,
 	pagefrz.FreezePageRelminMxid = MultiXactCutoff;
 	pagefrz.NoFreezePageRelfrozenXid = FreezeLimit;
 	pagefrz.NoFreezePageRelminMxid = MultiXactCutoff;
+	pagefrz.cutoffs = &cutoffs;
 
-	do_freeze = heap_prepare_freeze_tuple(tuple, &cutoffs,
-										  &pagefrz, &frz, &totally_frozen);
+	do_freeze = heap_prepare_freeze_tuple(tuple, &pagefrz, &frz, &totally_frozen);
 
 	/*
 	 * Note that because this is not a WAL-logged operation, we don't need to
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 20d5ad7b80..be06699523 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -17,6 +17,7 @@
 #include "access/heapam.h"
 #include "access/heapam_xlog.h"
 #include "access/htup_details.h"
+#include "access/multixact.h"
 #include "access/transam.h"
 #include "access/xlog.h"
 #include "access/xloginsert.h"
@@ -75,7 +76,7 @@ static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
 static void heap_prune_chain(Buffer buffer,
 							 OffsetNumber rootoffnum,
 							 int8 *htsv,
-							 PruneState *prstate);
+							 PruneState *prstate, PruneResult *presult);
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
 									   OffsetNumber offnum, OffsetNumber rdoffnum, bool was_normal);
@@ -83,7 +84,7 @@ static void heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum, boo
 static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber offnum, bool was_normal);
 static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum, bool was_normal);
 
-static void heap_prune_record_unchanged(Page page, int8 *htsv, PruneState *prstate, OffsetNumber offnum);
+static void heap_prune_record_unchanged(Page page, int8 *htsv, PruneState *prstate, PruneResult *presult, OffsetNumber offnum);
 static void heap_prune_record_unchanged_lp_dead(PruneState *prstate, OffsetNumber offnum);
 static void heap_prune_record_unchanged_lp_redirect(PruneState *prstate, OffsetNumber offnum);
 
@@ -167,6 +168,13 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
 		{
 			PruneResult presult;
 
+			presult.pagefrz.freeze_required = false;
+			presult.pagefrz.FreezePageRelfrozenXid = InvalidTransactionId;
+			presult.pagefrz.FreezePageRelminMxid = InvalidMultiXactId;
+			presult.pagefrz.NoFreezePageRelfrozenXid = InvalidTransactionId;
+			presult.pagefrz.NoFreezePageRelminMxid = InvalidMultiXactId;
+			presult.pagefrz.cutoffs = NULL;
+
 			/*
 			 * For now, do not set PRUNE_DO_MARK_UNUSED_NOW regardless of
 			 * whether or not the relation has indexes, since we cannot safely
@@ -266,6 +274,16 @@ heap_page_prune(Relation relation, Buffer buffer,
 	prstate.nchain_members = 0;
 	prstate.nchain_candidates = 0;
 
+	/*
+	 * If we will prepare to freeze tuples, consider that it might be possible
+	 * to set the page all-frozen in the visibility map.
+	 */
+	if (prstate.actions & PRUNE_DO_TRY_FREEZE)
+		presult->all_frozen = true;
+	else
+		presult->all_frozen = false;
+
+
 	/*
 	 * presult->htsv is not initialized here because all ntuple spots in the
 	 * array will be set either to a valid HTSV_Result value or -1.
@@ -273,6 +291,8 @@ heap_page_prune(Relation relation, Buffer buffer,
 	presult->ndeleted = 0;
 	presult->nnewlpdead = 0;
 
+	presult->nfrozen = 0;
+
 	maxoff = PageGetMaxOffsetNumber(page);
 	tup.t_tableOid = RelationGetRelid(relation);
 
@@ -384,7 +404,7 @@ heap_page_prune(Relation relation, Buffer buffer,
 			*off_loc = offnum;
 
 		/* Process this item or chain of items */
-		heap_prune_chain(buffer, offnum, presult->htsv, &prstate);
+		heap_prune_chain(buffer, offnum, presult->htsv, &prstate, presult);
 	}
 
 	/*
@@ -454,7 +474,7 @@ heap_page_prune(Relation relation, Buffer buffer,
 		 * marked by heap_prune_chain() and heap_prune_record_unchanged() will
 		 * return immediately.
 		 */
-		heap_prune_record_unchanged(page, presult->htsv, &prstate, offnum);
+		heap_prune_record_unchanged(page, presult->htsv, &prstate, presult, offnum);
 	}
 
 /* We should now have processed every tuple exactly once  */
@@ -597,7 +617,7 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
  */
 static void
 heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
-				 int8 *htsv, PruneState *prstate)
+				 int8 *htsv, PruneState *prstate, PruneResult *presult)
 {
 	Page		page = (Page) BufferGetPage(buffer);
 	ItemId		rootlp = PageGetItemId(page, rootoffnum);
@@ -764,7 +784,7 @@ process_chains:
 
 		/* the rest of tuples in the chain are normal, unchanged tuples */
 		for (; i < nchain; i++)
-			heap_prune_record_unchanged(page, htsv, prstate, chainitems[i]);
+			heap_prune_record_unchanged(page, htsv, prstate, presult, chainitems[i]);
 	}
 	else if (ndeadchain == nchain)
 	{
@@ -796,7 +816,7 @@ process_chains:
 
 		/* the rest of tuples in the chain are normal, unchanged tuples */
 		for (int i = ndeadchain; i < nchain; i++)
-			heap_prune_record_unchanged(page, htsv, prstate, chainitems[i]);
+			heap_prune_record_unchanged(page, htsv, prstate, presult, chainitems[i]);
 	}
 }
 
@@ -910,9 +930,10 @@ heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum, bool was_norm
  * Record LP_NORMAL line pointer that is left unchanged.
  */
 static void
-heap_prune_record_unchanged(Page page, int8 *htsv, PruneState *prstate, OffsetNumber offnum)
+heap_prune_record_unchanged(Page page, int8 *htsv, PruneState *prstate,
+							PruneResult *presult, OffsetNumber offnum)
 {
-	HeapTupleHeader htup;
+	HeapTupleHeader htup = (HeapTupleHeader) PageGetItem(page, PageGetItemId(page, offnum));
 
 	Assert(!prstate->marked[offnum]);
 	prstate->marked[offnum] = true;
@@ -933,8 +954,6 @@ heap_prune_record_unchanged(Page page, int8 *htsv, PruneState *prstate, OffsetNu
 		case HEAPTUPLE_RECENTLY_DEAD:
 		case HEAPTUPLE_DELETE_IN_PROGRESS:
 
-			htup = (HeapTupleHeader) PageGetItem(page, PageGetItemId(page, offnum));
-
 			/*
 			 * This tuple may soon become DEAD.  Update the hint field so that
 			 * the page is reconsidered for pruning in future.
@@ -953,6 +972,29 @@ heap_prune_record_unchanged(Page page, int8 *htsv, PruneState *prstate, OffsetNu
 			elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result %d", htsv[offnum]);
 			break;
 	}
+
+	/* Consider freezing any normal tuples which will not be removed */
+	if (prstate->actions & PRUNE_DO_TRY_FREEZE)
+	{
+		/* Tuple with storage -- consider need to freeze */
+		bool		totally_frozen;
+
+		if ((heap_prepare_freeze_tuple(htup, &presult->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;
+	}
 }
 
 
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 880a218cb4..679c6a866e 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1416,19 +1416,15 @@ 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_visible,
-				all_frozen;
+	bool		all_visible;
 	TransactionId visibility_cutoff_xid;
 	uint8		actions = 0;
 	int64		fpi_before = pgWalUsage.wal_fpi;
 	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
-	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
 
 	Assert(BufferGetBlockNumber(buf) == blkno);
 
@@ -1440,12 +1436,12 @@ lazy_scan_prune(LVRelState *vacrel,
 	maxoff = PageGetMaxOffsetNumber(page);
 
 	/* Initialize (or reset) page-level state */
-	pagefrz.freeze_required = false;
-	pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
-	pagefrz.FreezePageRelminMxid = vacrel->NewRelminMxid;
-	pagefrz.NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
-	pagefrz.NoFreezePageRelminMxid = vacrel->NewRelminMxid;
-	tuples_frozen = 0;
+	presult.pagefrz.freeze_required = false;
+	presult.pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
+	presult.pagefrz.FreezePageRelminMxid = vacrel->NewRelminMxid;
+	presult.pagefrz.NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
+	presult.pagefrz.NoFreezePageRelminMxid = vacrel->NewRelminMxid;
+	presult.pagefrz.cutoffs = &vacrel->cutoffs;
 	lpdead_items = 0;
 	live_tuples = 0;
 	recently_dead_tuples = 0;
@@ -1462,6 +1458,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * items LP_UNUSED, so PRUNE_DO_MARK_UNUSED_NOW should be set if no
 	 * indexes and unset otherwise.
 	 */
+	actions |= PRUNE_DO_TRY_FREEZE;
 
 	if (vacrel->nindexes == 0)
 		actions |= PRUNE_DO_MARK_UNUSED_NOW;
@@ -1479,7 +1476,6 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * Also keep track of the visibility cutoff xid for recovery conflicts.
 	 */
 	all_visible = true;
-	all_frozen = true;
 	visibility_cutoff_xid = InvalidTransactionId;
 
 	/*
@@ -1491,7 +1487,6 @@ lazy_scan_prune(LVRelState *vacrel,
 		 offnum = OffsetNumberNext(offnum))
 	{
 		HeapTupleHeader htup;
-		bool		totally_frozen;
 
 		/*
 		 * Set the offset number so that we can display it along with any
@@ -1638,22 +1633,6 @@ 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, &vacrel->cutoffs, &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;
 	}
 
 	/*
@@ -1670,18 +1649,18 @@ 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 ||
-		(all_visible && all_frozen &&
+	if (presult.pagefrz.freeze_required || presult.nfrozen == 0 ||
+		(all_visible && presult.all_frozen &&
 		 fpi_before != pgWalUsage.wal_fpi))
 	{
 		/*
 		 * We're freezing the page.  Our final NewRelfrozenXid doesn't need to
 		 * be affected by the XIDs that are just about to be frozen anyway.
 		 */
-		vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid;
-		vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid;
+		vacrel->NewRelfrozenXid = presult.pagefrz.FreezePageRelfrozenXid;
+		vacrel->NewRelminMxid = presult.pagefrz.FreezePageRelminMxid;
 
-		if (tuples_frozen == 0)
+		if (presult.nfrozen == 0)
 		{
 			/*
 			 * We have no freeze plans to execute, so there's no added cost
@@ -1709,7 +1688,7 @@ lazy_scan_prune(LVRelState *vacrel,
 			 * once we're done with it.  Otherwise we generate a conservative
 			 * cutoff by stepping back from OldestXmin.
 			 */
-			if (all_visible && all_frozen)
+			if (all_visible && presult.all_frozen)
 			{
 				/* Using same cutoff when setting VM is now unnecessary */
 				snapshotConflictHorizon = visibility_cutoff_xid;
@@ -1725,7 +1704,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
@@ -1734,10 +1713,10 @@ lazy_scan_prune(LVRelState *vacrel,
 		 * Page requires "no freeze" processing.  It might be set all-visible
 		 * in the visibility map, but it can never be set all-frozen.
 		 */
-		vacrel->NewRelfrozenXid = pagefrz.NoFreezePageRelfrozenXid;
-		vacrel->NewRelminMxid = pagefrz.NoFreezePageRelminMxid;
-		all_frozen = false;
-		tuples_frozen = 0;		/* avoid miscounts in instrumentation */
+		vacrel->NewRelfrozenXid = presult.pagefrz.NoFreezePageRelfrozenXid;
+		vacrel->NewRelminMxid = presult.pagefrz.NoFreezePageRelminMxid;
+		presult.all_frozen = false;
+		presult.nfrozen = 0;	/* avoid miscounts in instrumentation */
 	}
 
 	/*
@@ -1801,7 +1780,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;
@@ -1824,7 +1803,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	{
 		uint8		flags = VISIBILITYMAP_ALL_VISIBLE;
 
-		if (all_frozen)
+		if (presult.all_frozen)
 		{
 			Assert(!TransactionIdIsValid(visibility_cutoff_xid));
 			flags |= VISIBILITYMAP_ALL_FROZEN;
@@ -1895,7 +1874,7 @@ lazy_scan_prune(LVRelState *vacrel,
 	 * true, so we must check both all_visible and all_frozen.
 	 */
 	else if (all_visible_according_to_vm && 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 b5c711e790..a7f5f19916 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -189,6 +189,7 @@ typedef struct HeapPageFreeze
 	TransactionId NoFreezePageRelfrozenXid;
 	MultiXactId NoFreezePageRelminMxid;
 
+	struct VacuumCutoffs *cutoffs;
 } HeapPageFreeze;
 
 /*
@@ -202,6 +203,15 @@ typedef struct HeapPageFreeze
  */
 #define		PRUNE_DO_MARK_UNUSED_NOW (1 << 1)
 
+/*
+ * Prepare to freeze if advantageous or required and try to advance
+ * relfrozenxid and relminmxid. To attempt freezing, we will need to determine
+ * if the page is all frozen. So, if this action is set, we will also inform
+ * the caller if the page is all-visible and/or all-frozen and calculate a
+ * snapshot conflict horizon for updating the visibility map.
+ */
+#define		PRUNE_DO_TRY_FREEZE (1 << 2)
+
 /*
  * Per-page state returned from pruning
  */
@@ -220,6 +230,20 @@ typedef struct PruneResult
 	 * 1. Otherwise every access would need to subtract 1.
 	 */
 	int8		htsv[MaxHeapTuplesPerPage + 1];
+
+	/*
+	 * Prepare to freeze in heap_page_prune(). lazy_scan_prune() will use the
+	 * returned freeze plans to execute freezing.
+	 */
+	HeapPageFreeze pagefrz;
+
+	/*
+	 * Whether or not the page can be set all-frozen in the visibility map.
+	 * This is only set if the PRUNE_DO_TRY_FREEZE action flag is set.
+	 */
+	bool		all_frozen;
+	int			nfrozen;
+	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
 } PruneResult;
 
 /* 'reason' codes for heap_page_prune() */
@@ -314,7 +338,6 @@ extern TM_Result heap_lock_tuple(Relation relation, ItemPointer tid,
 
 extern void heap_inplace_update(Relation relation, HeapTuple tuple);
 extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
-									  const struct VacuumCutoffs *cutoffs,
 									  HeapPageFreeze *pagefrz,
 									  HeapTupleFreeze *frz, bool *totally_frozen);
 extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-- 
2.40.1

