Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitbb11ff2

Browse files
committed
Remove AtEOXact_CatCache().
The sole useful effect of this function, to check that no catcacheentries have positive refcounts at transaction end, has really beenobsolete since we introduced ResourceOwners in PG 8.1. We reduced thechecks to assertions years ago, so that the function was a completeno-op in production builds. There have been previous discussions aboutremoving it entirely, but consensus up to now was that it had some smallvalue as a cross-check for bugs in the ResourceOwner logic.However, it now emerges that it's possible to trigger these assertionsif you hit an assert-enabled backend with SIGTERM during a call toSearchCatCacheList, because that function temporarily increases therefcounts of entries it's intending to add to a catcache list construct.In a normal ERROR scenario, the extra refcounts are cleaned up bySearchCatCacheList's PG_CATCH block; but in a FATAL exit we do atransaction abort and exit without ever executing PG_CATCH handlers.There's a case to be made that this is a generic hazard and we shouldconsider restructuring elog(FATAL) handling so that pending PG_CATCHhandlers do get run. That's pretty scary though: it could easily createmore problems than it solves. Preliminary stress testing by AndreasSeltenreich suggests that there are not many live problems of this ilk,so we rejected that idea.There are more-localized ways to fix the problem; the most principledone would be to use PG_ENSURE_ERROR_CLEANUP instead of plain PG_TRY.But adding cycles to SearchCatCacheList isn't very appealing. We couldalso weaken the assertions in AtEOXact_CatCache in some more or lessad-hoc way, but that just makes its raison d'etre even less compelling.In the end, the most reasonable solution seems to be to just removeAtEOXact_CatCache altogether, on the grounds that it's not worth tryingto fix it. It hasn't found any bugs for us in many years.Per report from Jeevan Chalke. Back-patch to all supported branches.Discussion:https://postgr.es/m/CAM2+6=VEE30YtRQCZX7_sCFsEpoUkFBV1gZazL70fqLn8rcvBA@mail.gmail.com
1 parent06931a9 commitbb11ff2

File tree

3 files changed

+0
-60
lines changed

3 files changed

+0
-60
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1971,9 +1971,6 @@ CommitTransaction(void)
19711971
*/
19721972
smgrDoPendingDeletes(true);
19731973

1974-
/* Check we've released all catcache entries */
1975-
AtEOXact_CatCache(true);
1976-
19771974
AtCommit_Notify();
19781975
AtEOXact_GUC(true,1);
19791976
AtEOXact_SPI(true);
@@ -2238,9 +2235,6 @@ PrepareTransaction(void)
22382235
*/
22392236
PostPrepare_Twophase();
22402237

2241-
/* Check we've released all catcache entries */
2242-
AtEOXact_CatCache(true);
2243-
22442238
/* PREPARE acts the same as COMMIT as far as GUC is concerned */
22452239
AtEOXact_GUC(true,1);
22462240
AtEOXact_SPI(true);
@@ -2404,7 +2398,6 @@ AbortTransaction(void)
24042398
RESOURCE_RELEASE_AFTER_LOCKS,
24052399
false, true);
24062400
smgrDoPendingDeletes(false);
2407-
AtEOXact_CatCache(false);
24082401

24092402
AtEOXact_GUC(false,1);
24102403
AtEOXact_SPI(false);

‎src/backend/utils/cache/catcache.c

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -539,58 +539,6 @@ CreateCacheMemoryContext(void)
539539
}
540540

541541

542-
/*
543-
*AtEOXact_CatCache
544-
*
545-
* Clean up catcaches at end of main transaction (either commit or abort)
546-
*
547-
* As of PostgreSQL 8.1, catcache pins should get released by the
548-
* ResourceOwner mechanism. This routine is just a debugging
549-
* cross-check that no pins remain.
550-
*/
551-
void
552-
AtEOXact_CatCache(boolisCommit)
553-
{
554-
#ifdefUSE_ASSERT_CHECKING
555-
if (assert_enabled)
556-
{
557-
slist_itercache_iter;
558-
559-
slist_foreach(cache_iter,&CacheHdr->ch_caches)
560-
{
561-
CatCache*ccp=slist_container(CatCache,cc_next,cache_iter.cur);
562-
dlist_iteriter;
563-
inti;
564-
565-
/* Check CatCLists */
566-
dlist_foreach(iter,&ccp->cc_lists)
567-
{
568-
CatCList*cl=dlist_container(CatCList,cache_elem,iter.cur);
569-
570-
Assert(cl->cl_magic==CL_MAGIC);
571-
Assert(cl->refcount==0);
572-
Assert(!cl->dead);
573-
}
574-
575-
/* Check individual tuples */
576-
for (i=0;i<ccp->cc_nbuckets;i++)
577-
{
578-
dlist_head*bucket=&ccp->cc_bucket[i];
579-
580-
dlist_foreach(iter,bucket)
581-
{
582-
CatCTup*ct=dlist_container(CatCTup,cache_elem,iter.cur);
583-
584-
Assert(ct->ct_magic==CT_MAGIC);
585-
Assert(ct->refcount==0);
586-
Assert(!ct->dead);
587-
}
588-
}
589-
}
590-
}
591-
#endif
592-
}
593-
594542
/*
595543
*ResetCatalogCache
596544
*

‎src/include/utils/catcache.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ typedef struct catcacheheader
162162
externPGDLLIMPORTMemoryContextCacheMemoryContext;
163163

164164
externvoidCreateCacheMemoryContext(void);
165-
externvoidAtEOXact_CatCache(boolisCommit);
166165

167166
externCatCache*InitCatCache(intid,Oidreloid,Oidindexoid,
168167
intnkeys,constint*key,

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp