Horton

Re: type cache cleanup improvements

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
Cc: 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-20T19:00:53Z
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 Mon, Aug 5, 2024 at 4:16 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> I've revised the patchset.  First of all, I've re-ordered the patches.
>
> 0001-0002 (former 0002-0003)
> Comprises hash_search_with_hash_value() function and its application
> to avoid full hash iteration in InvalidateAttoptCacheCallback() and
> TypeCacheTypCallback().  I think this is quite straightforward
> optimization without negative side effects.  I've revised comments,
> commit message and did some code beautification.  I'm going to push
> this if no objections.
>
> 0003 (former 0001)
> I've revised this patch.  I think main concerns expressed in the
> thread about this path is that we don't have invalidation mechanism
> for relid => typid map.  Finally due to oid wraparound same relids
> could get reused.  That could lead to invalid entries in the map about
> existing relids and typeids.  This is rather messy, but I don't think
> this could cause a material bug.  The maps items are used only for
> cache invalidation.  Extra invalidation doesn't cause a bug.  If type
> with same relid will be cached, then correspoding map item will be
> overridden, so no missing invalidation.  However, I see the following
> reasons for keeping consistent state of relid => typid map.
>
> 1) As the main use-case for this optimization is flood of temporary
> tables, it would be nice not let relid => typid map bloat in this
> case.  I see that TypeCacheHash would get bloated, because its entries
> are never deleted.  However, I would prefer to not get this situation
> even worse.
> 2) In future we may find some more use-cases for relid => typid map
> besides cache invalidation.  Keeping that in consistent state could be
> advantage then.
>
> In the attached patch, I'm keeping relid => typid map when
> corresponding typentry have either TCFLAGS_HAVE_PG_TYPE_DATA, or
> TCFLAGS_OPERATOR_FLAGS, or tupdesc.  Thus, when temporary table gets
> deleted, we would invalidate the map item.
>
> It will be also nice to get rid of iteration over all the cached
> domain types in TypeCacheRelCallback().  However, this typically
> shouldn't be a problem since domain types are less tended to bloat.
> Domain types are created manually, unlike composite types which are
> automatically created for every temporary table.  We will probably
> need to optimize this in future, but I don't feel this to be necessary
> in present patch.
>
> I think the revised 0003 requires review.

The rebased remaining patch is attached.

------
Regards,
Alexander Korotkov
Supabase