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>
Cc: Alexander Lakhin <exclusion@gmail.com>, 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-31T19:33:23Z
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 29/8/2024 11:01, Alexander Korotkov wrote:
> On Mon, Aug 26, 2024 at 11:26 AM Alexander Korotkov
>> Secondly, I'm not terribly happy with current state of type cache.
>> The caller of lookup_type_cache() might get already invalidated data.
>> This probably OK, because caller probably hold locks on dependent
>> objects to guarantee that relevant properties of type actually
>> persists.  At very least this should be documented, but it doesn't
>> seem so.  Setting of tupdesc is sensitive to its order of execution.
>> That feels quite fragile to me, and not documented either.  I think
>> this area needs improvements before we push additional functionality
>> there.
> 
> I see fdd965d074 added a proper handling for concurrent invalidation
> for relation cache. If a concurrent invalidation occurs, we retry
> building a relation descriptor.  Thus, we end up with returning of a
> valid relation descriptor to caller.  I wonder if we can take the same
> approach to type cache.  That would make the whole type cache more
> consistent and less fragile.  Also, this patch will be simpler.
I think I understand the solution from the commit fdd965d074.
Just for the record, you mentioned invalidation inside the 
lookup_type_cache above. Passing through the code, I found the only 
place for such a case - the call of the GetDefaultOpClass, which 
triggers the opening of the relation pg_opclass, which can cause an 
AcceptInvalidationMessages call. Did you mean this case, or does a wider 
field of cases exist here?

-- 
regards, Andrei Lepikhov