Horton

Re: type cache cleanup improvements

Andrei Lepikhov <lepihov@gmail.com>

View thread
From: Andrei Lepikhov <lepihov@gmail.com>
To: Alexander Korotkov <aekorotkov@gmail.com>, Alexander Lakhin <exclusion@gmail.com>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>, Teodor Sigaev <teodor@sigaev.ru>, Pgsql Hackers <pgsql-hackers@postgresql.org>, Aleksander Alekseev <aleksander@timescale.com>, Tom Lane <tgl@sss.pgh.pa.us>, Michael Paquier <michael@paquier.xyz>
Date: 2024-08-26T06:37:50Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Maintain RelIdToTypeIdCacheHash in TypeCacheOpcCallback()

  2. Fix concurrrently in typcache_rel_type_cache.sql

  3. Avoid looping over all type cache entries in TypeCacheRelCallback()

  4. Update header comment for lookup_type_cache()

  5. Revert: Avoid looping over all type cache entries in TypeCacheRelCallback()

  6. Introduce hash_search_with_hash_value() function

  7. Optimize InvalidateAttoptCacheCallback() and TypeCacheTypCallback()

  8. Refactor initial hash lookup in dynahash.c

  9. Rationalize and improve error messages for some jsonpath items

  10. Avoid race in RelationBuildDesc() affecting CREATE INDEX CONCURRENTLY.

  11. Represent Lists as expansible arrays, not chains of cons-cells.

On 25/8/2024 23:22, Alexander Korotkov wrote:
> On Sun, Aug 25, 2024 at 10:21 PM Alexander Korotkov
>>> (This Assert is introduced by c14d4acb8.)
>>
>> Thank you for noticing.  I'm checking this.
> 
> I didn't take into account that TypeCacheEntry could be invalidated
> while lookup_type_cache() does syscache lookups.  When I realized that
> I was curious on how does it currently work.  It appears that type
> cache invalidation mostly only clears the flags while values are
> remaining in place and still available for lookup_type_cache() caller.
> TypeCacheEntry.tupDesc is invalidated directly, and it has guarantee
> to survive only because we don't do any syscache lookups for composite
> data types later in lookup_type_cache().  I'm becoming less fan of how
> this works...  I think these aspects needs to be at least documented
> in details.
> 
> Regarding c14d4acb8, it appears to require redesign.  I'm going to revert it.
Sorry, but I don't understand your point.
Let's refocus on the problem at hand. The issue arose when the 
TypeCacheTypCallback and the TypeCacheRelCallback were executed in 
sequence within InvalidateSystemCachesExtended.
The first callback cleaned the flags TCFLAGS_HAVE_PG_TYPE_DATA and 
TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS. But the call of the second callback 
checks the typentry->tupDesc and, because it wasn't NULL, attempted to 
remove this record a second time.
I think there is no case for redesign, but we have a mess in 
insertion/deletion conditions.

-- 
regards, Andrei Lepikhov