From ea89b5f9892a7c42c0b23f78545c4201024813f4 Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <pg@bowt.ie>
Date: Sun, 14 Feb 2021 17:38:34 -0800
Subject: [PATCH v5 3/3] Show "pages newly deleted" in VACUUM VERBOSE output.

Teach VACUUM VERBOSE to distinguish between pages that were deleted by
the current VACUUM operation and all deleted pages in the index (without
regard to when or how they became deleted).  The latter metric has been
output by VACUUM verbose for many years.  Showing both together seems
far more informative.

The new VACUUM VERBOSE field will be helpful to both PostgreSQL users
and PostgreSQL developers that want to understand when and how page
deletions are executed, and when and how free pages can actually be
recycled.
---
 src/include/access/genam.h            | 11 ++++++++---
 src/backend/access/gin/ginvacuum.c    |  1 +
 src/backend/access/gist/gistvacuum.c  | 19 ++++++++++++++++---
 src/backend/access/heap/vacuumlazy.c  |  4 +++-
 src/backend/access/nbtree/nbtpage.c   |  4 ++++
 src/backend/access/nbtree/nbtree.c    | 17 +++++++++++------
 src/backend/access/spgist/spgvacuum.c |  1 +
 7 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/src/include/access/genam.h b/src/include/access/genam.h
index ffa1a4c80d..13971c8b2a 100644
--- a/src/include/access/genam.h
+++ b/src/include/access/genam.h
@@ -63,8 +63,12 @@ typedef struct IndexVacuumInfo
  * of which this is just the first field; this provides a way for ambulkdelete
  * to communicate additional private data to amvacuumcleanup.
  *
- * Note: pages_deleted and pages_free refer to free space within the index
- * file.  Some index AMs may compute num_index_tuples by reference to
+ * Note: pages_newly_deleted is the number of pages in the index that were
+ * deleted by the current vacuum operation.  pages_deleted and pages_free
+ * refer to free space within the index file (and so pages_deleted must be >=
+ * pages_newly_deleted).
+ *
+ * Note: Some index AMs may compute num_index_tuples by reference to
  * num_heap_tuples, in which case they should copy the estimated_count field
  * from IndexVacuumInfo.
  */
@@ -74,7 +78,8 @@ typedef struct IndexBulkDeleteResult
 	bool		estimated_count;	/* num_index_tuples is an estimate */
 	double		num_index_tuples;	/* tuples remaining */
 	double		tuples_removed; /* # removed during vacuum operation */
-	BlockNumber pages_deleted;	/* # unused pages in index */
+	BlockNumber pages_newly_deleted;	/* # pages marked deleted by us  */
+	BlockNumber pages_deleted;	/* # pages marked deleted (could be by us) */
 	BlockNumber pages_free;		/* # pages available for reuse */
 } IndexBulkDeleteResult;
 
diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
index a0453b36cd..a276eb020b 100644
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -231,6 +231,7 @@ ginDeletePage(GinVacuumState *gvs, BlockNumber deleteBlkno, BlockNumber leftBlkn
 
 	END_CRIT_SECTION();
 
+	gvs->result->pages_newly_deleted++;
 	gvs->result->pages_deleted++;
 }
 
diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c
index ddecb8ab18..0663193531 100644
--- a/src/backend/access/gist/gistvacuum.c
+++ b/src/backend/access/gist/gistvacuum.c
@@ -133,9 +133,21 @@ gistvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	MemoryContext oldctx;
 
 	/*
-	 * Reset counts that will be incremented during the scan; needed in case
-	 * of multiple scans during a single VACUUM command.
+	 * Reset fields that track information about the entire index now.  This
+	 * avoids double-counting in the case where a single VACUUM command
+	 * requires multiple scans of the index.
+	 *
+	 * Avoid resetting the tuples_removed and pages_newly_deleted fields here,
+	 * since they track information about the VACUUM command, and so must last
+	 * across each call to gistvacuumscan().
+	 *
+	 * (Note that pages_free is treated as state about the whole index, not
+	 * the current VACUUM.  This is appropriate because RecordFreeIndexPage()
+	 * calls are idempotent, and get repeated for the same deleted pages in
+	 * some scenarios.  The point for us is to track the number of recyclable
+	 * pages in the index at the end of the VACUUM command.)
 	 */
+	stats->num_pages = 0;
 	stats->estimated_count = false;
 	stats->num_index_tuples = 0;
 	stats->pages_deleted = 0;
@@ -281,8 +293,8 @@ restart:
 	{
 		/* Okay to recycle this page */
 		RecordFreeIndexPage(rel, blkno);
-		vstate->stats->pages_free++;
 		vstate->stats->pages_deleted++;
+		vstate->stats->pages_free++;
 	}
 	else if (GistPageIsDeleted(page))
 	{
@@ -636,6 +648,7 @@ gistdeletepage(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	/* mark the page as deleted */
 	MarkBufferDirty(leafBuffer);
 	GistPageSetDeleted(leafPage, txid);
+	stats->pages_newly_deleted++;
 	stats->pages_deleted++;
 
 	/* remove the downlink from the parent */
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 0bb78162f5..d8f847b0e6 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2521,9 +2521,11 @@ lazy_cleanup_index(Relation indrel,
 						(*stats)->num_index_tuples,
 						(*stats)->num_pages),
 				 errdetail("%.0f index row versions were removed.\n"
-						   "%u index pages have been deleted, %u are currently reusable.\n"
+						   "%u index pages were newly deleted.\n"
+						   "%u index pages are currently deleted, of which %u are currently reusable.\n"
 						   "%s.",
 						   (*stats)->tuples_removed,
+						   (*stats)->pages_newly_deleted,
 						   (*stats)->pages_deleted, (*stats)->pages_free,
 						   pg_rusage_show(&ru0))));
 	}
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 55395c87c1..9de24b7a54 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -2674,11 +2674,15 @@ _bt_unlink_halfdead_page(Relation rel, Buffer leafbuf, BlockNumber scanblkno,
 		_bt_relbuf(rel, buf);
 
 	/*
+	 * Maintain pages_newly_deleted, which is simply the number of pages
+	 * deleted by the ongoing VACUUM operation.
+	 *
 	 * Maintain pages_deleted in a way that takes into account how
 	 * btvacuumpage() will count deleted pages that have yet to become
 	 * scanblkno -- only count page when it's not going to get that treatment
 	 * later on.
 	 */
+	stats->pages_newly_deleted++;
 	if (target <= scanblkno)
 		stats->pages_deleted++;
 
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index b18022936b..8bf5fc439b 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -873,6 +873,9 @@ _bt_newly_deleted_pages_recycle(Relation rel, BTVacState *vstate)
 	IndexBulkDeleteResult *stats = vstate->stats;
 	Relation	heapRel;
 
+	Assert(vstate->ndeleted > 0);
+	Assert(stats->pages_newly_deleted >= vstate->ndeleted);
+
 	/*
 	 * Recompute VACUUM XID boundaries.
 	 *
@@ -1075,9 +1078,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 * avoids double-counting in the case where a single VACUUM command
 	 * requires multiple scans of the index.
 	 *
-	 * Avoid resetting the tuples_removed field here, since it tracks
-	 * information about the VACUUM command, and so must last across each call
-	 * to btvacuumscan().
+	 * Avoid resetting the tuples_removed and pages_newly_deleted fields here,
+	 * since they track information about the VACUUM command, and so must last
+	 * across each call to btvacuumscan().
 	 *
 	 * (Note that pages_free is treated as state about the whole index, not
 	 * the current VACUUM.  This is appropriate because RecordFreeIndexPage()
@@ -1318,8 +1321,8 @@ backtrack:
 	else if (P_ISHALFDEAD(opaque))
 	{
 		/*
-		 * Half-dead leaf page.  Try to delete now.  Might update
-		 * pages_deleted below.
+		 * Half-dead leaf page.  Try to delete now.  Might end up incrementing
+		 * pages_newly_deleted/pages_deleted inside _bt_pagedel.
 		 */
 		attempt_pagedel = true;
 	}
@@ -1531,7 +1534,9 @@ backtrack:
 		oldcontext = MemoryContextSwitchTo(vstate->pagedelcontext);
 
 		/*
-		 * _bt_pagedel maintains the bulk delete stats on our behalf
+		 * _bt_pagedel maintains the bulk delete stats on our behalf;
+		 * pages_newly_deleted and pages_deleted are likely to be incremented
+		 * during call
 		 */
 		Assert(blkno == scanblkno);
 		_bt_pagedel(rel, buf, vstate);
diff --git a/src/backend/access/spgist/spgvacuum.c b/src/backend/access/spgist/spgvacuum.c
index 0d02a02222..a9ffca5183 100644
--- a/src/backend/access/spgist/spgvacuum.c
+++ b/src/backend/access/spgist/spgvacuum.c
@@ -891,6 +891,7 @@ spgvacuumscan(spgBulkDeleteState *bds)
 
 	/* Report final stats */
 	bds->stats->num_pages = num_pages;
+	bds->stats->pages_newly_deleted = bds->stats->pages_deleted;
 	bds->stats->pages_free = bds->stats->pages_deleted;
 }
 
-- 
2.27.0

