Modify AtEOXact_CatCache and AtEOXact_RelationCache to assume that the

ResourceOwner mechanism already released all reference counts for the
cache entries; therefore, we do not need to scan the catcache or relcache
at transaction end, unless we want to do it as a debugging crosscheck.
Do the crosscheck only in Assert mode.  This is the same logic we had
previously installed in AtEOXact_Buffers to avoid overhead with large
numbers of shared buffers.  I thought it'd be a good idea to do it here
too, in view of Kari Lavikka's recent report showing a real-world case
where AtEOXact_CatCache is taking a significant fraction of runtime.

src/backend/access/transam/xact.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.211 2005/07/25 22:12:31 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.212 2005/08/08 19:17:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1549,6 +1549,9 @@ CommitTransaction(void)
 	/* Check we've released all buffer pins */
 	AtEOXact_Buffers(true);
 
+	/* Clean up the relation cache */
+	AtEOXact_RelationCache(true);
+
 	/*
 	 * Make catalog changes visible to all backends.  This has to happen
 	 * after relcache references are dropped (see comments for
@@ -1576,6 +1579,9 @@ CommitTransaction(void)
 						 RESOURCE_RELEASE_AFTER_LOCKS,
 						 true, true);
 
+	/* Check we've released all catcache entries */
+	AtEOXact_CatCache(true);
+
 	AtEOXact_GUC(true, false);
 	AtEOXact_SPI(true);
 	AtEOXact_on_commit_actions(true);
@@ -1768,6 +1774,9 @@ PrepareTransaction(void)
 	/* Check we've released all buffer pins */
 	AtEOXact_Buffers(true);
 
+	/* Clean up the relation cache */
+	AtEOXact_RelationCache(true);
+
 	/* notify and flatfiles don't need a postprepare call */
 
 	PostPrepare_Inval();
@@ -1785,6 +1794,9 @@ PrepareTransaction(void)
 						 RESOURCE_RELEASE_AFTER_LOCKS,
 						 true, true);
 
+	/* Check we've released all catcache entries */
+	AtEOXact_CatCache(true);
+
 	/* PREPARE acts the same as COMMIT as far as GUC is concerned */
 	AtEOXact_GUC(true, false);
 	AtEOXact_SPI(true);
@@ -1922,6 +1934,7 @@ AbortTransaction(void)
 						 RESOURCE_RELEASE_BEFORE_LOCKS,
 						 false, true);
 	AtEOXact_Buffers(false);
+	AtEOXact_RelationCache(false);
 	AtEOXact_Inval(false);
 	smgrDoPendingDeletes(false);
 	AtEOXact_MultiXact();
@@ -1931,6 +1944,7 @@ AbortTransaction(void)
 	ResourceOwnerRelease(TopTransactionResourceOwner,
 						 RESOURCE_RELEASE_AFTER_LOCKS,
 						 false, true);
+	AtEOXact_CatCache(false);
 
 	AtEOXact_GUC(false, false);
 	AtEOXact_SPI(false);

src/backend/utils/cache/catcache.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/catcache.c,v 1.121 2005/05/06 17:24:54 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/catcache.c,v 1.122 2005/08/08 19:17:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -530,62 +530,43 @@ CreateCacheMemoryContext(void)
  *
  * Clean up catcaches at end of main transaction (either commit or abort)
  *
- * We scan the caches to reset refcounts to zero.  This is of course
- * necessary in the abort case, since elog() may have interrupted routines.
- * In the commit case, any nonzero counts indicate failure to call
- * ReleaseSysCache, so we put out a notice for debugging purposes.
+ * As of PostgreSQL 8.1, catcache pins should get released by the
+ * ResourceOwner mechanism.  This routine is just a debugging
+ * cross-check that no pins remain.
  */
 void
 AtEOXact_CatCache(bool isCommit)
 {
+#ifdef USE_ASSERT_CHECKING
+	if (assert_enabled)
+	{
 		CatCache   *ccp;
-	Dlelem	   *elt,
-			   *nextelt;
+		Dlelem	   *elt;
 
-	/*
-	 * First clean up CatCLists
-	 */
+		/* Check CatCLists */
 		for (ccp = CacheHdr->ch_caches; ccp; ccp = ccp->cc_next)
 		{
-		for (elt = DLGetHead(&ccp->cc_lists); elt; elt = nextelt)
+			for (elt = DLGetHead(&ccp->cc_lists); elt; elt = DLGetSucc(elt))
 			{
 				CatCList   *cl = (CatCList *) DLE_VAL(elt);
 
-			nextelt = DLGetSucc(elt);
-
-			if (cl->refcount != 0)
-			{
-				if (isCommit)
-					PrintCatCacheListLeakWarning(cl);
-				cl->refcount = 0;
-			}
-
-			/* Clean up any now-deletable dead entries */
-			if (cl->dead)
-				CatCacheRemoveCList(ccp, cl);
+				Assert(cl->cl_magic == CL_MAGIC);
+				Assert(cl->refcount == 0);
+				Assert(!cl->dead);
 			}
 		}
 
-	/*
-	 * Now clean up tuples; we can scan them all using the global LRU list
-	 */
-	for (elt = DLGetHead(&CacheHdr->ch_lrulist); elt; elt = nextelt)
+		/* Check individual tuples */
+		for (elt = DLGetHead(&CacheHdr->ch_lrulist); elt; elt = DLGetSucc(elt))
 		{
 			CatCTup    *ct = (CatCTup *) DLE_VAL(elt);
 
-		nextelt = DLGetSucc(elt);
-
-		if (ct->refcount != 0)
-		{
-			if (isCommit)
-				PrintCatCacheLeakWarning(&ct->tuple);
-			ct->refcount = 0;
+			Assert(ct->ct_magic == CT_MAGIC);
+			Assert(ct->refcount == 0);
+			Assert(!ct->dead);
 		}
-
-		/* Clean up any now-deletable dead entries */
-		if (ct->dead)
-			CatCacheRemoveCTup(ct->my_cache, ct);
 	}
+#endif
 }
 
 /*
@@ -1329,11 +1310,9 @@ SearchCatCacheList(CatCache *cache,
 	Dlelem	   *elt;
 	CatCList   *cl;
 	CatCTup    *ct;
-	List	   *ctlist;
+	List	   * volatile ctlist;
 	ListCell   *ctlist_item;
 	int			nmembers;
-	Relation	relation;
-	SysScanDesc scandesc;
 	bool		ordered;
 	HeapTuple	ntp;
 	MemoryContext oldcxt;
@@ -1433,7 +1412,21 @@ SearchCatCacheList(CatCache *cache,
 	 * List was not found in cache, so we have to build it by reading the
 	 * relation.  For each matching tuple found in the relation, use an
 	 * existing cache entry if possible, else build a new one.
+	 *
+	 * We have to bump the member refcounts immediately to ensure they
+	 * won't get dropped from the cache while loading other members.
+	 * We use a PG_TRY block to ensure we can undo those refcounts if
+	 * we get an error before we finish constructing the CatCList.
 	 */
+	ResourceOwnerEnlargeCatCacheListRefs(CurrentResourceOwner);
+
+	ctlist = NIL;
+
+	PG_TRY();
+	{
+		Relation	relation;
+		SysScanDesc scandesc;
+
 		relation = heap_open(cache->cc_reloid, AccessShareLock);
 
 		scandesc = systable_beginscan(relation,
@@ -1446,9 +1439,6 @@ SearchCatCacheList(CatCache *cache,
 		/* The list will be ordered iff we are doing an index scan */
 		ordered = (scandesc->irel != NULL);
 
-	ctlist = NIL;
-	nmembers = 0;
-
 		while (HeapTupleIsValid(ntp = systable_getnext(scandesc)))
 		{
 			uint32		hashValue;
@@ -1497,18 +1487,9 @@ SearchCatCacheList(CatCache *cache,
 											 false);
 			}
 
-		/*
-		 * We have to bump the member refcounts immediately to ensure they
-		 * won't get dropped from the cache while loading other members.
-		 * If we get an error before we finish constructing the CatCList
-		 * then we will leak those reference counts.  This is annoying but
-		 * it has no real consequence beyond possibly generating some
-		 * warning messages at the next transaction commit, so it's not
-		 * worth fixing.
-		 */
-		ct->refcount++;
+			/* Careful here: add entry to ctlist, then bump its refcount */
 			ctlist = lappend(ctlist, ct);
-		nmembers++;
+			ct->refcount++;
 		}
 
 		systable_endscan(scandesc);
@@ -1521,11 +1502,42 @@ SearchCatCacheList(CatCache *cache,
 		 */
 		ntp = build_dummy_tuple(cache, nkeys, cur_skey);
 		oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
-	cl = (CatCList *) palloc(sizeof(CatCList) + nmembers * sizeof(CatCTup *));
+		nmembers = list_length(ctlist);
+		cl = (CatCList *)
+			palloc(sizeof(CatCList) + nmembers * sizeof(CatCTup *));
 		heap_copytuple_with_tuple(ntp, &cl->tuple);
 		MemoryContextSwitchTo(oldcxt);
 		heap_freetuple(ntp);
 
+		/*
+		 * We are now past the last thing that could trigger an elog before
+		 * we have finished building the CatCList and remembering it in the
+		 * resource owner.  So it's OK to fall out of the PG_TRY, and indeed
+		 * we'd better do so before we start marking the members as belonging
+		 * to the list.
+		 */
+
+	}
+	PG_CATCH();
+	{
+		foreach(ctlist_item, ctlist)
+		{
+			ct = (CatCTup *) lfirst(ctlist_item);
+			Assert(ct->c_list == NULL);
+			Assert(ct->refcount > 0);
+			ct->refcount--;
+			if (ct->refcount == 0
+#ifndef CATCACHE_FORCE_RELEASE
+				&& ct->dead
+#endif
+				)
+				CatCacheRemoveCTup(cache, ct);
+		}
+
+		PG_RE_THROW();
+	}
+	PG_END_TRY();
+
 	cl->cl_magic = CL_MAGIC;
 	cl->my_cache = cache;
 	DLInitElem(&cl->cache_elem, cl);
@@ -1536,29 +1548,27 @@ SearchCatCacheList(CatCache *cache,
 	cl->hash_value = lHashValue;
 	cl->n_members = nmembers;
 
-	Assert(nmembers == list_length(ctlist));
-	ctlist_item = list_head(ctlist);
-	for (i = 0; i < nmembers; i++)
+	i = 0;
+	foreach(ctlist_item, ctlist)
 	{
-		cl->members[i] = ct = (CatCTup *) lfirst(ctlist_item);
+		cl->members[i++] = ct = (CatCTup *) lfirst(ctlist_item);
 		Assert(ct->c_list == NULL);
 		ct->c_list = cl;
 		/* mark list dead if any members already dead */
 		if (ct->dead)
 			cl->dead = true;
-		ctlist_item = lnext(ctlist_item);
 	}
+	Assert(i == nmembers);
 
 	DLAddHead(&cache->cc_lists, &cl->cache_elem);
 
-	CACHE3_elog(DEBUG2, "SearchCatCacheList(%s): made list of %d members",
-				cache->cc_relname, nmembers);
-
 	/* Finally, bump the list's refcount and return it */
-	ResourceOwnerEnlargeCatCacheListRefs(CurrentResourceOwner);
 	cl->refcount++;
 	ResourceOwnerRememberCatCacheListRef(CurrentResourceOwner, cl);
 
+	CACHE3_elog(DEBUG2, "SearchCatCacheList(%s): made list of %d members",
+				cache->cc_relname, nmembers);
+
 	return cl;
 }
 

src/backend/utils/cache/relcache.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.225 2005/05/29 04:23:05 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.226 2005/08/08 19:17:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -122,9 +122,9 @@ static long relcacheInvalsReceived = 0L;
 static List *initFileRelationIds = NIL;
 
 /*
- * This flag lets us optimize away work in AtEOSubXact_RelationCache().
+ * This flag lets us optimize away work in AtEO(Sub)Xact_RelationCache().
  */
-static bool need_eosubxact_work = false;
+static bool need_eoxact_work = false;
 
 
 /*
@@ -1816,6 +1816,12 @@ RelationCacheInvalidate(void)
  * In the case of abort, we don't want to try to rebuild any invalidated
  * cache entries (since we can't safely do database accesses).  Therefore
  * we must reset refcnts before handling pending invalidations.
+ *
+ * As of PostgreSQL 8.1, relcache refcnts should get released by the
+ * ResourceOwner mechanism.  This routine just does a debugging
+ * cross-check that no pins remain.  However, we also need to do special
+ * cleanup when the current transaction created any relations or made use
+ * of forced index lists.
  */
 void
 AtEOXact_RelationCache(bool isCommit)
@@ -1823,13 +1829,48 @@ AtEOXact_RelationCache(bool isCommit)
 	HASH_SEQ_STATUS status;
 	RelIdCacheEnt *idhentry;
 
+	/*
+	 * To speed up transaction exit, we want to avoid scanning the relcache
+	 * unless there is actually something for this routine to do.  Other
+	 * than the debug-only Assert checks, most transactions don't create
+	 * any work for us to do here, so we keep a static flag that gets set
+	 * if there is anything to do.  (Currently, this means either a relation
+	 * is created in the current xact, or an index list is forced.)  For
+	 * simplicity, the flag remains set till end of top-level transaction,
+	 * even though we could clear it at subtransaction end in some cases.
+	 */
+	if (!need_eoxact_work
+#ifdef USE_ASSERT_CHECKING
+		&& !assert_enabled
+#endif
+		)
+		return;
+
 	hash_seq_init(&status, RelationIdCache);
 
 	while ((idhentry = (RelIdCacheEnt *) hash_seq_search(&status)) != NULL)
 	{
 		Relation	relation = idhentry->reldesc;
+
+		/*
+		 * The relcache entry's ref count should be back to its normal
+		 * not-in-a-transaction state: 0 unless it's nailed in cache.
+		 *
+		 * In bootstrap mode, this is NOT true, so don't check it ---
+		 * the bootstrap code expects relations to stay open across
+		 * start/commit transaction calls.  (That seems bogus, but it's
+		 * not worth fixing.)
+		 */
+#ifdef USE_ASSERT_CHECKING
+		if (!IsBootstrapProcessingMode())
+		{
 			int			expected_refcnt;
 
+			expected_refcnt = relation->rd_isnailed ? 1 : 0;
+			Assert(relation->rd_refcnt == expected_refcnt);
+		}
+#endif
+
 		/*
 		 * Is it a relation created in the current transaction?
 		 *
@@ -1851,40 +1892,6 @@ AtEOXact_RelationCache(bool isCommit)
 			}
 		}
 
-		/*
-		 * During transaction abort, we must also reset relcache entry ref
-		 * counts to their normal not-in-a-transaction state.  A ref count
-		 * may be too high because some routine was exited by ereport()
-		 * between incrementing and decrementing the count.
-		 *
-		 * During commit, we should not have to do this, but it's still
-		 * useful to check that the counts are correct to catch missed
-		 * relcache closes.
-		 *
-		 * In bootstrap mode, do NOT reset the refcnt nor complain that it's
-		 * nonzero --- the bootstrap code expects relations to stay open
-		 * across start/commit transaction calls.  (That seems bogus, but
-		 * it's not worth fixing.)
-		 */
-		expected_refcnt = relation->rd_isnailed ? 1 : 0;
-
-		if (isCommit)
-		{
-			if (relation->rd_refcnt != expected_refcnt &&
-				!IsBootstrapProcessingMode())
-			{
-				elog(WARNING, "relcache reference leak: relation \"%s\" has refcnt %d instead of %d",
-					 RelationGetRelationName(relation),
-					 relation->rd_refcnt, expected_refcnt);
-				relation->rd_refcnt = expected_refcnt;
-			}
-		}
-		else
-		{
-			/* abort case, just reset it quietly */
-			relation->rd_refcnt = expected_refcnt;
-		}
-
 		/*
 		 * Flush any temporary index list.
 		 */
@@ -1896,8 +1903,8 @@ AtEOXact_RelationCache(bool isCommit)
 		}
 	}
 
-	/* Once done with the transaction, we can reset need_eosubxact_work */
-	need_eosubxact_work = false;
+	/* Once done with the transaction, we can reset need_eoxact_work */
+	need_eoxact_work = false;
 }
 
 /*
@@ -1915,18 +1922,10 @@ AtEOSubXact_RelationCache(bool isCommit, SubTransactionId mySubid,
 	RelIdCacheEnt *idhentry;
 
 	/*
-	 * In the majority of subtransactions there is not anything for this
-	 * routine to do, and since there are usually many entries in the
-	 * relcache, uselessly scanning the cache represents a surprisingly
-	 * large fraction of the subtransaction entry/exit overhead.  To avoid
-	 * this, we keep a static flag that must be set whenever a condition
-	 * is created that requires subtransaction-end work.  (Currently, this
-	 * means either a relation is created in the current xact, or an index
-	 * list is forced.)  For simplicity, the flag remains set till end of
-	 * top-level transaction, even though we could clear it earlier in some
-	 * cases.
+	 * Skip the relcache scan if nothing to do --- see notes for
+	 * AtEOXact_RelationCache.
 	 */
-	if (!need_eosubxact_work)
+	if (!need_eoxact_work)
 		return;
 
 	hash_seq_init(&status, RelationIdCache);
@@ -2032,7 +2031,7 @@ RelationBuildLocalRelation(const char *relname,
 	rel->rd_createSubid = GetCurrentSubTransactionId();
 
 	/* must flag that we have rels created in this transaction */
-	need_eosubxact_work = true;
+	need_eoxact_work = true;
 
 	/* is it a temporary relation? */
 	rel->rd_istemp = isTempNamespace(relnamespace);
@@ -2626,7 +2625,7 @@ RelationSetIndexList(Relation relation, List *indexIds)
 	relation->rd_indexlist = indexIds;
 	relation->rd_indexvalid = 2;	/* mark list as forced */
 	/* must flag that we have a forced index list */
-	need_eosubxact_work = true;
+	need_eoxact_work = true;
 }
 
 /*

src/backend/utils/resowner/resowner.c

@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.12 2005/04/06 04:34:22 neilc Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/resowner/resowner.c,v 1.13 2005/08/08 19:17:23 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -213,25 +213,13 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 			ReleaseBuffer(owner->buffers[owner->nbuffers - 1]);
 		}
 
-		/* Release relcache references */
-		if (isTopLevel)
-		{
 		/*
-			 * For a top-level xact we are going to release all
-			 * references, so just do a single relcache call at the top of
-			 * the recursion.
-			 */
-			if (owner == TopTransactionResourceOwner)
-				AtEOXact_RelationCache(isCommit);
-			/* Mark object as owning no relrefs, just for sanity */
-			owner->nrelrefs = 0;
-		}
-		else
-		{
-			/*
-			 * Release relcache refs retail.  Note that RelationClose will
+		 * Release relcache references.  Note that RelationClose will
 		 * remove the relref entry from my list, so I just have to
 		 * iterate till there are none.
+		 *
+		 * As with buffer pins, warn if any are left at commit time,
+		 * and release back-to-front for speed.
 		 */
 		while (owner->nrelrefs > 0)
 		{
@@ -240,7 +228,6 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 			RelationClose(owner->relrefs[owner->nrelrefs - 1]);
 		}
 	}
-	}
 	else if (phase == RESOURCE_RELEASE_LOCKS)
 	{
 		if (isTopLevel)
@@ -268,27 +255,14 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 		}
 	}
 	else if (phase == RESOURCE_RELEASE_AFTER_LOCKS)
-	{
-		/* Release catcache references */
-		if (isTopLevel)
-		{
-			/*
-			 * For a top-level xact we are going to release all
-			 * references, so just do a single catcache call at the top of
-			 * the recursion.
-			 */
-			if (owner == TopTransactionResourceOwner)
-				AtEOXact_CatCache(isCommit);
-			/* Mark object as owning no catrefs, just for sanity */
-			owner->ncatrefs = 0;
-			owner->ncatlistrefs = 0;
-		}
-		else
 	{
 		/*
-			 * Release catcache refs retail.  Note that ReleaseCatCache
+		 * Release catcache references.  Note that ReleaseCatCache
 		 * will remove the catref entry from my list, so I just have
 		 * to iterate till there are none.	Ditto for catcache lists.
+		 *
+		 * As with buffer pins, warn if any are left at commit time,
+		 * and release back-to-front for speed.
 		 */
 		while (owner->ncatrefs > 0)
 		{
@@ -302,7 +276,7 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
 				PrintCatCacheListLeakWarning(owner->catlistrefs[owner->ncatlistrefs - 1]);
 			ReleaseCatCacheList(owner->catlistrefs[owner->ncatlistrefs - 1]);
 		}
-		}
+
 		/* Clean up index scans too */
 		ReleaseResources_gist();
 		ReleaseResources_hash();