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: Artur Zakirov <zaartur@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, jian he <jian.universality@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-10-20T17:36:13Z
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 Thu, Oct 17, 2024 at 12:41 PM Andrei Lepikhov <lepihov@gmail.com> wrote:
> On 10/15/24 15:08, Alexander Korotkov wrote:
> > Yep, they didn't get updated.  Fixed in the attached patchset.
> Let me wear Alexander Lakhin's mask for a moment and say that the code
> may cause a segfault:
>
> #0  0x000055e0da186000 in insert_rel_type_cache_if_needed (typentry=0x0)
> at typcache.c:3066
> b3066           if (typentry->typtype != TYPTYPE_COMPOSITE)
> (gdb) bt 20
> #0  0x000055e0da186000 in insert_rel_type_cache_if_needed (typentry=0x0)
> at typcache.c:3066
> #1  0x000055e0da18844f in cleanup_in_progress_typentries () at
> typcache.c:3172
> #2  0x000055e0da1883f9 in AtEOXact_TypeCache () at typcache.c:3181
> #3  0x000055e0d9a22e59 in AbortTransaction () at xact.c:2961
> #4  0x000055e0d9a1f75c in AbortCurrentTransactionInternal () at xact.c:3491
> #5  0x000055e0d9a1f6be in AbortCurrentTransaction () at xact.c:3445
> #6  0x000055e0d9f55f28 in PostgresMain (dbname=0x55e1057fb838
> "regression", username=0x55e1057fb818 "danolivo") at postgres.c:4508
> #7  0x000055e0d9f4e4a3 in BackendMain (startup_data=0x7ffeaf051310 "",
> startup_data_len=4) at backend_startup.c:107
> #8  0x000055e0d9e4bfee in postmaster_child_launch (child_type=B_BACKEND,
> startup_data=0x7ffeaf051310 "", startup_data_len=4,
> client_sock=0x7ffeaf051358) at launch_backend.c:274
> #9  0x000055e0d9e522a3 in BackendStartup (client_sock=0x7ffeaf051358) at
> postmaster.c:3420
> #10 0x000055e0d9e502f9 in ServerLoop () at postmaster.c:1653
> #11 0x000055e0d9e4f4fe in PostmasterMain (argc=3, argv=0x55e1057bb520)
> at postmaster.c:1351
> #12 0x000055e0d9cf4b2d in main (argc=3, argv=0x55e1057bb520) at main.c:197
>
> It can happen if something triggers an error in the middle of
> lookup_type_cache when in_progress_list[i] is already filled, but the
> typentry wasn't created.
> I think it can be easily shielded (see attached). Also, the name
> cleanup_in_progress_typentries causes a lot of ponderings, I guess it
> would be better to rename it likewise  finalize_in_progress_typentries.
> Also, I added trivial comments to better understand what the function does.
>
> I think the first patch may already be committed, and this little burden
> may be avoided in future versions.

Thank you!
I've integrated your patch.  But I think
finalize_in_progress_typentries() is more appropriate place to check
typentry for NULL.  Also, I've revised the
finalize_in_progress_typentries() header comment.  I'm going to push
these patches if no objections.

------
Regards,
Alexander Korotkov
Supabase