Re: type cache cleanup improvements

From: Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
To: Teodor Sigaev <teodor(at)sigaev(dot)ru>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Aleksander Alekseev <aleksander(at)timescale(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Paquier <michael(at)paquier(dot)xyz>
Subject: Re: type cache cleanup improvements
Date: 2024-04-03 06:07:27
Message-ID: e58f0f24-e808-4772-a961-639bdff67000@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-minor_improvements.diff text/x-patch 3.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2024-04-03 06:13:13 Re: Fix for timestamp lag issue from emit_log_hook when GUC log_line_prefix has '%m'
Previous Message Michael Paquier 2024-04-03 06:02:16 Re: Missing LWLock protection in pgstat_reset_replslot()