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 <[email protected]>
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