Re: Support to define custom wait events for extensions

On 2023-08-08 10:05, Michael Paquier wrote:
> On Tue, Aug 08, 2023 at 09:39:02AM +0900, Masahiro Ikeda wrote:
>> I am thinking a bit that we also need another hash where the key
>> is a custom string. For extensions that have no dependencies
>> with shared_preload_libraries, I think we need to avoid that
>> WaitEventExtensionNew() is called repeatedly and a new eventId
>> is issued each time.
>>
>> So, is it better to have another hash where the key is
>> a custom string and uniqueness is identified by it to determine
>> if a new eventId should be issued?
>
> Yeah, I was also considering if something like that is really
> necessary, but I cannot stop worrying about adding more contention to
> the hash table lookup each time an extention needs to retrieve an
> event ID to use for WaitLatch() or such. The results of the hash
> table lookups could be cached in each backend, still it creates an
> extra cost when combined with queries running in parallel on
> pg_stat_activity that do the opposite lookup event_id -> event_name.
>
> My suggestion adds more load to AddinShmemInitLock instead.
> Hence, I was just thinking about relying on AddinShmemInitLock to
> insert new entries in the hash table, only if its shmem state is not
> found when calling ShmemInitStruct(). Or perhaps it is just OK to not
> care about the impact event_name -> event_id lookup for fresh
> connections, and just bite the bullet with two lookup keys instead of
> relying on AddinShmemInitLock for the addition of new entries in the
> hash table? Hmm, perhaps you're right with your approach here, at the
> end.

For the first idea, I agree that if a lot of new connections come in,
it is easy to leads many conflicts. The only solution I can think of
is to use connection pooling now.

IIUC, the second idea is based on the premise of allocating their shared
memory for each extension. In that case, the cons of the first idea can
be solved because the wait event infos are saved in their shared memory
and
they don't need call WaitEventExtensionNew() anymore. Is that right?

Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION