Clear up issue with FSM and oldest bpto.xact.

petergeoghegan · petergeoghegan · commit e1477db92105 · 2020-05-01T12:19:42.000-07:00
On further reflection, code comments added by commit b0229f2 slightly misrepresented how we determine the oldest bpto.xact for the index. btvacuumpage() does not treat the bpto.xact of a page that it put in the FSM as a candidate to be the oldest deleted page (the delete-marked page that has the oldest bpto.xact XID among all pages encountered). The definition of a deleted page for the purposes of the bpto.xact calculation is different from the definition used by the bulk delete statistics. The bulk delete statistics don't distinguish between pages that were deleted by the current VACUUM, pages deleted by a previous VACUUM operation but not yet recyclable/reusable, and pages that are reusable (though reusable pages are counted separately). Backpatch: 11-, just like commit b0229f2.
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
@@ -818,12 +818,6 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
 		 * If any oldest btpo.xact from a previously deleted page in the index
 		 * is older than RecentGlobalXmin, then at least one deleted page can
 		 * be recycled -- don't skip cleanup.
-		 *
-		 * Note that btvacuumpage currently doesn't make any effort to
-		 * recognize when a recycled page is already in the FSM (i.e. put
-		 * there by a previous VACUUM operation).  We have to be conservative
-		 * because the FSM isn't crash safe.  Hopefully recycled pages get
-		 * reused before too long.
 		 */
 		result = true;
 	}
@@ -1073,15 +1067,6 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 
 	MemoryContextDelete(vstate.pagedelcontext);
 
-	/*
-	 * Maintain a count of the oldest btpo.xact and current number of heap
-	 * tuples in the metapage (for the benefit of _bt_vacuum_needs_cleanup).
-	 * The oldest page is typically a page deleted by a previous VACUUM
-	 * operation.
-	 */
-	_bt_update_meta_cleanup_info(rel, vstate.oldestBtpoXact,
-								 info->num_heap_tuples);
-
 	/*
 	 * If we found any recyclable pages (and recorded them in the FSM), then
 	 * forcibly update the upper-level FSM pages to ensure that searchers can
@@ -1097,6 +1082,21 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	if (vstate.totFreePages > 0)
 		IndexFreeSpaceMapVacuum(rel);
 
+	/*
+	 * Maintain the oldest btpo.xact and a count of the current number of heap
+	 * tuples in the metapage (for the benefit of _bt_vacuum_needs_cleanup).
+	 *
+	 * The page with the oldest btpo.xact is typically a page deleted by this
+	 * VACUUM operation, since pages deleted by a previous VACUUM operation
+	 * tend to be placed in the FSM (by the current VACUUM operation) -- such
+	 * pages are not candidates to be the oldest btpo.xact.  (Note that pages
+	 * placed in the FSM are reported as deleted pages in the bulk delete
+	 * statistics, despite not counting as deleted pages for the purposes of
+	 * determining the oldest btpo.xact.)
+	 */
+	_bt_update_meta_cleanup_info(rel, vstate.oldestBtpoXact,
+								 info->num_heap_tuples);
+
 	/* update statistics */
 	stats->num_pages = num_pages;
 	stats->pages_free = vstate.totFreePages;
