Horton

Re: type cache cleanup improvements

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Andrei Lepikhov <lepihov@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, Pavel Borisov <pashkin.elfe@gmail.com>, 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-09-12T23:38:40Z
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 Sat, Aug 31, 2024 at 10:33 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
> On 29/8/2024 11:01, Alexander Korotkov wrote:
> > On Mon, Aug 26, 2024 at 11:26 AM Alexander Korotkov
> >> Secondly, I'm not terribly happy with current state of type cache.
> >> The caller of lookup_type_cache() might get already invalidated data.
> >> This probably OK, because caller probably hold locks on dependent
> >> objects to guarantee that relevant properties of type actually
> >> persists.  At very least this should be documented, but it doesn't
> >> seem so.  Setting of tupdesc is sensitive to its order of execution.
> >> That feels quite fragile to me, and not documented either.  I think
> >> this area needs improvements before we push additional functionality
> >> there.
> >
> > I see fdd965d074 added a proper handling for concurrent invalidation
> > for relation cache. If a concurrent invalidation occurs, we retry
> > building a relation descriptor.  Thus, we end up with returning of a
> > valid relation descriptor to caller.  I wonder if we can take the same
> > approach to type cache.  That would make the whole type cache more
> > consistent and less fragile.  Also, this patch will be simpler.
> I think I understand the solution from the commit fdd965d074.
> Just for the record, you mentioned invalidation inside the
> lookup_type_cache above. Passing through the code, I found the only
> place for such a case - the call of the GetDefaultOpClass, which
> triggers the opening of the relation pg_opclass, which can cause an
> AcceptInvalidationMessages call. Did you mean this case, or does a wider
> field of cases exist here?

I've tried to implement handling of concurrent invalidation similar to
commit fdd965d074.  However that appears to be more difficult that I
thought, because for some datatypes like arrays, ranges etc we might
need fill the element type and reference it.  So, I decided to
continue with the current approach but borrowing some ideas from
fdd965d074.  The revised patchset attached.

0001 - adds comment about concurrent invalidation handling
0002 - revised c14d4acb8.  Now we track type oids, whose
TypeCacheEntry's filing is in-progress.  Add entry to
RelIdToTypeIdCacheHash at the end of lookup_type_cache() or on the
transaction abort.  During invalidation don't assert
RelIdToTypeIdCacheHash to be here if TypeCacheEntry is in-progress.

------
Regards,
Alexander Korotkov
Supabase