From d39ebcdc835f840d36f79a3fa05db6c2d5918cf0 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 16 Jan 2024 17:28:07 -0500
Subject: [PATCH v8 4/4] Combine FSM updates for prune and no prune cases

The goal is to update the FSM once for each page vacuumed. The logic for
ensuring this is the same for lazy_scan_prune() and lazy_scan_noprune().
Combine those FSM updates.

There is one functional difference. Now, the FSM may be vacuumed
whenever we update the FSM in lazy_scan_heap(). Previously, it was not
considered if lazy_scan_noprune() returned true or if we called
lazy_scan_prune() for a block of a relation with indexes (or a relation
with no indexes but which did not have any dead items after pruning).
---
 src/backend/access/heap/vacuumlazy.c | 66 ++++++++++++----------------
 1 file changed, 27 insertions(+), 39 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 610740c44e7..86b087016d2 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -816,6 +816,7 @@ lazy_scan_heap(LVRelState *vacrel)
 	Buffer		vmbuffer = InvalidBuffer;
 	bool		next_unskippable_allvis,
 				skipping_current_range;
+	bool		do_prune;
 	const int	initprog_index[] = {
 		PROGRESS_VACUUM_PHASE,
 		PROGRESS_VACUUM_TOTAL_HEAP_BLKS,
@@ -954,48 +955,28 @@ lazy_scan_heap(LVRelState *vacrel)
 
 			/*
 			 * Collect LP_DEAD items in dead_items array, count tuples,
-			 * determine if rel truncation is safe
+			 * determine if rel truncation is safe. If lazy_scan_noprune()
+			 * returns false, we must get a cleanup lock and call
+			 * lazy_scan_prune().
 			 */
-			if (lazy_scan_noprune(vacrel, buf, blkno, page, &has_lpdead_items))
-			{
-				Size		freespace = 0;
-				bool		recordfreespace;
-
-				/*
-				 * We processed the page successfully (without a cleanup
-				 * lock).
-				 *
-				 * Update the FSM, just as we would in the case where
-				 * lazy_scan_prune() is called. Our goal is to update the
-				 * freespace map the last time we touch the page. If the
-				 * relation has no indexes, or if index vacuuming is disabled,
-				 * there will be no second heap pass; if this particular page
-				 * has no dead items, the second heap pass will not touch this
-				 * page. So, in those cases, update the FSM now.
-				 *
-				 * After a call to lazy_scan_prune(), we would also try to
-				 * adjust the page-level all-visible bit and the visibility
-				 * map, but we skip that step in this path.
-				 */
-				recordfreespace = vacrel->nindexes == 0
-					|| !vacrel->do_index_vacuuming
-					|| !has_lpdead_items;
-				if (recordfreespace)
-					freespace = PageGetHeapFreeSpace(page);
-				UnlockReleaseBuffer(buf);
-				if (recordfreespace)
-					RecordPageWithFreeSpace(vacrel->rel, blkno, freespace);
-				continue;
-			}
+			do_prune = !lazy_scan_noprune(vacrel, buf, blkno, page,
+										  &has_lpdead_items);
 
 			/*
 			 * lazy_scan_noprune could not do all required processing.  Wait
-			 * for a cleanup lock, and call lazy_scan_prune in the usual way.
+			 * for a cleanup lock, and call lazy_scan_prune() in the usual
+			 * way.
 			 */
-			Assert(vacrel->aggressive);
-			LockBuffer(buf, BUFFER_LOCK_UNLOCK);
-			LockBufferForCleanup(buf);
+			if (do_prune)
+			{
+				Assert(vacrel->aggressive);
+				LockBuffer(buf, BUFFER_LOCK_UNLOCK);
+				LockBufferForCleanup(buf);
+			}
 		}
+		/* If we do get a cleanup lock, we will definitely prune */
+		else
+			do_prune = true;
 
 		/* Check for new or empty pages before lazy_scan_prune call */
 		if (lazy_scan_new_or_empty(vacrel, buf, blkno, page, false, vmbuffer))
@@ -1014,9 +995,10 @@ lazy_scan_heap(LVRelState *vacrel)
 		 * tuple headers of remaining items with storage. It also determines
 		 * if truncating this block is safe.
 		 */
-		lazy_scan_prune(vacrel, buf, blkno, page,
-						vmbuffer, all_visible_according_to_vm,
-						&has_lpdead_items);
+		if (do_prune)
+			lazy_scan_prune(vacrel, buf, blkno, page,
+							vmbuffer, all_visible_according_to_vm,
+							&has_lpdead_items);
 
 		/*
 		 * Final steps for block: drop cleanup lock, record free space in the
@@ -1028,6 +1010,12 @@ lazy_scan_heap(LVRelState *vacrel)
 		 * space that lazy_vacuum_heap_page() will make available in cases
 		 * where it's possible to truncate the page's line pointer array.
 		 *
+		 * Our goal is to update the freespace map the last time we touch the
+		 * page. If the relation has no indexes, or if index vacuuming is
+		 * disabled, there will be no second heap pass; if this particular
+		 * page has no dead items, the second heap pass will not touch this
+		 * page. So, in those cases, update the FSM now.
+		 *
 		 * Note: It's not in fact 100% certain that we really will call
 		 * lazy_vacuum_heap_rel() -- lazy_vacuum() might yet opt to skip index
 		 * vacuuming (and so must skip heap vacuuming).  This is deemed okay
-- 
2.37.2

