• Tom Lane's avatar
    Remove AtEOXact_CatCache(). · 004a9702
    Tom Lane authored
    The sole useful effect of this function, to check that no catcache
    entries have positive refcounts at transaction end, has really been
    obsolete since we introduced ResourceOwners in PG 8.1.  We reduced the
    checks to assertions years ago, so that the function was a complete
    no-op in production builds.  There have been previous discussions about
    removing it entirely, but consensus up to now was that it had some small
    value as a cross-check for bugs in the ResourceOwner logic.
    
    However, it now emerges that it's possible to trigger these assertions
    if you hit an assert-enabled backend with SIGTERM during a call to
    SearchCatCacheList, because that function temporarily increases the
    refcounts of entries it's intending to add to a catcache list construct.
    In a normal ERROR scenario, the extra refcounts are cleaned up by
    SearchCatCacheList's PG_CATCH block; but in a FATAL exit we do a
    transaction abort and exit without ever executing PG_CATCH handlers.
    
    There's a case to be made that this is a generic hazard and we should
    consider restructuring elog(FATAL) handling so that pending PG_CATCH
    handlers do get run.  That's pretty scary though: it could easily create
    more problems than it solves.  Preliminary stress testing by Andreas
    Seltenreich 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 principled
    one would be to use PG_ENSURE_ERROR_CLEANUP instead of plain PG_TRY.
    But adding cycles to SearchCatCacheList isn't very appealing.  We could
    also weaken the assertions in AtEOXact_CatCache in some more or less
    ad-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 remove
    AtEOXact_CatCache altogether, on the grounds that it's not worth trying
    to 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
    004a9702
catcache.c 48.7 KB