Re: Support to define custom wait events for extensions

On 2023-07-11 16:45, Michael Paquier wrote:
> On Fri, Jun 23, 2023 at 05:56:26PM +0900, Masahiro Ikeda wrote:
>> I updated the patches to handle the warning mentioned
>> by PostgreSQL Patch Tester, and removed unnecessary spaces.
>
> I have begun hacking on that, and the API layer inspired from the
> LWLocks is sound. I have been playing with it in my own extensions
> and it is nice to be able to plug in custom wait events into
> pg_stat_activity, particularly for bgworkers. Finally.

Great!

> The patch needed a rebase after the recent commit that introduced the
> automatic generation of docs and code for wait events. It requires
> two tweaks in generate-wait_event_types.pl, feel free to double-check
> them.

Thanks for rebasing. I confirmed it works with the current master.

I know this is a little off-topic from what we're talking about here,
but I'm curious about generate-wait_event_types.pl.

# generate-wait_event_types.pl
- # An exception is required for LWLock and Lock as these don't require
- # any C and header files generated.
+ # An exception is required for Extension, LWLock and Lock as these
don't
+ # require any C and header files generated.
die "wait event names must start with 'WAIT_EVENT_'"
if ( $trimmedwaiteventname eq $waiteventenumname
+ && $waiteventenumname !~ /^Extension/
&& $waiteventenumname !~ /^LWLock/
&& $waiteventenumname !~ /^Lock/);

In my understanding, the first column of the row for WaitEventExtension
in
wait_event_names.txt can be any value and the above code should not die.
But if I use the following input, it falls on the last line.

# wait_event_names.txt
Section: ClassName - WaitEventExtension

WAIT_EVENT_EXTENSION "Extension" "Waiting in an extension."
Extension "Extension" "Waiting in an extension."
EXTENSION "Extension" "Waiting in an extension."

If the behavior is unexpected, we need to change the current code.
I have created a patch for the areas that I felt needed to be changed.
- 0001-change-the-die-condition-in-generate-wait_event_type.patch
(In addition to the above, "$continue = ",\n";" doesn't appear to be
necessary.)

> Some of the new structures and routine names don't quite reflect the
> fact that we have wait events for extensions, so I have taken a stab
> at that.

Sorry. I confirmed the change.

> Note that the test module test_custom_wait_events would crash if
> attempting to launch a worker when not loaded in
> shared_preload_libraries, so we'd better have some protection in
> wait_worker_launch() (this function should be renamed as well).

OK, I will handle it. Since Andres gave me other comments for the
test module, I'll think about what is best.

> Attached is a rebased patch that I have begun tweaking here and
> there. For now, the patch is moved as waiting on author. I have
> merged the test module with the main patch for the moment, for
> simplicity. A split is straight-forward as the code paths touched are
> different.
>
> Another and *very* important thing is that we are going to require
> some documentation in xfunc.sgml to explain how to use these routines
> and what to expect from them. Ikeda-san, could you write some? You
> could look at the part about shmem and LWLock to get some
> inspiration.

OK. Yes, I planned to write documentation.

Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION