Don't reset pg_class.reltuples and relpages in VACUUM, if any pages were

hlinnaka · hlinnaka · commit dcf8409985f4 · 2008-12-17T09:15:03.000Z
skipped. We could update relpages anyway, but it seems better to only
update it together with reltuples, because we use the reltuples/relpages
ratio in the planner. Also don't update n_live_tuples in pgstat.

ANALYZE in VACUUM ANALYZE now needs to update pg_class, if the
VACUUM-phase didn't do so. Added some boolean-passing to let analyze_rel
know if it should update pg_class or not.

I also moved the relcache invalidation (to update rd_targblock) from
vac_update_relstats to where RelationTruncate is called, because
vac_update_relstats is not called for partial vacuums anymore. It's more
obvious to send the invalidation close to the truncation that requires it.

Per report by Ned T. Crigler.
diff --git 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.129 2008/12/13 19:13:44 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/analyze.c,v 1.130 2008/12/17 09:15:02 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -101,10 +101,18 @@ static bool std_typanalyze(VacAttrStats *stats);
 
 /*
  *	analyze_rel() -- analyze one relation
+ *
+ * If update_reltuples is true, we update reltuples and relpages columns
+ * in pg_class.  Caller should pass false if we're part of VACUUM ANALYZE,
+ * and the VACUUM didn't skip any pages.  We only have an approximate count,
+ * so we don't want to overwrite the accurate values already inserted by the
+ * VACUUM in that case.  VACUUM always scans all indexes, however, so the
+ * pg_class entries for indexes are never updated if we're part of VACUUM
+ * ANALYZE.
  */
 void
 analyze_rel(Oid relid, VacuumStmt *vacstmt,
-			BufferAccessStrategy bstrategy)
+			BufferAccessStrategy bstrategy, bool update_reltuples)
 {
 	Relation	onerel;
 	int			attr_cnt,
@@ -364,7 +372,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
 		 * autovacuum code doesn't go nuts trying to get stats about a
 		 * zero-column table.
 		 */
-		if (!vacstmt->vacuum)
+		if (update_reltuples)
 			pgstat_report_analyze(onerel, 0, 0);
 		goto cleanup;
 	}
@@ -455,18 +463,24 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
 	}
 
 	/*
-	 * If we are running a standalone ANALYZE, update pages/tuples stats in
-	 * pg_class.  We know the accurate page count from the smgr, but only an
-	 * approximate number of tuples; therefore, if we are part of VACUUM
-	 * ANALYZE do *not* overwrite the accurate count already inserted by
-	 * VACUUM.	The same consideration applies to indexes.
+	 * Update pages/tuples stats in pg_class.
 	 */
-	if (!vacstmt->vacuum)
+	if (update_reltuples)
 	{
 		vac_update_relstats(onerel,
 							RelationGetNumberOfBlocks(onerel),
 							totalrows, hasindex, InvalidTransactionId);
+		/* report results to the stats collector, too */
+		pgstat_report_analyze(onerel, totalrows, totaldeadrows);
+	}
 
+	/*
+	 * Same for indexes. Vacuum always scans all indexes, so if we're part of
+	 * VACUUM ANALYZE, don't overwrite the accurate count already inserted by 
+	 * VACUUM.
+	 */
+	if (!vacstmt->vacuum)
+	{
 		for (ind = 0; ind < nindexes; ind++)
 		{
 			AnlIndexData *thisdata = &indexdata[ind];
@@ -477,9 +491,6 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
 								RelationGetNumberOfBlocks(Irel[ind]),
 								totalindexrows, false, InvalidTransactionId);
 		}
-
-		/* report results to the stats collector, too */
-		pgstat_report_analyze(onerel, totalrows, totaldeadrows);
 	}
 
 	/* We skip to here if there were no analyzable columns */
diff --git 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.382 2008/12/03 13:05:22 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.383 2008/12/17 09:15:02 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -216,7 +216,7 @@ static List *get_rel_oids(Oid relid, const RangeVar *vacrel,
 			 const char *stmttype);
 static void vac_truncate_clog(TransactionId frozenXID);
 static void vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast,
-		   bool for_wraparound);
+		   bool for_wraparound, bool *scanned_all);
 static void full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt);
 static void scan_heap(VRelStats *vacrelstats, Relation onerel,
 		  VacPageList vacuum_pages, VacPageList fraged_pages);
@@ -436,9 +436,11 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
 		foreach(cur, relations)
 		{
 			Oid			relid = lfirst_oid(cur);
+			bool		scanned_all = false;
 
 			if (vacstmt->vacuum)
-				vacuum_rel(relid, vacstmt, do_toast, for_wraparound);
+				vacuum_rel(relid, vacstmt, do_toast, for_wraparound,
+						   &scanned_all);
 
 			if (vacstmt->analyze)
 			{
@@ -460,7 +462,7 @@ vacuum(VacuumStmt *vacstmt, Oid relid, bool do_toast,
 				else
 					old_context = MemoryContextSwitchTo(anl_context);
 
-				analyze_rel(relid, vacstmt, vac_strategy);
+				analyze_rel(relid, vacstmt, vac_strategy, !scanned_all);
 
 				if (use_own_xacts)
 				{
@@ -756,21 +758,9 @@ vac_update_relstats(Relation relation,
 		dirty = true;
 	}
 
-	/*
-	 * If anything changed, write out the tuple.  Even if nothing changed,
-	 * force relcache invalidation so all backends reset their rd_targblock
-	 * --- otherwise it might point to a page we truncated away.
-	 */
+	/* If anything changed, write out the tuple. */
 	if (dirty)
-	{
 		heap_inplace_update(rd, ctup);
-		/* the above sends a cache inval message */
-	}
-	else
-	{
-		/* no need to change tuple, but force relcache inval anyway */
-		CacheInvalidateRelcacheByTuple(ctup);
-	}
 
 	heap_close(rd, RowExclusiveLock);
 }
@@ -986,10 +976,14 @@ vac_truncate_clog(TransactionId frozenXID)
  *		many small transactions.  Otherwise, two-phase locking would require
  *		us to lock the entire database during one pass of the vacuum cleaner.
  *
+ *		We'll return true in *scanned_all if the vacuum scanned all heap
+ *		pages, and updated pg_class.
+ *
  *		At entry and exit, we are not inside a transaction.
  */
 static void
-vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound)
+vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
+		   bool *scanned_all)
 {
 	LOCKMODE	lmode;
 	Relation	onerel;
@@ -998,6 +992,9 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound)
 	Oid			save_userid;
 	bool		save_secdefcxt;
 
+	if (scanned_all)
+		*scanned_all = false;
+
 	/* Begin a transaction for vacuuming this relation */
 	StartTransactionCommand();
 
@@ -1162,7 +1159,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound)
 	if (vacstmt->full)
 		full_vacuum_rel(onerel, vacstmt);
 	else
-		lazy_vacuum_rel(onerel, vacstmt, vac_strategy);
+		lazy_vacuum_rel(onerel, vacstmt, vac_strategy, scanned_all);
 
 	/* Restore userid */
 	SetUserIdAndContext(save_userid, save_secdefcxt);
@@ -1184,7 +1181,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound)
 	 * totally unimportant for toast relations.
 	 */
 	if (toast_relid != InvalidOid)
-		vacuum_rel(toast_relid, vacstmt, false, for_wraparound);
+		vacuum_rel(toast_relid, vacstmt, false, for_wraparound, NULL);
 
 	/*
 	 * Now release the session-level lock on the master table.
@@ -1296,7 +1293,7 @@ full_vacuum_rel(Relation onerel, VacuumStmt *vacstmt)
 
 	/* report results to the stats collector, too */
 	pgstat_report_vacuum(RelationGetRelid(onerel), onerel->rd_rel->relisshared,
-						 vacstmt->analyze, vacrelstats->rel_tuples);
+						 true, vacstmt->analyze, vacrelstats->rel_tuples);
 }
 
 
@@ -2866,6 +2863,10 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
 	if (blkno < nblocks)
 	{
 		RelationTruncate(onerel, blkno);
+
+		/* force relcache inval so all backends reset their rd_targblock */
+		CacheInvalidateRelcache(onerel);
+
 		vacrelstats->rel_pages = blkno; /* set new number of blocks */
 	}
 
@@ -3286,6 +3287,10 @@ vacuum_heap(VRelStats *vacrelstats, Relation onerel, VacPageList vacuum_pages)
 						RelationGetRelationName(onerel),
 						vacrelstats->rel_pages, relblocks)));
 		RelationTruncate(onerel, relblocks);
+
+		/* force relcache inval so all backends reset their rd_targblock */
+		CacheInvalidateRelcache(onerel);
+
 		vacrelstats->rel_pages = relblocks;		/* set new number of blocks */
 	}
 }
diff --git 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.113 2008/12/04 11:42:23 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.114 2008/12/17 09:15:02 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -50,6 +50,7 @@
 #include "storage/bufmgr.h"
 #include "storage/freespace.h"
 #include "storage/lmgr.h"
+#include "utils/inval.h"
 #include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/pg_rusage.h"
@@ -135,7 +136,7 @@ static int	vac_cmp_itemptr(const void *left, const void *right);
  */
 void
 lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
-				BufferAccessStrategy bstrategy)
+				BufferAccessStrategy bstrategy, bool *scanned_all)
 {
 	LVRelStats *vacrelstats;
 	Relation   *Irel;
@@ -163,7 +164,7 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
 	vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));
 
 	vacrelstats->num_index_scans = 0;
-	vacrelstats->scanned_all = true;
+	vacrelstats->scanned_all = true; /* will be cleared if we skip a page */
 
 	/* Open all indexes of the relation */
 	vac_open_indexes(onerel, RowExclusiveLock, &nindexes, &Irel);
@@ -190,16 +191,23 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
 	FreeSpaceMapVacuum(onerel);
 
 	/*
-	 * Update statistics in pg_class. We can only advance relfrozenxid if we
-	 * didn't skip any pages.
+	 * Update statistics in pg_class.  But only if we didn't skip any pages;
+	 * the tuple count only includes tuples from the pages we've visited, and
+	 * we haven't frozen tuples in unvisited pages either.  The page count is
+	 * accurate in any case, but because we use the reltuples / relpages
+	 * ratio in the planner, it's better to not update relpages either if we
+	 * can't update reltuples.
 	 */
-	vac_update_relstats(onerel,
-						vacrelstats->rel_pages, vacrelstats->rel_tuples,
-						vacrelstats->hasindex,
-						vacrelstats->scanned_all ? FreezeLimit : InvalidOid);
+	if (vacrelstats->scanned_all)
+		vac_update_relstats(onerel,
+							vacrelstats->rel_pages, vacrelstats->rel_tuples,
+							vacrelstats->hasindex,
+							FreezeLimit);
 
 	/* report results to the stats collector, too */
-	pgstat_report_vacuum(RelationGetRelid(onerel), onerel->rd_rel->relisshared,
+	pgstat_report_vacuum(RelationGetRelid(onerel),
+						 onerel->rd_rel->relisshared,
+						 vacrelstats->scanned_all,
 						 vacstmt->analyze, vacrelstats->rel_tuples);
 
 	/* and log the action if appropriate */
@@ -221,6 +229,9 @@ lazy_vacuum_rel(Relation onerel, VacuumStmt *vacstmt,
 						vacrelstats->tuples_deleted, vacrelstats->rel_tuples,
 							pg_rusage_show(&ru0))));
 	}
+
+	if (scanned_all)
+		*scanned_all = vacrelstats->scanned_all;
 }
 
 
@@ -952,12 +963,14 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
 	 */
 	RelationTruncate(onerel, new_rel_pages);
 
+	/* force relcache inval so all backends reset their rd_targblock */
+	CacheInvalidateRelcache(onerel);
+
 	/*
 	 * Note: once we have truncated, we *must* keep the exclusive lock until
-	 * commit.	The sinval message that will be sent at commit (as a result of
-	 * vac_update_relstats()) must be received by other backends, to cause
-	 * them to reset their rd_targblock values, before they can safely access
-	 * the table again.
+	 * commit.	The sinval message won't be sent until commit, and other
+	 * backends must see it and reset their rd_targblock values before they
+	 * can safely access the table again.
 	 */
 
 	/* update statistics */
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.643 2008/12/04 17:51:26 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.644 2008/12/17 09:15:02 heikki Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -5875,7 +5875,7 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 					n->analyze = false;
 					n->full = $2;
 					n->freeze_min_age = $3 ? 0 : -1;
-					n->scan_all = $3;
+					n->scan_all = $2 || $3;
 					n->verbose = $4;
 					n->relation = NULL;
 					n->va_cols = NIL;
@@ -5888,7 +5888,7 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 					n->analyze = false;
 					n->full = $2;
 					n->freeze_min_age = $3 ? 0 : -1;
-					n->scan_all = $3;
+					n->scan_all = $2 || $3;
 					n->verbose = $4;
 					n->relation = $5;
 					n->va_cols = NIL;
@@ -5900,7 +5900,7 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 					n->vacuum = true;
 					n->full = $2;
 					n->freeze_min_age = $3 ? 0 : -1;
-					n->scan_all = $3;
+					n->scan_all = $2 || $3;
 					n->verbose |= $4;
 					$$ = (Node *)n;
 				}
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
@@ -13,7 +13,7 @@
  *
  *	Copyright (c) 2001-2008, PostgreSQL Global Development Group
  *
- *	$PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.185 2008/12/08 15:44:54 tgl Exp $
+ *	$PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.186 2008/12/17 09:15:03 heikki Exp $
  * ----------
  */
 #include "postgres.h"
@@ -1178,7 +1178,7 @@ pgstat_report_autovac(Oid dboid)
  * ---------
  */
 void
-pgstat_report_vacuum(Oid tableoid, bool shared,
+pgstat_report_vacuum(Oid tableoid, bool shared, bool scanned_all,
 					 bool analyze, PgStat_Counter tuples)
 {
 	PgStat_MsgVacuum msg;
@@ -1189,6 +1189,7 @@ pgstat_report_vacuum(Oid tableoid, bool shared,
 	pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_VACUUM);
 	msg.m_databaseid = shared ? InvalidOid : MyDatabaseId;
 	msg.m_tableoid = tableoid;
+	msg.m_scanned_all = scanned_all;
 	msg.m_analyze = analyze;
 	msg.m_autovacuum = IsAutoVacuumWorkerProcess();		/* is this autovacuum? */
 	msg.m_vacuumtime = GetCurrentTimestamp();
@@ -3765,12 +3766,14 @@ pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len)
 		tabentry->autovac_vacuum_timestamp = msg->m_vacuumtime;
 	else
 		tabentry->vacuum_timestamp = msg->m_vacuumtime;
-	tabentry->n_live_tuples = msg->m_tuples;
+	if (msg->m_scanned_all)
+		tabentry->n_live_tuples = msg->m_tuples;
 	/* Resetting dead_tuples to 0 is an approximation ... */
 	tabentry->n_dead_tuples = 0;
 	if (msg->m_analyze)
 	{
-		tabentry->last_anl_tuples = msg->m_tuples;
+		if (msg->m_scanned_all)
+			tabentry->last_anl_tuples = msg->m_tuples;
 		if (msg->m_autovacuum)
 			tabentry->autovac_analyze_timestamp = msg->m_vacuumtime;
 		else
@@ -3780,7 +3783,7 @@ pgstat_recv_vacuum(PgStat_MsgVacuum *msg, int len)
 	{
 		/* last_anl_tuples must never exceed n_live_tuples+n_dead_tuples */
 		tabentry->last_anl_tuples = Min(tabentry->last_anl_tuples,
-										msg->m_tuples);
+										tabentry->n_live_tuples);
 	}
 }
 
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
