From 32ea236361ebedbb27b52a865d017b2858203812 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 6 Jun 2009 22:13:52 +0000
Subject: [PATCH] Improve the IndexVacuumInfo/IndexBulkDeleteResult API to
 allow somewhat sane behavior in cases where we don't know the heap tuple
 count accurately; in particular partial vacuum, but this also makes the API a
 bit more useful for ANALYZE.  This patch adds "estimated_count" flags to both
 structs so that an approximate count can be flagged as such, and adjusts the
 logic so that approximate counts are not used for updating
 pg_class.reltuples.

This fixes my previous complaint that VACUUM was putting ridiculous values
into pg_class.reltuples for indexes.  The actual impact of that bug is
limited, because the planner only pays attention to reltuples for an index
if the index is partial; which probably explains why beta testers hadn't
noticed a degradation in plan quality from it.  But it needs to be fixed.

The whole thing is a bit messy and should be redesigned in future, because
reltuples now has the potential to drift quite far away from reality when
a long period elapses with no non-partial vacuums.  But this is as good as
it's going to get for 8.4.
---
 src/backend/access/gin/ginvacuum.c   |  3 ++-
 src/backend/access/gist/gistvacuum.c |  5 ++--
 src/backend/access/hash/hash.c       |  5 +++-
 src/backend/access/nbtree/nbtree.c   | 10 ++++----
 src/backend/catalog/index.c          |  5 ++--
 src/backend/commands/analyze.c       |  5 ++--
 src/backend/commands/vacuum.c        | 34 +++++++++++++++++++---------
 src/backend/commands/vacuumlazy.c    | 30 +++++++++++++++---------
 src/backend/postmaster/pgstat.c      |  9 +++++++-
 src/include/access/genam.h           | 14 ++++++++----
 10 files changed, 81 insertions(+), 39 deletions(-)

diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
index dd98b9fd284..934bf7c3628 100644
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *			$PostgreSQL: pgsql/src/backend/access/gin/ginvacuum.c,v 1.28 2009/03/24 20:17:11 tgl Exp $
+ *			$PostgreSQL: pgsql/src/backend/access/gin/ginvacuum.c,v 1.29 2009/06/06 22:13:50 tgl Exp $
  *-------------------------------------------------------------------------
  */
 
@@ -741,6 +741,7 @@ ginvacuumcleanup(PG_FUNCTION_ARGS)
 	 * tell how many distinct heap entries are referenced by a GIN index.
 	 */
 	stats->num_index_tuples = info->num_heap_tuples;
+	stats->estimated_count = info->estimated_count;
 
 	/*
 	 * If vacuum full, we already have exclusive lock on the index. Otherwise,
diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.c
index 01b8512d070..833e6c574eb 100644
--- a/src/backend/access/gist/gistvacuum.c
+++ b/src/backend/access/gist/gistvacuum.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.43 2009/03/24 20:17:11 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.44 2009/06/06 22:13:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -524,8 +524,8 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
 	{
 		stats = (GistBulkDeleteResult *) palloc0(sizeof(GistBulkDeleteResult));
 		/* use heap's tuple count */
-		Assert(info->num_heap_tuples >= 0);
 		stats->std.num_index_tuples = info->num_heap_tuples;
+		stats->std.estimated_count = info->estimated_count;
 
 		/*
 		 * XXX the above is wrong if index is partial.	Would it be OK to just
@@ -679,6 +679,7 @@ gistbulkdelete(PG_FUNCTION_ARGS)
 	if (stats == NULL)
 		stats = (GistBulkDeleteResult *) palloc0(sizeof(GistBulkDeleteResult));
 	/* we'll re-count the tuples each time */
+	stats->std.estimated_count = false;
 	stats->std.num_index_tuples = 0;
 
 	stack = (GistBDItem *) palloc0(sizeof(GistBDItem));
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 58cccb6e724..4c1cd5ceda9 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/hash/hash.c,v 1.110 2009/05/05 19:36:32 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/hash/hash.c,v 1.111 2009/06/06 22:13:50 tgl Exp $
  *
  * NOTES
  *	  This file contains only the public interface routines.
@@ -610,6 +610,8 @@ loop_top:
 		/*
 		 * Otherwise, our count is untrustworthy since we may have
 		 * double-scanned tuples in split buckets.	Proceed by dead-reckoning.
+		 * (Note: we still return estimated_count = false, because using this
+		 * count is better than not updating reltuples at all.)
 		 */
 		if (metap->hashm_ntuples > tuples_removed)
 			metap->hashm_ntuples -= tuples_removed;
@@ -623,6 +625,7 @@ loop_top:
 	/* return statistics */
 	if (stats == NULL)
 		stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
+	stats->estimated_count = false;
 	stats->num_index_tuples = num_index_tuples;
 	stats->tuples_removed += tuples_removed;
 	/* hashvacuumcleanup will fill in num_pages */
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index a90a0b18340..55d947e9f27 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtree.c,v 1.169 2009/05/05 19:36:32 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtree.c,v 1.170 2009/06/06 22:13:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -579,10 +579,11 @@ btvacuumcleanup(PG_FUNCTION_ARGS)
 	/*
 	 * During a non-FULL vacuum it's quite possible for us to be fooled by
 	 * concurrent page splits into double-counting some index tuples, so
-	 * disbelieve any total that exceeds the underlying heap's count. (We
-	 * can't check this during btbulkdelete.)
+	 * disbelieve any total that exceeds the underlying heap's count ...
+	 * if we know that accurately.  Otherwise this might just make matters
+	 * worse.
 	 */
-	if (!info->vacuum_full)
+	if (!info->vacuum_full && !info->estimated_count)
 	{
 		if (stats->num_index_tuples > info->num_heap_tuples)
 			stats->num_index_tuples = info->num_heap_tuples;
@@ -618,6 +619,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
 	 * Reset counts that will be incremented during the scan; needed in case
 	 * of multiple scans during a single VACUUM command
 	 */
+	stats->estimated_count = false;
 	stats->num_index_tuples = 0;
 	stats->pages_deleted = 0;
 
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index c1001c0fac7..1e19cbff31e 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.316 2009/05/31 20:55:37 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/index.c,v 1.317 2009/06/06 22:13:51 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1938,8 +1938,9 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
 	ivinfo.index = indexRelation;
 	ivinfo.vacuum_full = false;
 	ivinfo.analyze_only = false;
+	ivinfo.estimated_count = true;
 	ivinfo.message_level = DEBUG2;
-	ivinfo.num_heap_tuples = -1;
+	ivinfo.num_heap_tuples = heapRelation->rd_rel->reltuples;
 	ivinfo.strategy = NULL;
 
 	state.tuplesort = tuplesort_begin_datum(TIDOID,
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 5c5eb0444dd..d549fa3bb9f 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.137 2009/05/19 08:30:00 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.138 2009/06/06 22:13:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -498,8 +498,9 @@ cleanup:
 			ivinfo.index = Irel[ind];
 			ivinfo.vacuum_full = false;
 			ivinfo.analyze_only = true;
+			ivinfo.estimated_count = true;
 			ivinfo.message_level = elevel;
-			ivinfo.num_heap_tuples = -1; /* not known for sure */
+			ivinfo.num_heap_tuples = onerel->rd_rel->reltuples;
 			ivinfo.strategy = vac_strategy;
 
 			stats = index_vacuum_cleanup(&ivinfo, NULL);
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 30c1972bcfc..4a5ae53d484 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.387 2009/03/31 22:12:48 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.388 2009/06/06 22:13:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3389,6 +3389,7 @@ scan_index(Relation indrel, double num_tuples)
 	ivinfo.index = indrel;
 	ivinfo.vacuum_full = true;
 	ivinfo.analyze_only = false;
+	ivinfo.estimated_count = false;
 	ivinfo.message_level = elevel;
 	ivinfo.num_heap_tuples = num_tuples;
 	ivinfo.strategy = vac_strategy;
@@ -3398,10 +3399,14 @@ scan_index(Relation indrel, double num_tuples)
 	if (!stats)
 		return;
 
-	/* now update statistics in pg_class */
-	vac_update_relstats(indrel,
-						stats->num_pages, stats->num_index_tuples,
-						false, InvalidTransactionId);
+	/*
+	 * Now update statistics in pg_class, but only if the index says the
+	 * count is accurate.
+	 */
+	if (!stats->estimated_count)
+		vac_update_relstats(indrel,
+							stats->num_pages, stats->num_index_tuples,
+							false, InvalidTransactionId);
 
 	ereport(elevel,
 			(errmsg("index \"%s\" now contains %.0f row versions in %u pages",
@@ -3417,7 +3422,8 @@ scan_index(Relation indrel, double num_tuples)
 	 * Check for tuple count mismatch.	If the index is partial, then it's OK
 	 * for it to have fewer tuples than the heap; else we got trouble.
 	 */
-	if (stats->num_index_tuples != num_tuples)
+	if (!stats->estimated_count &&
+		stats->num_index_tuples != num_tuples)
 	{
 		if (stats->num_index_tuples > num_tuples ||
 			!vac_is_partial_index(indrel))
@@ -3456,6 +3462,7 @@ vacuum_index(VacPageList vacpagelist, Relation indrel,
 	ivinfo.index = indrel;
 	ivinfo.vacuum_full = true;
 	ivinfo.analyze_only = false;
+	ivinfo.estimated_count = false;
 	ivinfo.message_level = elevel;
 	ivinfo.num_heap_tuples = num_tuples + keep_tuples;
 	ivinfo.strategy = vac_strategy;
@@ -3469,10 +3476,14 @@ vacuum_index(VacPageList vacpagelist, Relation indrel,
 	if (!stats)
 		return;
 
-	/* now update statistics in pg_class */
-	vac_update_relstats(indrel,
-						stats->num_pages, stats->num_index_tuples,
-						false, InvalidTransactionId);
+	/*
+	 * Now update statistics in pg_class, but only if the index says the
+	 * count is accurate.
+	 */
+	if (!stats->estimated_count)
+		vac_update_relstats(indrel,
+							stats->num_pages, stats->num_index_tuples,
+							false, InvalidTransactionId);
 
 	ereport(elevel,
 			(errmsg("index \"%s\" now contains %.0f row versions in %u pages",
@@ -3490,7 +3501,8 @@ vacuum_index(VacPageList vacpagelist, Relation indrel,
 	 * Check for tuple count mismatch.	If the index is partial, then it's OK
 	 * for it to have fewer tuples than the heap; else we got trouble.
 	 */
-	if (stats->num_index_tuples != num_tuples + keep_tuples)
+	if (!stats->estimated_count &&
+		stats->num_index_tuples != num_tuples + keep_tuples)
 	{
 		if (stats->num_index_tuples > num_tuples + keep_tuples ||
 			!vac_is_partial_index(indrel))
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index cb73cfa87a7..dd0691d0fe7 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -29,7 +29,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.119 2009/03/24 20:17:14 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.120 2009/06/06 22:13:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -84,9 +84,11 @@ typedef struct LVRelStats
 {
 	/* hasindex = true means two-pass strategy; false means one-pass */
 	bool		hasindex;
+	bool		scanned_all;	/* have we scanned all pages (this far)? */
 	/* Overall statistics about rel */
 	BlockNumber rel_pages;
-	double		rel_tuples;
+	double		old_rel_tuples;	/* previous value of pg_class.reltuples */
+	double		rel_tuples;		/* counts only tuples on scanned pages */
 	BlockNumber pages_removed;
 	double		tuples_deleted;
 	BlockNumber nonempty_pages; /* actually, last nonempty page + 1 */
@@ -96,7 +98,6 @@ typedef struct LVRelStats
 	int			max_dead_tuples;	/* # slots allocated in array */
 	ItemPointer dead_tuples;	/* array of ItemPointerData */
 	int			num_index_scans;
-	bool		scanned_all;	/* have we scanned all pages (this far)? */
 } LVRelStats;
 
 
@@ -174,8 +175,9 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
 
 	vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));
 
-	vacrelstats->num_index_scans = 0;
 	vacrelstats->scanned_all = true; /* will be cleared if we skip a page */
+	vacrelstats->old_rel_tuples = onerel->rd_rel->reltuples;
+	vacrelstats->num_index_scans = 0;
 
 	/* Open all indexes of the relation */
 	vac_open_indexes(onerel, RowExclusiveLock, &nindexes, &Irel);
@@ -876,9 +878,9 @@ lazy_vacuum_index(Relation indrel,
 	ivinfo.index = indrel;
 	ivinfo.vacuum_full = false;
 	ivinfo.analyze_only = false;
+	ivinfo.estimated_count = true;
 	ivinfo.message_level = elevel;
-	/* We don't yet know rel_tuples, so pass -1 */
-	ivinfo.num_heap_tuples = -1;
+	ivinfo.num_heap_tuples = vacrelstats->old_rel_tuples;
 	ivinfo.strategy = vac_strategy;
 
 	/* Do bulk deletion */
@@ -908,8 +910,10 @@ lazy_cleanup_index(Relation indrel,
 	ivinfo.index = indrel;
 	ivinfo.vacuum_full = false;
 	ivinfo.analyze_only = false;
+	ivinfo.estimated_count = !vacrelstats->scanned_all;
 	ivinfo.message_level = elevel;
-	ivinfo.num_heap_tuples = vacrelstats->rel_tuples;
+	/* use rel_tuples only if we scanned all pages, else fall back */
+	ivinfo.num_heap_tuples = vacrelstats->scanned_all ? vacrelstats->rel_tuples : vacrelstats->old_rel_tuples;
 	ivinfo.strategy = vac_strategy;
 
 	stats = index_vacuum_cleanup(&ivinfo, stats);
@@ -917,10 +921,14 @@ lazy_cleanup_index(Relation indrel,
 	if (!stats)
 		return;
 
-	/* now update statistics in pg_class */
-	vac_update_relstats(indrel,
-						stats->num_pages, stats->num_index_tuples,
-						false, InvalidTransactionId);
+	/*
+	 * Now update statistics in pg_class, but only if the index says the
+	 * count is accurate.
+	 */
+	if (!stats->estimated_count)
+		vac_update_relstats(indrel,
+							stats->num_pages, stats->num_index_tuples,
+							false, InvalidTransactionId);
 
 	ereport(elevel,
 			(errmsg("index \"%s\" now contains %.0f row versions in %u pages",
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index f626b91748b..4925e6b3b85 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -13,7 +13,7 @@
  *
  *	Copyright (c) 2001-2009, PostgreSQL Global Development Group
  *
- *	$PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.187 2009/01/01 17:23:46 momjian Exp $
+ *	$PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.188 2009/06/06 22:13:51 tgl Exp $
  * ----------
  */
 #include "postgres.h"
@@ -3774,6 +3774,13 @@ pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len)
 	{
 		if (msg->m_scanned_all)
 			tabentry->last_anl_tuples = msg->m_tuples;
+		else
+		{
+			/* last_anl_tuples must never exceed n_live_tuples+n_dead_tuples */
+			tabentry->last_anl_tuples = Min(tabentry->last_anl_tuples,
+											tabentry->n_live_tuples);
+		}
+
 		if (msg->m_autovacuum)
 			tabentry->autovac_analyze_timestamp = msg->m_vacuumtime;
 		else
diff --git a/src/include/access/genam.h b/src/include/access/genam.h
index 65fd7f73310..868980e3266 100644
--- a/src/include/access/genam.h
+++ b/src/include/access/genam.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/access/genam.h,v 1.76 2009/03/24 20:17:14 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/access/genam.h,v 1.77 2009/06/06 22:13:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -34,14 +34,17 @@ typedef struct IndexBuildResult
 /*
  * Struct for input arguments passed to ambulkdelete and amvacuumcleanup
  *
- * Note that num_heap_tuples will not be valid during ambulkdelete,
- * only amvacuumcleanup.
+ * num_heap_tuples is accurate only when estimated_count is false;
+ * otherwise it's just an estimate (currently, the estimate is the
+ * prior value of the relation's pg_class.reltuples field).  It will
+ * always just be an estimate during ambulkdelete.
  */
 typedef struct IndexVacuumInfo
 {
 	Relation	index;			/* the index being vacuumed */
 	bool		vacuum_full;	/* VACUUM FULL (we have exclusive lock) */
 	bool		analyze_only;	/* ANALYZE (without any actual vacuum) */
+	bool		estimated_count;	/* num_heap_tuples is an estimate */
 	int			message_level;	/* ereport level for progress messages */
 	double		num_heap_tuples;	/* tuples remaining in heap */
 	BufferAccessStrategy strategy;		/* access strategy for reads */
@@ -60,12 +63,15 @@ typedef struct IndexVacuumInfo
  *
  * Note: pages_removed is the amount by which the index physically shrank,
  * if any (ie the change in its total size on disk).  pages_deleted and
- * pages_free refer to free space within the index file.
+ * pages_free refer to free space within the index file.  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.
  */
 typedef struct IndexBulkDeleteResult
 {
 	BlockNumber num_pages;		/* pages remaining in index */
 	BlockNumber pages_removed;	/* # removed during vacuum operation */
+	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 */
-- 
GitLab