Horton

Re: type cache cleanup improvements

Andrei Lepikhov <a.lepikhov@postgrespro.ru>

View thread
From: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
To: Teodor Sigaev <teodor@sigaev.ru>, Pgsql Hackers <pgsql-hackers@postgresql.org>
Cc: Aleksander Alekseev <aleksander@timescale.com>, Tom Lane <tgl@sss.pgh.pa.us>, Michael Paquier <michael@paquier.xyz>
Date: 2024-04-03T06:07:27Z
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.

Attachments

On 3/15/24 17:57, Teodor Sigaev wrote:
>> Okay, I've applied this piece for now.  Not sure I'll have much room
>> to look at the rest.
> 
> Thank you very much!
I have spent some time reviewing this feature. I think we can discuss 
and apply it step-by-step. So, the 0001-* patch is at this moment.
The feature addresses the issue of TypCache being bloated by intensive 
usage of non-standard types and domains. It adds significant overhead 
during relcache invalidation by thoroughly scanning this hash table.
IMO, this feature will be handy soon, as we already see some patches 
where TypCache is intensively used for storing composite types—for 
example, look into solutions proposed in [1].
One of my main concerns with this feature is the possibility of lost 
entries, which could be mistakenly used by relations with the same oid 
in the future. This seems particularly possible in cases with multiple 
temporary tables. The author has attempted to address this by replacing 
the typrelid and type_id fields in the mapRelType on each call of 
lookup_type_cache. However, I believe we could further improve this by 
removing the entry from mapRelType on invalidation, thus avoiding this 
potential issue.
While reviewing the patch, I made some minor changes (see attachment) 
that you're free to adopt or reject. However, it's crucial that the 
patch includes a detailed explanation, not just a single sentence, to 
ensure everyone understands the changes.
Upon closer inspection, I noticed that the current implementation only 
invalidates the cache entry. While this is acceptable for standard 
types, it may not be sufficient to maintain numerous custom types (as in 
the example in the initial letter) or in cases where whole-row vars are 
heavily used. In such scenarios, removing the entry and reducing the 
hash table's size might be more efficient.
In toto, the 0001-* patch looks good, and I would be glad to see it in 
the core.

[1] 
https://www.postgresql.org/message-id/flat/CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg%40mail.gmail.com

-- 
regards,
Andrei Lepikhov
Postgres Professional