Re: type cache cleanup improvements

Alexander Korotkov <aekorotkov@gmail.com>

From: Alexander Korotkov <aekorotkov@gmail.com>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>, 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-25T21:22:03Z
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 Sun, Aug 25, 2024 at 10:21 PM Alexander Korotkov
<aekorotkov@gmail.com> wrote:
> On Sun, Aug 25, 2024 at 10:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> > 22.08.2024 19:52, Alexander Korotkov wrotd:
> > > If no objections, I'm planning to push this after reverting PARTITION
> > > SPLIT/MERGE.
> > >
> >
> > Please try to perform `make check` against a CLOBBER_CACHE_ALWAYS build.
> > trilobite failed it:
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=trilobite&dt=2024-08-25%2005%3A22%3A07
> >
> > and I'm observing the same locally:
> > ...
> > #5  0x00005636d37555f8 in ExceptionalCondition (conditionName=0x5636d39b1940 "found",
> >      fileName=0x5636d39b1308 "typcache.c", lineNumber=3077) at assert.c:66
> > #6  0x00005636d37554a4 in delete_rel_type_cache_if_needed (typentry=0x5636d41d5d10) at typcache.c:3077
> > #7  0x00005636d3754063 in InvalidateCompositeTypeCacheEntry (typentry=0x5636d41d5d10) at typcache.c:2355
> > #8  0x00005636d37541d3 in TypeCacheRelCallback (arg=0, relid=0) at typcache.c:2441
> > ...
> >
> > (gdb) f 6
> > #6  0x00005636d37554a4 in delete_rel_type_cache_if_needed (typentry=0x5636d41d5d10) at typcache.c:3077
> > 3077                    Assert(found);
> > (gdb) p found
> > $1 = false
> >
> > (This Assert is introduced by c14d4acb8.)
>
> Thank you for noticing.  I'm checking this.

I didn't take into account that TypeCacheEntry could be invalidated
while lookup_type_cache() does syscache lookups.  When I realized that
I was curious on how does it currently work.  It appears that type
cache invalidation mostly only clears the flags while values are
remaining in place and still available for lookup_type_cache() caller.
TypeCacheEntry.tupDesc is invalidated directly, and it has guarantee
to survive only because we don't do any syscache lookups for composite
data types later in lookup_type_cache().  I'm becoming less fan of how
this works...  I think these aspects needs to be at least documented
in details.

Regarding c14d4acb8, it appears to require redesign.  I'm going to revert it.

------
Regards,
Alexander Korotkov
Supabase