From e6acb258fb831253dbc452a84b1b5a8381dc293c Mon Sep 17 00:00:00 2001 From: John Naylor Date: Sun, 8 Aug 2021 10:44:13 -0400 Subject: [PATCH v1 1/6] Remove "dead" flag from CatCTup We don't want to have to compare to this anymore, since that would require including it in the upcoming L1 area of the bucket cache. Remove, and where necessary, replace with setting or comparing ct->cache_elem.prev to NULL. Extracted from a larger patch by Kyotaro Horiguchi, per suggestion from Heikki Linnakangas Discussion: https://www.postgresql.org/message-id/0b58c41e-4fbc-c73d-d293-a35e4d8223f7%40iki.fi --- src/backend/utils/cache/catcache.c | 45 +++++++++++++++--------------- src/include/utils/catcache.h | 7 ----- 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c index 4fbdc62d8c..47fb14e1db 100644 --- a/src/backend/utils/cache/catcache.c +++ b/src/backend/utils/cache/catcache.c @@ -459,21 +459,24 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct) Assert(ct->refcount == 0); Assert(ct->my_cache == cache); + /* delink from linked list if we haven't already */ + if (ct->cache_elem.prev) + { + dlist_delete(&ct->cache_elem); + ct->cache_elem.prev = NULL; + } + if (ct->c_list) { /* * The cleanest way to handle this is to call CatCacheRemoveCList, * which will recurse back to me, and the recursive call will do the - * work. Set the "dead" flag to make sure it does recurse. + * work. */ - ct->dead = true; CatCacheRemoveCList(cache, ct->c_list); return; /* nothing left to do */ } - /* delink from linked list */ - dlist_delete(&ct->cache_elem); - /* * Free keys when we're dealing with a negative entry, normal entries just * point into tuple, allocated together with the CatCTup. @@ -493,7 +496,7 @@ CatCacheRemoveCTup(CatCache *cache, CatCTup *ct) * * Unlink and delete the given cache list entry * - * NB: any dead member entries that become unreferenced are deleted too. + * NB: any delinked member entries that become unreferenced are deleted too. */ static void CatCacheRemoveCList(CatCache *cache, CatCList *cl) @@ -510,10 +513,11 @@ CatCacheRemoveCList(CatCache *cache, CatCList *cl) Assert(ct->c_list == cl); ct->c_list = NULL; - /* if the member is dead and now has no references, remove it */ + + /* if the member is delinked and now has no references, remove it */ if ( #ifndef CATCACHE_FORCE_RELEASE - ct->dead && + ct->cache_elem.prev == NULL && #endif ct->refcount == 0) CatCacheRemoveCTup(cache, ct); @@ -588,7 +592,9 @@ CatCacheInvalidate(CatCache *cache, uint32 hashValue) if (ct->refcount > 0 || (ct->c_list && ct->c_list->refcount > 0)) { - ct->dead = true; + dlist_delete(&ct->cache_elem); + ct->cache_elem.prev = NULL; + /* list, if any, was marked dead above */ Assert(ct->c_list == NULL || ct->c_list->dead); } @@ -667,7 +673,8 @@ ResetCatalogCache(CatCache *cache) if (ct->refcount > 0 || (ct->c_list && ct->c_list->refcount > 0)) { - ct->dead = true; + dlist_delete(&ct->cache_elem); + ct->cache_elem.prev = NULL; /* list, if any, was marked dead above */ Assert(ct->c_list == NULL || ct->c_list->dead); } @@ -1249,9 +1256,6 @@ SearchCatCacheInternal(CatCache *cache, { ct = dlist_container(CatCTup, cache_elem, iter.cur); - if (ct->dead) - continue; /* ignore dead entries */ - if (ct->hash_value != hashValue) continue; /* quickly skip entry if wrong hash val */ @@ -1453,7 +1457,7 @@ ReleaseCatCache(HeapTuple tuple) if ( #ifndef CATCACHE_FORCE_RELEASE - ct->dead && + ct->cache_elem.prev == NULL && #endif ct->refcount == 0 && (ct->c_list == NULL || ct->c_list->refcount == 0)) @@ -1660,8 +1664,8 @@ SearchCatCacheList(CatCache *cache, { ct = dlist_container(CatCTup, cache_elem, iter.cur); - if (ct->dead || ct->negative) - continue; /* ignore dead and negative entries */ + if (ct->negative) + continue; /* ignore negative entries */ if (ct->hash_value != hashValue) continue; /* quickly skip entry if wrong hash val */ @@ -1722,14 +1726,13 @@ SearchCatCacheList(CatCache *cache, { foreach(ctlist_item, ctlist) { + Assert(ct->cache_elem.prev != NULL); + ct = (CatCTup *) lfirst(ctlist_item); Assert(ct->c_list == NULL); Assert(ct->refcount > 0); ct->refcount--; if ( -#ifndef CATCACHE_FORCE_RELEASE - ct->dead && -#endif ct->refcount == 0 && (ct->c_list == NULL || ct->c_list->refcount == 0)) CatCacheRemoveCTup(cache, ct); @@ -1757,9 +1760,6 @@ SearchCatCacheList(CatCache *cache, /* release the temporary refcount on the member */ Assert(ct->refcount > 0); ct->refcount--; - /* mark list dead if any members already dead */ - if (ct->dead) - cl->dead = true; } Assert(i == nmembers); @@ -1887,7 +1887,6 @@ CatalogCacheCreateEntry(CatCache *cache, HeapTuple ntp, Datum *arguments, ct->my_cache = cache; ct->c_list = NULL; ct->refcount = 0; /* for the moment */ - ct->dead = false; ct->negative = negative; ct->hash_value = hashValue; diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h index ddc2762eb3..eac6a83f29 100644 --- a/src/include/utils/catcache.h +++ b/src/include/utils/catcache.h @@ -104,19 +104,12 @@ typedef struct catctup dlist_node cache_elem; /* list member of per-bucket list */ /* - * A tuple marked "dead" must not be returned by subsequent searches. - * However, it won't be physically deleted from the cache until its - * refcount goes to zero. (If it's a member of a CatCList, the list's - * refcount must go to zero, too; also, remember to mark the list dead at - * the same time the tuple is marked.) - * * A negative cache entry is an assertion that there is no tuple matching * a particular key. This is just as useful as a normal entry so far as * avoiding catalog searches is concerned. Management of positive and * negative entries is identical. */ int refcount; /* number of active references */ - bool dead; /* dead but not yet removed? */ bool negative; /* negative cache entry? */ HeapTupleData tuple; /* tuple management header */ -- 2.31.1