[Masahiro Ikeda <ikedamsh@oss.nttdata.com>]

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

Attachments:

0001-change-the-die-condition-in-generate-wait_event_type.patch (text/x-diff, 1.1K) download
From 5198fc6302a3bf4232252b23b45d39d987e736bc Mon Sep 17 00:00:00 2001
From: Masahiro Ikeda <masahiro.ikeda.us@hco.ntt.co.jp>
Date: Wed, 12 Jul 2023 16:28:27 +0900
Subject: [PATCH] change the die condition in generate-wait_event_types.pl

---
 src/backend/utils/activity/generate-wait_event_types.pl | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl
index 6d1a2af42a..fbe011039a 100644
--- a/src/backend/utils/activity/generate-wait_event_types.pl
+++ b/src/backend/utils/activity/generate-wait_event_types.pl
@@ -89,9 +89,8 @@ foreach my $line (@lines_sorted)
 	# any C and header files generated.
 	die "wait event names must start with 'WAIT_EVENT_'"
 	  if ( $trimmedwaiteventname eq $waiteventenumname
-		&& $waiteventenumname !~ /^LWLock/
-		&& $waiteventenumname !~ /^Lock/);
-	$continue = ",\n";
+		&& $waitclassname !~ /^WaitEventLWLock$/
+		&& $waitclassname !~ /^WaitEventLock$/);
 	push(@{ $hashwe{$waitclassname} }, @waiteventlist);
 }
 
-- 
2.25.1