Re: Support to define custom wait events for extensions

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Tristan Partin <tristan(at)neon(dot)tech>
Cc: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>, andres(at)anarazel(dot)de, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Support to define custom wait events for extensions
Date: 2023-07-27 09:16:46
Message-ID: ZMI1/tQp/TQ1/m3B@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jul 19, 2023 at 10:57:39AM -0500, Tristan Partin wrote:
> > + <sect2 id="xfunc-addin-wait-events">
> > + <title>Custom Wait Events for Add-ins</title>
>
> This would be the second use of "Add-ins" ever, according to my search.
> Should this be "Extensions" instead?

Yes, I would think that just "Custom Wait Events" is enough here.
And I'd recommend to also use Shared Memory here. The case of
dynamically loaded things is possible, more advanced and can work, but
I am not sure we really need to do down to that as long as we mention
to use shared_preload_libraries.

I've rewritten the docs in their entirety, but honestly I still need
to spend more time polishing that.

Another part of the patch that has been itching me a lot are the
regression tests. I have spent some time today migrating the tests of
worker_spi to TAP for the sake of this thread, resulting in commit
320c311, and concluded that we need to care about three new cases:
- For custom wait events where the shmem state is not loaded, check
that we report the default of 'extension'.
- Check that it is possible to allocate and load a custom wait event
dynamically. Here, I have used a new SQL function in worker_spi,
called worker_spi_init(). That feels a bit hack-ish but for a test in
a template module that works great.
- Check that wait events loaded through shared_preload_libraries work
correctly.

The tests of worker_spi can take care easily of all these cases, once
a few things for the shmem handling are put in place for the dynamic
and preloading cases.

+Datum
+get_new_wait_event_info(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_UINT32(WaitEventExtensionNew());
+}

While looking at the previous patch and the test, I've noticed this
pattern. WaitEventExtensionNew() should not be called without holding
AddinShmemInitLock, or that opens the door to race conditions.

I am still mid-way through the review of the core APIs, but attached
is my current version in progress, labelled v8. I'll continue
tomorrow. I'm aware of some typos in the commit message of this
patch, and the dynamic bgworker launch is failing in the CI for
VS2019 (just too tired to finish debugging that today).

Thoughts are welcome.
--
Michael

Attachment Content-Type Size
v8-0001-Support-custom-wait-events-for-extensions.patch text/x-diff 21.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiro Ikeda 2023-07-27 09:29:22 Re: Support to define custom wait events for extensions
Previous Message David Rowley 2023-07-27 08:53:16 Re: Performance degradation on concurrent COPY into a single relation in PG16.