Horton

Re: type cache cleanup improvements

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Pavel Borisov <pashkin.elfe@gmail.com>
Cc: Andrei Lepikhov <a.lepikhov@postgrespro.ru>, 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-22T16:52:32Z
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

Hi!

On Thu, Aug 22, 2024 at 1:02 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
Looked at v9:
> Patch looks good to me. I'd only suggest comments changes:
>
> "The map from relation's OID to the corresponding composite type OID" -> "The mapping of relation's OID to the corresponding composite type OID"
> "We're keeping the map entry when corresponding typentry have either TCFLAGS_HAVE_PG_TYPE_DATA, or TCFLAGS_OPERATOR_FLAGS, or tupdesc.  That is we're keeping map entry if the entry has something to clear." -> "We're keeping the map entry when the corresponding typentry has something to clear i.e it has either TCFLAGS_HAVE_PG_TYPE_DATA, or TCFLAGS_OPERATOR_FLAGS, or tupdesc."
> "Invalidate particular TypeCacheEntry on Relcache inval callback" - remove extra tabs before. Maybe also add empty line above.
> "Typically shouldn't be a problem" -> "Typically this shouldn't affect performance"
> "Relid = 0, so we need" -> "Relid is invalid. By convention we need"
> "if cleaned TCFLAGS_HAVE_PG_TYPE_DATA flag" -> "if we cleaned TCFLAGS_HAVE_PG_TYPE_DATA flag previously"
> "+/*
> + * Delete entry RelIdToTypeIdCacheHash if needed after resetting of the
> + * TCFLAGS_HAVE_PG_TYPE_DATA flag, or any of TCFLAGS_OPERATOR_FLAGS flags,
> + * or tupDesc if needed." - remove one "if needed"

Thank you for your feedback.  I've integrated all your edits except
the formatting change of InvalidateCompositeTypeCacheEntry() header
comment.  I think the functions below have the same formatting of
header comments, and it's not necessary to change format.

If no objections, I'm planning to push this after reverting PARTITION
SPLIT/MERGE.

------
Regards,
Alexander Korotkov
Supabase