Fix up pgstats counting of live and dead tuples to recognize that com…

…mitted and aborted transactions have different effects; also teach it not to assume that prepared transactions are always committed. Along the way, simplify the pgstats API by tying counting directly to Relations; I cannot detect any redeeming social value in having stats pointers in HeapScanDesc and IndexScanDesc structures. And fix a few corner cases in which counts might be missed because the relation's pgstat_info pointer hadn't been set.
amazon-aurora · May 27, 2007 · 77947c5 · 77947c5
1 parent cadb783
commit 77947c5
Show file tree

Hide file tree

Showing 20 changed files with 802 additions and 414 deletions.
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *			$PostgreSQL: pgsql/src/backend/access/gin/ginscan.c,v 1.9 2007/01/31 15:09:45 teodor Exp $
+ *			$PostgreSQL: pgsql/src/backend/access/gin/ginscan.c,v 1.10 2007/05/27 03:50:38 tgl Exp $
  *-------------------------------------------------------------------------
  */
 
@@ -189,7 +189,7 @@ newScanKey(IndexScanDesc scan)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("GIN index does not support search with void query")));
 
-	pgstat_count_index_scan(&scan->xs_pgstat_info);
+	pgstat_count_index_scan(scan->indexRelation);
 }
 
 Datum

diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.65 2007/04/06 22:33:41 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/gist/gistget.c,v 1.66 2007/05/27 03:50:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -165,7 +165,7 @@ gistnext(IndexScanDesc scan, ScanDirection dir, ItemPointer tids,
 		stk->next = NULL;
 		stk->block = GIST_ROOT_BLKNO;
 
-		pgstat_count_index_scan(&scan->xs_pgstat_info);
+		pgstat_count_index_scan(scan->indexRelation);
 	}
 	else if (so->curbuf == InvalidBuffer)
 	{

diff --git a/src/backend/access/hash/hashsearch.c b/src/backend/access/hash/hashsearch.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/hash/hashsearch.c,v 1.49 2007/05/03 16:45:58 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/hash/hashsearch.c,v 1.50 2007/05/27 03:50:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -127,7 +127,7 @@ _hash_first(IndexScanDesc scan, ScanDirection dir)
 	ItemPointer current;
 	OffsetNumber offnum;
 
-	pgstat_count_index_scan(&scan->xs_pgstat_info);
+	pgstat_count_index_scan(rel);
 
 	current = &(so->hashso_curpos);
 	ItemPointerSetInvalid(current);

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.232 2007/04/08 01:26:27 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.233 2007/05/27 03:50:38 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -100,7 +100,7 @@ initscan(HeapScanDesc scan, ScanKey key)
 	if (key != NULL)
 		memcpy(scan->rs_key, key, scan->rs_nkeys * sizeof(ScanKeyData));
 
-	pgstat_count_heap_scan(&scan->rs_pgstat_info);
+	pgstat_count_heap_scan(scan->rs_rd);
 }
 
 /*
@@ -701,6 +701,8 @@ relation_open(Oid relationId, LOCKMODE lockmode)
 	if (!RelationIsValid(r))
 		elog(ERROR, "could not open relation with OID %u", relationId);
 
+	pgstat_initstats(r);
+
 	return r;
 }
 
@@ -743,6 +745,8 @@ try_relation_open(Oid relationId, LOCKMODE lockmode)
 	if (!RelationIsValid(r))
 		elog(ERROR, "could not open relation with OID %u", relationId);
 
+	pgstat_initstats(r);
+
 	return r;
 }
 
@@ -787,6 +791,8 @@ relation_open_nowait(Oid relationId, LOCKMODE lockmode)
 	if (!RelationIsValid(r))
 		elog(ERROR, "could not open relation with OID %u", relationId);
 
+	pgstat_initstats(r);
+
 	return r;
 }
 
@@ -873,8 +879,6 @@ heap_open(Oid relationId, LOCKMODE lockmode)
 				 errmsg("\"%s\" is a composite type",
 						RelationGetRelationName(r))));
 
-	pgstat_initstats(&r->pgstat_info, r);
-
 	return r;
 }
 
@@ -903,8 +907,6 @@ heap_openrv(const RangeVar *relation, LOCKMODE lockmode)
 				 errmsg("\"%s\" is a composite type",
 						RelationGetRelationName(r))));
 
-	pgstat_initstats(&r->pgstat_info, r);
-
 	return r;
 }
 
@@ -954,8 +956,6 @@ heap_beginscan(Relation relation, Snapshot snapshot,
 	else
 		scan->rs_key = NULL;
 
-	pgstat_initstats(&scan->rs_pgstat_info, relation);
-
 	initscan(scan, key);
 
 	return scan;
@@ -1059,7 +1059,7 @@ heap_getnext(HeapScanDesc scan, ScanDirection direction)
 	 */
 	HEAPDEBUG_3;				/* heap_getnext returning tuple */
 
-	pgstat_count_heap_getnext(&scan->rs_pgstat_info);
+	pgstat_count_heap_getnext(scan->rs_rd);
 
 	return &(scan->rs_ctup);
 }
@@ -1086,6 +1086,10 @@ heap_getnext(HeapScanDesc scan, ScanDirection direction)
  * and return it in *userbuf (so the caller must eventually unpin it); when
  * keep_buf = false, the pin is released and *userbuf is set to InvalidBuffer.
  *
+ * stats_relation is the relation to charge the heap_fetch operation against
+ * for statistical purposes.  (This could be the heap rel itself, an
+ * associated index, or NULL to not count the fetch at all.)
+ *
  * It is somewhat inconsistent that we ereport() on invalid block number but
  * return false on invalid item number.  There are a couple of reasons though.
  * One is that the caller can relatively easily check the block number for
@@ -1101,12 +1105,12 @@ heap_fetch(Relation relation,
 		   HeapTuple tuple,
 		   Buffer *userbuf,
 		   bool keep_buf,
-		   PgStat_Info *pgstat_info)
+		   Relation stats_relation)
 {
 	/* Assume *userbuf is undefined on entry */
 	*userbuf = InvalidBuffer;
 	return heap_release_fetch(relation, snapshot, tuple,
-							  userbuf, keep_buf, pgstat_info);
+							  userbuf, keep_buf, stats_relation);
 }
 
 /*
@@ -1125,7 +1129,7 @@ heap_release_fetch(Relation relation,
 				   HeapTuple tuple,
 				   Buffer *userbuf,
 				   bool keep_buf,
-				   PgStat_Info *pgstat_info)
+				   Relation stats_relation)
 {
 	ItemPointer tid = &(tuple->t_self);
 	ItemId		lp;
@@ -1210,9 +1214,9 @@ heap_release_fetch(Relation relation,
 		 */
 		*userbuf = buffer;
 
-		/* Count the successful fetch in *pgstat_info, if given. */
-		if (pgstat_info != NULL)
-			pgstat_count_heap_fetch(pgstat_info);
+		/* Count the successful fetch against appropriate rel, if any */
+		if (stats_relation != NULL)
+			pgstat_count_heap_fetch(stats_relation);
 
 		return true;
 	}
@@ -1517,7 +1521,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
 	 */
 	CacheInvalidateHeapTuple(relation, heaptup);
 
-	pgstat_count_heap_insert(&relation->pgstat_info);
+	pgstat_count_heap_insert(relation);
 
 	/*
 	 * If heaptup is a private copy, release it.  Don't forget to copy t_self
@@ -1807,7 +1811,7 @@ heap_delete(Relation relation, ItemPointer tid,
 	if (have_tuple_lock)
 		UnlockTuple(relation, &(tp.t_self), ExclusiveLock);
 
-	pgstat_count_heap_delete(&relation->pgstat_info);
+	pgstat_count_heap_delete(relation);
 
 	return HeapTupleMayBeUpdated;
 }
@@ -2269,7 +2273,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	if (have_tuple_lock)
 		UnlockTuple(relation, &(oldtup.t_self), ExclusiveLock);
 
-	pgstat_count_heap_update(&relation->pgstat_info);
+	pgstat_count_heap_update(relation);
 
 	/*
 	 * If heaptup is a private copy, release it.  Don't forget to copy t_self

diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.61 2007/01/20 18:43:35 neilc Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/index/genam.c,v 1.62 2007/05/27 03:50:38 tgl Exp $
  *
  * NOTES
  *	  many of the old access method routines have been turned into
@@ -96,8 +96,6 @@ RelationGetIndexScan(Relation indexRelation,
 	scan->xs_ctup.t_data = NULL;
 	scan->xs_cbuf = InvalidBuffer;
 
-	pgstat_initstats(&scan->xs_pgstat_info, indexRelation);
-
 	/*
 	 * Let the AM fill in the key and any opaque data it wants.
 	 */

diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.97 2007/01/05 22:19:23 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/index/indexam.c,v 1.98 2007/05/27 03:50:38 tgl Exp $
  *
  * INTERFACE ROUTINES
  *		index_open		- open an index relation by relation OID
@@ -145,8 +145,6 @@ index_open(Oid relationId, LOCKMODE lockmode)
 				 errmsg("\"%s\" is not an index",
 						RelationGetRelationName(r))));
 
-	pgstat_initstats(&r->pgstat_info, r);
-
 	return r;
 }
 
@@ -433,14 +431,14 @@ index_getnext(IndexScanDesc scan, ScanDirection direction)
 			return NULL;		/* failure exit */
 		}
 
-		pgstat_count_index_tuples(&scan->xs_pgstat_info, 1);
+		pgstat_count_index_tuples(scan->indexRelation, 1);
 
 		/*
 		 * Fetch the heap tuple and see if it matches the snapshot.
 		 */
 		if (heap_release_fetch(scan->heapRelation, scan->xs_snapshot,
 							   heapTuple, &scan->xs_cbuf, true,
-							   &scan->xs_pgstat_info))
+							   scan->indexRelation))
 			break;
 
 		/* Skip if no undeleted tuple at this location */
@@ -502,7 +500,7 @@ index_getnext_indexitem(IndexScanDesc scan,
 									   Int32GetDatum(direction)));
 
 	if (found)
-		pgstat_count_index_tuples(&scan->xs_pgstat_info, 1);
+		pgstat_count_index_tuples(scan->indexRelation, 1);
 
 	return found;
 }
@@ -543,7 +541,7 @@ index_getmulti(IndexScanDesc scan,
 									   Int32GetDatum(max_tids),
 									   PointerGetDatum(returned_tids)));
 
-	pgstat_count_index_tuples(&scan->xs_pgstat_info, *returned_tids);
+	pgstat_count_index_tuples(scan->indexRelation, *returned_tids);
 
 	return found;
 }

diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtsearch.c,v 1.112 2007/04/06 22:33:42 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtsearch.c,v 1.113 2007/05/27 03:50:39 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -453,7 +453,7 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
 	int			i;
 	StrategyNumber strat_total;
 
-	pgstat_count_index_scan(&scan->xs_pgstat_info);
+	pgstat_count_index_scan(rel);
 
 	/*
 	 * Examine the scan keys and eliminate any redundant keys; also mark the

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *		$PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.30 2007/04/30 21:01:52 tgl Exp $
+ *		$PostgreSQL: pgsql/src/backend/access/transam/twophase.c,v 1.31 2007/05/27 03:50:39 tgl Exp $
  *
  * NOTES
  *		Each global transaction is associated with a global transaction
@@ -1211,7 +1211,8 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
 	else
 		ProcessRecords(bufptr, xid, twophase_postabort_callbacks);
 
-	pgstat_count_xact_commit();
+	/* Count the prepared xact as committed or aborted */
+	AtEOXact_PgStat(isCommit);
 
 	/*
 	 * And now we can clean up our mess.

diff --git a/src/backend/access/transam/twophase_rmgr.c b/src/backend/access/transam/twophase_rmgr.c
@@ -8,14 +8,15 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/transam/twophase_rmgr.c,v 1.4 2007/01/05 22:19:23 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/transam/twophase_rmgr.c,v 1.5 2007/05/27 03:50:39 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
 #include "access/twophase_rmgr.h"
 #include "commands/async.h"
+#include "pgstat.h"
 #include "storage/lock.h"
 #include "utils/flatfiles.h"
 #include "utils/inval.h"
@@ -27,7 +28,8 @@ const TwoPhaseCallback twophase_recover_callbacks[TWOPHASE_RM_MAX_ID + 1] =
 	lock_twophase_recover,		/* Lock */
 	NULL,						/* Inval */
 	NULL,						/* flat file update */
-	NULL						/* notify/listen */
+	NULL,						/* notify/listen */
+	NULL						/* pgstat */
 };
 
 const TwoPhaseCallback twophase_postcommit_callbacks[TWOPHASE_RM_MAX_ID + 1] =
@@ -36,7 +38,8 @@ const TwoPhaseCallback twophase_postcommit_callbacks[TWOPHASE_RM_MAX_ID + 1] =
 	lock_twophase_postcommit,	/* Lock */
 	inval_twophase_postcommit,	/* Inval */
 	flatfile_twophase_postcommit,		/* flat file update */
-	notify_twophase_postcommit	/* notify/listen */
+	notify_twophase_postcommit,	/* notify/listen */
+	pgstat_twophase_postcommit	/* pgstat */
 };
 
 const TwoPhaseCallback twophase_postabort_callbacks[TWOPHASE_RM_MAX_ID + 1] =
@@ -45,5 +48,6 @@ const TwoPhaseCallback twophase_postabort_callbacks[TWOPHASE_RM_MAX_ID + 1] =
 	lock_twophase_postabort,	/* Lock */
 	NULL,						/* Inval */
 	NULL,						/* flat file update */
-	NULL						/* notify/listen */
+	NULL,						/* notify/listen */
+	pgstat_twophase_postabort	/* pgstat */
 };