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
From 5198fc6302a3bf4232252b23b45d39d987e736bc Mon Sep 17 00:00:00 2001
From: Masahiro Ikeda <masahiro.ikeda...@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

Reply via email to