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