From 98c24db674571767b1272b430dae83d6babd39bf Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Thu, 3 Mar 2022 13:28:34 -0800
Subject: [PATCH v2] vacuumlazy.c: Document vistest and OldestXmin.

Explain the relationship between vacuumlazy.c's vistest cutoff (cutoff
used by pruning) and OldestXmin cutoff (cutoff used by freezing).  Also
document a closely related assumption: we must establish the rel_pages
used by VACUUM's heap scan _after_ OldestXmin is established.

Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20211211045710.ljtuu4gfloh754rs@alap3.anarazel.de
Discussion: https://postgr.es/m/CAH2-WznYsUxVT156rCQ+q=YD4S4=1M37hWvvHLz-H1pwSM8-Ew@mail.gmail.com
---
 src/backend/access/heap/vacuumlazy.c | 121 ++++++++++++++++-----------
 1 file changed, 71 insertions(+), 50 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 40101e0cb..37b42d50d 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -167,9 +167,10 @@ typedef struct LVRelState
 	MultiXactId relminmxid;
 	double		old_live_tuples;	/* previous value of pg_class.reltuples */
 
-	/* VACUUM operation's cutoff for pruning */
+	/* VACUUM operation's cutoffs for freezing and pruning */
 	TransactionId OldestXmin;
-	/* VACUUM operation's cutoff for freezing XIDs and MultiXactIds */
+	GlobalVisState *vistest;
+	/* VACUUM operation's target cutoffs for freezing XIDs and MultiXactIds */
 	TransactionId FreezeLimit;
 	MultiXactId MultiXactCutoff;
 	/* Are FreezeLimit/MultiXactCutoff still valid? */
@@ -185,8 +186,6 @@ typedef struct LVRelState
 	bool		verbose;		/* VACUUM VERBOSE? */
 
 	/*
-	 * State managed by lazy_scan_heap() follows.
-	 *
 	 * dead_items stores TIDs whose index tuples are deleted by index
 	 * vacuuming. Each TID points to an LP_DEAD line pointer from a heap page
 	 * that has been processed by lazy_scan_prune.  Also needed by
@@ -252,7 +251,6 @@ static bool lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf,
 								   bool sharelock, Buffer vmbuffer);
 static void lazy_scan_prune(LVRelState *vacrel, Buffer buf,
 							BlockNumber blkno, Page page,
-							GlobalVisState *vistest,
 							LVPagePruneState *prunestate);
 static bool lazy_scan_noprune(LVRelState *vacrel, Buffer buf,
 							  BlockNumber blkno, Page page,
@@ -468,27 +466,67 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
 	vacrel->relminmxid = rel->rd_rel->relminmxid;
 	vacrel->old_live_tuples = rel->rd_rel->reltuples;
 
-	/* Set cutoffs for entire VACUUM */
+	/* Initialize page counters explicitly (be tidy) */
+	vacrel->scanned_pages = 0;
+	vacrel->frozenskipped_pages = 0;
+	vacrel->removed_pages = 0;
+	vacrel->lpdead_item_pages = 0;
+	vacrel->missed_dead_pages = 0;
+	vacrel->nonempty_pages = 0;
+
+	/* Allocate/initialize output statistics state */
+	vacrel->new_rel_tuples = 0;
+	vacrel->new_live_tuples = 0;
+	vacrel->indstats = (IndexBulkDeleteResult **)
+		palloc0(vacrel->nindexes * sizeof(IndexBulkDeleteResult *));
+
+	/* Initialize remaining counters (be tidy) */
+	vacrel->num_index_scans = 0;
+	vacrel->tuples_deleted = 0;
+	vacrel->lpdead_items = 0;
+	vacrel->live_tuples = 0;
+	vacrel->recently_dead_tuples = 0;
+	vacrel->missed_dead_tuples = 0;
+
+	/*
+	 * Determine the extent of the blocks that we'll scan in lazy_scan_heap,
+	 * and finalize cutoffs used for freezing and pruning in lazy_scan_prune.
+	 *
+	 * We expect vistest will always make heap_page_prune remove any deleted
+	 * tuple whose xmax is < OldestXmin.  lazy_scan_prune must never become
+	 * confused about whether a tuple should be frozen or removed.  (In the
+	 * future we might want to teach lazy_scan_prune to recompute vistest from
+	 * time to time, to increase the number of dead tuples it can prune away.)
+	 *
+	 * We must determine rel_pages _after_ OldestXmin has been established.
+	 * Any XIDs < OldestXmin must at least be considered for freezing now
+	 * (though perhaps not actually frozen).  lazy_scan_heap's physical heap
+	 * scan (scan of pages < rel_pages) is therefore guaranteed to not miss
+	 * any XIDs of interest.
+	 */
+	orig_rel_pages = RelationGetNumberOfBlocks(rel);
 	vacrel->OldestXmin = OldestXmin;
+	vacrel->vistest = GlobalVisTestFor(rel);
+	/* FreezeLimit indicates which tuples to freeze (always <= OldestXmin) */
 	vacrel->FreezeLimit = FreezeLimit;
 	vacrel->MultiXactCutoff = MultiXactCutoff;
+	vacrel->rel_pages = orig_rel_pages;
 	/* Track if cutoffs became invalid (possible in !aggressive case only) */
 	vacrel->freeze_cutoffs_valid = true;
 
 	/*
 	 * Call lazy_scan_heap to perform all required heap pruning, index
-	 * vacuuming, and heap vacuuming (plus related processing)
+	 * vacuuming, and heap vacuuming (plus related processing).
+	 *
+	 * Note: lazy_scan_heap allocates vacrel->dead_items space for itself.
+	 * Everything else from vacrel has been initialized by now.
 	 */
 	lazy_scan_heap(vacrel, params->nworkers);
 
 	/* Done with indexes */
 	vac_close_indexes(vacrel->nindexes, vacrel->indrels, NoLock);
 
-	/*
-	 * Optionally truncate the relation.  But remember the relation size used
-	 * by lazy_scan_heap for later first.
-	 */
-	orig_rel_pages = vacrel->rel_pages;
+	/* Optionally truncate the relation */
 	if (should_attempt_truncation(vacrel))
 	{
 		update_vacuum_error_info(vacrel, NULL, VACUUM_ERRCB_PHASE_TRUNCATE,
@@ -505,7 +543,12 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
 								 PROGRESS_VACUUM_PHASE_FINAL_CLEANUP);
 
 	/*
-	 * Update statistics in pg_class.
+	 * Prepare to update rel's pg_class entry.
+	 *
+	 * rel's indexes may have already been updated at end of lazy_scan_heap.
+	 * That happens on a best-effort basis, since there is no need to maintain
+	 * critical state like relfrozenxid.  Indexes only need VACUUM to maintain
+	 * reltuples and relpages statistics.
 	 *
 	 * In principle new_live_tuples could be -1 indicating that we (still)
 	 * don't know the tuple count.  In practice that probably can't happen,
@@ -523,6 +566,8 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
 		new_rel_allvisible = new_rel_pages;
 
 	/*
+	 * Now actually update rel's pg_class entry.
+	 *
 	 * Aggressive VACUUM must reliably advance relfrozenxid (and relminmxid).
 	 * We are able to advance relfrozenxid in a non-aggressive VACUUM too,
 	 * provided we didn't skip any all-visible (not all-frozen) pages using
@@ -787,7 +832,7 @@ static void
 lazy_scan_heap(LVRelState *vacrel, int nworkers)
 {
 	VacDeadItems *dead_items;
-	BlockNumber nblocks,
+	BlockNumber rel_pages = vacrel->rel_pages,
 				blkno,
 				next_unskippable_block,
 				next_failsafe_block,
@@ -800,29 +845,6 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 		PROGRESS_VACUUM_MAX_DEAD_TUPLES
 	};
 	int64		initprog_val[3];
-	GlobalVisState *vistest;
-
-	nblocks = RelationGetNumberOfBlocks(vacrel->rel);
-	vacrel->rel_pages = nblocks;
-	vacrel->scanned_pages = 0;
-	vacrel->frozenskipped_pages = 0;
-	vacrel->removed_pages = 0;
-	vacrel->lpdead_item_pages = 0;
-	vacrel->missed_dead_pages = 0;
-	vacrel->nonempty_pages = 0;
-
-	/* Initialize instrumentation counters */
-	vacrel->num_index_scans = 0;
-	vacrel->tuples_deleted = 0;
-	vacrel->lpdead_items = 0;
-	vacrel->live_tuples = 0;
-	vacrel->recently_dead_tuples = 0;
-	vacrel->missed_dead_tuples = 0;
-
-	vistest = GlobalVisTestFor(vacrel->rel);
-
-	vacrel->indstats = (IndexBulkDeleteResult **)
-		palloc0(vacrel->nindexes * sizeof(IndexBulkDeleteResult *));
 
 	/*
 	 * Do failsafe precheck before calling dead_items_alloc.  This ensures
@@ -843,7 +865,7 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 
 	/* Report that we're scanning the heap, advertising total # of blocks */
 	initprog_val[0] = PROGRESS_VACUUM_PHASE_SCAN_HEAP;
-	initprog_val[1] = nblocks;
+	initprog_val[1] = rel_pages;
 	initprog_val[2] = dead_items->max_items;
 	pgstat_progress_update_multi_param(3, initprog_index, initprog_val);
 
@@ -867,9 +889,9 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 	 * Before entering the main loop, establish the invariant that
 	 * next_unskippable_block is the next block number >= blkno that we can't
 	 * skip based on the visibility map, either all-visible for a regular scan
-	 * or all-frozen for an aggressive scan.  We set it to nblocks if there's
-	 * no such block.  We also set up the skipping_blocks flag correctly at
-	 * this stage.
+	 * or all-frozen for an aggressive scan.  We set it to rel_pages when
+	 * there's no such block.  We also set up the skipping_blocks flag
+	 * correctly at this stage.
 	 *
 	 * Note: The value returned by visibilitymap_get_status could be slightly
 	 * out-of-date, since we make this test before reading the corresponding
@@ -887,7 +909,7 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 	next_unskippable_block = 0;
 	if (vacrel->skipwithvm)
 	{
-		while (next_unskippable_block < nblocks)
+		while (next_unskippable_block < rel_pages)
 		{
 			uint8		vmstatus;
 
@@ -914,7 +936,7 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 	else
 		skipping_blocks = false;
 
-	for (blkno = 0; blkno < nblocks; blkno++)
+	for (blkno = 0; blkno < rel_pages; blkno++)
 	{
 		Buffer		buf;
 		Page		page;
@@ -932,7 +954,7 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 			next_unskippable_block++;
 			if (vacrel->skipwithvm)
 			{
-				while (next_unskippable_block < nblocks)
+				while (next_unskippable_block < rel_pages)
 				{
 					uint8		vmskipflags;
 
@@ -977,7 +999,7 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 			/*
 			 * The current page can be skipped if we've seen a long enough run
 			 * of skippable blocks to justify skipping it -- provided it's not
-			 * the last page in the relation (according to rel_pages/nblocks).
+			 * the last page in the relation (according to rel_pages).
 			 *
 			 * We always scan the table's last page to determine whether it
 			 * has tuples or not, even if it would otherwise be skipped. This
@@ -985,7 +1007,7 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 			 * on the table to attempt a truncation that just fails
 			 * immediately because there are tuples on the last page.
 			 */
-			if (skipping_blocks && blkno < nblocks - 1)
+			if (skipping_blocks && blkno < rel_pages - 1)
 			{
 				/*
 				 * Tricky, tricky.  If this is in aggressive vacuum, the page
@@ -1153,7 +1175,7 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 		 * were pruned some time earlier.  Also considers freezing XIDs in the
 		 * tuple headers of remaining items with storage.
 		 */
-		lazy_scan_prune(vacrel, buf, blkno, page, vistest, &prunestate);
+		lazy_scan_prune(vacrel, buf, blkno, page, &prunestate);
 
 		Assert(!prunestate.all_visible || !prunestate.has_lpdead_items);
 
@@ -1352,7 +1374,7 @@ lazy_scan_heap(LVRelState *vacrel, int nworkers)
 	vacrel->blkno = InvalidBlockNumber;
 
 	/* now we can compute the new value for pg_class.reltuples */
-	vacrel->new_live_tuples = vac_estimate_reltuples(vacrel->rel, nblocks,
+	vacrel->new_live_tuples = vac_estimate_reltuples(vacrel->rel, rel_pages,
 													 vacrel->scanned_pages,
 													 vacrel->live_tuples);
 
@@ -1559,7 +1581,6 @@ lazy_scan_prune(LVRelState *vacrel,
 				Buffer buf,
 				BlockNumber blkno,
 				Page page,
-				GlobalVisState *vistest,
 				LVPagePruneState *prunestate)
 {
 	Relation	rel = vacrel->rel;
@@ -1598,7 +1619,7 @@ retry:
 	 * lpdead_items's final value can be thought of as the number of tuples
 	 * that were deleted from indexes.
 	 */
-	tuples_deleted = heap_page_prune(rel, buf, vistest,
+	tuples_deleted = heap_page_prune(rel, buf, vacrel->vistest,
 									 InvalidTransactionId, 0, &nnewlpdead,
 									 &vacrel->offnum);
 
-- 
2.30.2

