From c174ee1a38459d0b84ae92e1dcf21e12d0aadb1d Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Sun, 7 Jan 2024 15:35:58 -0500
Subject: [PATCH v1 07/15] Execute freezing in heap_page_prune()

As a step toward combining the prune and freeze WAL records, execute
freezing in heap_page_prune(). The logic to determine whether or not to
execute freeze plans was moved from lazy_scan_prune() over to
heap_page_prune() with little modification. Updating
vacrel->NewRelfrozenXid and NewRelminMixid remain in lazy_scan_prune().
---
 src/backend/access/heap/pruneheap.c  | 54 +++++++++++++++++++++--
 src/backend/access/heap/vacuumlazy.c | 66 +++++-----------------------
 src/include/access/heapam.h          |  8 ++--
 3 files changed, 65 insertions(+), 63 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index d05bd5c0723..7770da38d84 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -21,6 +21,7 @@
 #include "access/xlog.h"
 #include "access/xloginsert.h"
 #include "catalog/catalog.h"
+#include "executor/instrument.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "storage/bufmgr.h"
@@ -66,7 +67,8 @@ static int	heap_prune_chain(Buffer buffer,
 							 PruneState *prstate, PruneResult *presult);
 
 static void prune_prepare_freeze_tuple(Page page, OffsetNumber offnum,
-									   HeapPageFreeze *pagefrz, PruneResult *presult);
+									   HeapPageFreeze *pagefrz, HeapTupleFreeze *frozen,
+									   PruneResult *presult);
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
 									   OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -233,6 +235,14 @@ heap_page_prune(Relation relation, Buffer buffer,
 				maxoff;
 	PruneState	prstate;
 	HeapTupleData tup;
+	bool		do_freeze;
+	int64		fpi_before = pgWalUsage.wal_fpi;
+	TransactionId frz_conflict_horizon = InvalidTransactionId;
+
+	/*
+	 * One entry for every tuple that we may freeze.
+	 */
+	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
 
 	/*
 	 * Our strategy is to scan the page and make lists of items to change,
@@ -428,7 +438,7 @@ heap_page_prune(Relation relation, Buffer buffer,
 
 		if (pagefrz)
 			prune_prepare_freeze_tuple(page, offnum,
-									   pagefrz, presult);
+									   pagefrz, frozen, presult);
 
 		/* Ignore items already processed as part of an earlier chain */
 		if (prstate.marked[offnum])
@@ -543,6 +553,41 @@ heap_page_prune(Relation relation, Buffer buffer,
 
 	/* Record number of newly-set-LP_DEAD items for caller */
 	presult->nnewlpdead = prstate.ndead;
+
+	/*
+	 * Freeze the page when heap_prepare_freeze_tuple indicates that at least
+	 * one XID/MXID from before FreezeLimit/MultiXactCutoff is present.  Also
+	 * 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)
+		do_freeze = pagefrz->freeze_required ||
+			(presult->all_visible_except_removable && presult->all_frozen &&
+			 presult->nfrozen > 0 &&
+			 fpi_before != pgWalUsage.wal_fpi);
+	else
+		do_freeze = false;
+
+	if (do_freeze)
+	{
+
+		frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
+
+		/* Execute all freeze plans for page as a single atomic action */
+		heap_freeze_execute_prepared(relation, buffer,
+									 frz_conflict_horizon,
+									 frozen, presult->nfrozen);
+	}
+	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
+	{
+		/*
+		 * If we will neither freeze tuples on the page nor set the page all
+		 * frozen in the visibility map, the page is not all frozen and there
+		 * will be no newly frozen tuples.
+		 */
+		presult->all_frozen = false;
+		presult->nfrozen = 0;	/* avoid miscounts in instrumentation */
+	}
 }
 
 
@@ -885,6 +930,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 static void
 prune_prepare_freeze_tuple(Page page, OffsetNumber offnum,
 						   HeapPageFreeze *pagefrz,
+						   HeapTupleFreeze *frozen,
 						   PruneResult *presult)
 {
 	bool		totally_frozen;
@@ -907,11 +953,11 @@ prune_prepare_freeze_tuple(Page page, OffsetNumber offnum,
 
 	/* Tuple with storage -- consider need to freeze */
 	if ((heap_prepare_freeze_tuple(htup, pagefrz,
-								   &presult->frozen[presult->nfrozen],
+								   &frozen[presult->nfrozen],
 								   &totally_frozen)))
 	{
 		/* Save prepared freeze plan for later */
-		presult->frozen[presult->nfrozen++].offset = offnum;
+		frozen[presult->nfrozen++].offset = offnum;
 	}
 
 	/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 90741aad17b..ef9abeb9c87 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -270,9 +270,6 @@ static void update_vacuum_error_info(LVRelState *vacrel,
 static void restore_vacuum_error_info(LVRelState *vacrel,
 									  const LVSavedErrInfo *saved_vacrel);
 
-static TransactionId heap_frz_conflict_horizon(PruneResult *presult,
-											   HeapPageFreeze *pagefrz);
-
 /*
  *	heap_vacuum_rel() -- perform VACUUM for one heap relation
  *
@@ -1350,7 +1347,7 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
  * Determine the snapshotConflictHorizon for freezing. Must only be called
  * after pruning and determining if the page is freezable.
  */
-static TransactionId
+TransactionId
 heap_frz_conflict_horizon(PruneResult *presult, HeapPageFreeze *pagefrz)
 {
 	TransactionId result;
@@ -1421,8 +1418,6 @@ lazy_scan_prune(LVRelState *vacrel,
 				recently_dead_tuples;
 	HeapPageFreeze pagefrz;
 	bool		hastup = false;
-	bool		do_freeze;
-	int64		fpi_before = pgWalUsage.wal_fpi;
 	OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
 
 	Assert(BufferGetBlockNumber(buf) == blkno);
@@ -1575,21 +1570,8 @@ lazy_scan_prune(LVRelState *vacrel,
 
 	vacrel->offnum = InvalidOffsetNumber;
 
-	/*
-	 * Freeze the page when heap_prepare_freeze_tuple indicates that at least
-	 * one XID/MXID from before FreezeLimit/MultiXactCutoff is present.  Also
-	 * 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).
-	 */
-	do_freeze = pagefrz.freeze_required ||
-		(presult.all_visible_except_removable && presult.all_frozen &&
-		 presult.nfrozen > 0 &&
-		 fpi_before != pgWalUsage.wal_fpi);
-
-	if (do_freeze)
+	if (presult.all_frozen || presult.nfrozen > 0)
 	{
-		TransactionId snapshotConflictHorizon;
-
 		/*
 		 * 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.
@@ -1597,50 +1579,26 @@ lazy_scan_prune(LVRelState *vacrel,
 		vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid;
 		vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid;
 
-		vacrel->frozen_pages++;
-
-		snapshotConflictHorizon = heap_frz_conflict_horizon(&presult, &pagefrz);
+		/*
+		 * We never increment the frozen_pages instrumentation counter when
+		 * nfrozen == 0, since it only counts pages with newly frozen tuples
+		 * (don't confuse that with pages newly set all-frozen in VM).
+		 */
+		if (presult.nfrozen > 0)
+			vacrel->frozen_pages++;
 
 		/* Using same cutoff when setting VM is now unnecessary */
-		if (presult.all_visible_except_removable && presult.all_frozen)
+		if (presult.nfrozen > 0 && presult.all_frozen)
 			presult.frz_conflict_horizon = InvalidTransactionId;
-
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(vacrel->rel, buf,
-									 snapshotConflictHorizon,
-									 presult.frozen, presult.nfrozen);
-	}
-	else if (presult.all_frozen && presult.nfrozen == 0)
-	{
-		/* Page should be all visible except to-be-removed tuples */
-		Assert(presult.all_visible_except_removable);
-
-		/*
-		 * We have no freeze plans to execute, so there's no added cost from
-		 * following the freeze path.  That's why it was chosen. This is
-		 * important in the case where the page only contains totally frozen
-		 * tuples at this point (perhaps only following pruning). Such pages
-		 * can be marked all-frozen in the VM by our caller, even though none
-		 * of its tuples were newly frozen here (note that the "no freeze"
-		 * path never sets pages all-frozen).
-		 *
-		 * We never increment the frozen_pages instrumentation counter here,
-		 * since it only counts pages with newly frozen tuples (don't confuse
-		 * that with pages newly set all-frozen in VM).
-		 */
-		vacrel->NewRelfrozenXid = pagefrz.FreezePageRelfrozenXid;
-		vacrel->NewRelminMxid = pagefrz.FreezePageRelminMxid;
 	}
 	else
 	{
 		/*
-		 * Page requires "no freeze" processing.  It might be set all-visible
-		 * in the visibility map, but it can never be set all-frozen.
+		 * Page was "no freeze" processed. 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;
-		presult.all_frozen = false;
-		presult.nfrozen = 0;	/* avoid miscounts in instrumentation */
 	}
 
 	/*
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index bea35afc4bd..e89ebc8cace 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -220,11 +220,6 @@ typedef struct PruneResult
 
 	/* Number of newly frozen tuples */
 	int			nfrozen;
-
-	/*
-	 * One entry for every tuple that we may freeze.
-	 */
-	HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
 } PruneResult;
 
 /*
@@ -307,6 +302,9 @@ extern TM_Result heap_lock_tuple(Relation relation, HeapTuple tuple,
 								 Buffer *buffer, struct TM_FailureData *tmfd);
 
 extern void heap_inplace_update(Relation relation, HeapTuple tuple);
+
+extern TransactionId heap_frz_conflict_horizon(PruneResult *presult,
+											   HeapPageFreeze *pagefrz);
 extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
 									  HeapPageFreeze *pagefrz,
 									  HeapTupleFreeze *frz, bool *totally_frozen);
-- 
2.37.2

