Horton

Re: type cache cleanup improvements

Andrei Lepikhov <lepihov@gmail.com>

View thread
From: Andrei Lepikhov <lepihov@gmail.com>
To: Alexander Korotkov <aekorotkov@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-17T09:41:15Z
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 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.

-- 
regards, Andrei Lepikhov