Hi,

On Tue, Mar 19, 2024 at 09:59:35AM +0900, Michael Paquier wrote:
> On Mon, Mar 18, 2024 at 05:57:02PM +0000, Bertrand Drouvot wrote:
> > Thanks for looking at it!
> > Oh right, the comment is wrong, re-worded in v2 attached.
> 
> I've added a couple of fake events in my txt file, and this results in
> an ordering of the wait events in the docs while the backpatched wait
> events are added at the end of the enums, based on their order in the
> txt file.

Thanks for testing!

>  # When adding a new wait event, make sure it is placed in the appropriate
> -# ClassName section.
> +# ClassName section. If the wait event is backpatched from master to a 
> version
> +# >= 17 then put it under a "Backpatch:" delimiter at the end of the related
> +# ClassName section (on the non master branches) or at its natural position 
> on
> +# the master branch.
> +# Ensure that the backpatch regions are always empty on the master branch.
> 
> I'd recommend to not mention a version number at all, as this would
> need a manual refresh each time a new stable branch is forked.

I'm not sure as v2 used the "version >= 17" wording which I think would not need
manual refresh each time a new stable branch is forked.

But to avoid any doubt, I'm following your recommendation in v3 attached (then
only mentioning the "master branch" and "any other branch").

> Your solution is simpler than what I finished in mind when looking at
> the code yesterday, with the addition of a second array that's pushed
> to be at the end of the "sorted" lines ordered by the second column.
> That does the job.

Yeah.

> (Note that I'll go silent for some time; I'll handle this thread when
> I get back as this is not urgent.)

Right and enjoy!

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
>From 8755162399faa070cbcb56bb4687f1ca1df6b4b1 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot...@gmail.com>
Date: Mon, 18 Mar 2024 08:34:19 +0000
Subject: [PATCH v3] Add "Backpatch" regions in wait_event_names.txt

When backpatching, adding an event will renumber others, which can make an
extension report the wrong event until recompiled. This is due to the fact that
generate-wait_event_types.pl sorts events to position them. The "Backpatch"
region added here ensures no ordering is done for the wait events found after
this delimiter.
---
 .../activity/generate-wait_event_types.pl     | 26 ++++++++++++++++++-
 .../utils/activity/wait_event_names.txt       | 18 ++++++++++++-
 2 files changed, 42 insertions(+), 2 deletions(-)
 100.0% src/backend/utils/activity/

diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl
index f1adf0e8e7..d129d94889 100644
--- a/src/backend/utils/activity/generate-wait_event_types.pl
+++ b/src/backend/utils/activity/generate-wait_event_types.pl
@@ -38,7 +38,9 @@ die "Not possible to specify --docs and --code simultaneously"
 
 open my $wait_event_names, '<', $ARGV[0] or die;
 
+my @backpatch_lines;
 my @lines;
+my $backpatch = 0;
 my $section_name;
 my $note;
 my $note_name;
@@ -59,10 +61,26 @@ while (<$wait_event_names>)
 	{
 		$section_name = $_;
 		$section_name =~ s/^.*- //;
+		$backpatch = 0;
 		next;
 	}
 
-	push(@lines, $section_name . "\t" . $_);
+	# Are we dealing with backpatch wait events?
+	if (/^Backpatch:$/)
+	{
+		$backpatch = 1;
+		next;
+	}
+
+	# Backpatch wait events won't be sorted during code generation
+	if ($gen_code && $backpatch)
+	{
+		push(@backpatch_lines, $section_name . "\t" . $_);
+	}
+	else
+	{
+		push(@lines, $section_name . "\t" . $_);
+	}
 }
 
 # Sort the lines based on the second column.
@@ -70,6 +88,12 @@ while (<$wait_event_names>)
 my @lines_sorted =
   sort { uc((split(/\t/, $a))[1]) cmp uc((split(/\t/, $b))[1]) } @lines;
 
+# If we are generating code then concat @lines_sorted and @backpatch_lines.
+if ($gen_code)
+{
+	push(@lines_sorted, @backpatch_lines);
+}
+
 # Read the sorted lines and populate the hash table
 foreach my $line (@lines_sorted)
 {
diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt
index c08e00d1d6..0c4788fe77 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -24,7 +24,10 @@
 #      SGML tables of wait events for inclusion in the documentation.
 #
 # When adding a new wait event, make sure it is placed in the appropriate
-# ClassName section.
+# ClassName section. Put it in its natural position in the master branch, and
+# then put it in the "Backpatch:" region for any other branch (should the wait
+# event be backpatched).
+# Ensure that the backpatch regions are always empty on the master branch.
 #
 # WaitEventLWLock and WaitEventLock have their own C code for their wait event
 # enums and function names.  Hence, for these, only the SGML documentation is
@@ -61,6 +64,7 @@ WAL_SENDER_MAIN	"Waiting in main loop of WAL sender process."
 WAL_SUMMARIZER_WAL	"Waiting in WAL summarizer for more WAL to be generated."
 WAL_WRITER_MAIN	"Waiting in main loop of WAL writer process."
 
+Backpatch:
 
 #
 # Wait Events - Client
@@ -82,6 +86,7 @@ WAIT_FOR_STANDBY_CONFIRMATION	"Waiting for WAL to be received and flushed by the
 WAL_SENDER_WAIT_FOR_WAL	"Waiting for WAL to be flushed in WAL sender process."
 WAL_SENDER_WRITE_DATA	"Waiting for any activity when processing replies from WAL receiver in WAL sender process."
 
+Backpatch:
 
 #
 # Wait Events - IPC
@@ -149,6 +154,7 @@ WAL_RECEIVER_WAIT_START	"Waiting for startup process to send initial data for st
 WAL_SUMMARY_READY	"Waiting for a new WAL summary to be generated."
 XACT_GROUP_UPDATE	"Waiting for the group leader to update transaction status at end of a parallel operation."
 
+Backpatch:
 
 #
 # Wait Events - Timeout
@@ -169,6 +175,7 @@ VACUUM_DELAY	"Waiting in a cost-based vacuum delay point."
 VACUUM_TRUNCATE	"Waiting to acquire an exclusive lock to truncate off any empty pages at the end of a table vacuumed."
 WAL_SUMMARIZER_ERROR	"Waiting after a WAL summarizer error."
 
+Backpatch:
 
 #
 # Wait Events - IO
@@ -256,6 +263,7 @@ WAL_SYNC	"Waiting for a WAL file to reach durable storage."
 WAL_SYNC_METHOD_ASSIGN	"Waiting for data to reach durable storage while assigning a new WAL sync method."
 WAL_WRITE	"Waiting for a write to a WAL file."
 
+Backpatch:
 
 #
 # Wait Events - Buffer Pin
@@ -265,6 +273,7 @@ Section: ClassName - WaitEventBufferPin
 
 BUFFER_PIN	"Waiting to acquire an exclusive pin on a buffer."
 
+Backpatch:
 
 #
 # Wait Events - Extension
@@ -274,6 +283,8 @@ Section: ClassName - WaitEventExtension
 
 Extension	"Waiting in an extension."
 
+Backpatch:
+
 #
 # Wait Events - LWLock
 #
@@ -330,6 +341,8 @@ DSMRegistry	"Waiting to read or update the dynamic shared memory registry."
 InjectionPoint	"Waiting to read or update information related to injection points."
 SerialControl	"Waiting to read or update shared <filename>pg_serial</filename> state."
 
+# Don't create a Backpatch region here.
+
 #
 # END OF PREDEFINED LWLOCKS (DO NOT CHANGE THIS LINE)
 #
@@ -377,6 +390,7 @@ SerialSLRU	"Waiting to access the serializable transaction conflict SLRU cache."
 SubtransSLRU	"Waiting to access the sub-transaction SLRU cache."
 XactSLRU	"Waiting to access the transaction status SLRU cache."
 
+# Don't create a Backpatch region here.
 
 #
 # Wait Events - Lock
@@ -399,3 +413,5 @@ object	"Waiting to acquire a lock on a non-relation database object."
 userlock	"Waiting to acquire a user lock."
 advisory	"Waiting to acquire an advisory user lock."
 applytransaction	"Waiting to acquire a lock on a remote transaction being applied by a logical replication subscriber."
+
+Backpatch:
-- 
2.34.1

Reply via email to