On Tue, Mar 19, 2024 at 07:34:09AM +0000, Bertrand Drouvot wrote: > 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").
I don't see why we could not be more generic, TBH. Note that the Backpatch region should be empty not only the master branch but also on stable and unreleased branches (aka REL_XX_STABLE branches from their fork from master to their .0 release). I have reworded the whole, mentioning ABI compatibility, as well. The position of the Backpatch regions were a bit incorrect (extra one in LWLock, and the one in Lock was not needed). We could be stricter with the order of the elements in pgstat_wait_event.c and wait_event_funcs_data.c, but there's no consequence feature-wise and I cannot get excited about the extra complexity this creates in generate-wait_event_types.pl between the enum generation and the rest. Is "Backpatch" the best choice we have, though? It speaks by itself but I was thinking about something different, like "Stable". Other ideas or objections are welcome. My naming sense is usually not that good, so there's that. 0001 is the patch with my tweaks. 0002 includes some dummy test data I've used to validate the whole. -- Michael
From 7579d48b61ca17f7114b517ab88aabf30c99b64d Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 4 Apr 2024 15:34:37 +0900
Subject: [PATCH v3 1/2] 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 | 27 ++++++++++++++++++-
.../utils/activity/wait_event_names.txt | 20 +++++++++++++-
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/activity/generate-wait_event_types.pl b/src/backend/utils/activity/generate-wait_event_types.pl
index f1adf0e8e7..456a00b86a 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,27 @@ while (<$wait_event_names>)
{
$section_name = $_;
$section_name =~ s/^.*- //;
+ $backpatch = 0;
next;
}
- push(@lines, $section_name . "\t" . $_);
+ # Backpatch region, preserving ABI compatibility of the code
+ # generated. Any wait events listed in this part of the file
+ # will not be sorted during the code generation.
+ if (/^Backpatch:$/)
+ {
+ $backpatch = 1;
+ next;
+ }
+
+ 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 +89,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, concat @lines_sorted and then @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 0d288d6b3d..5afa4d4839 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -26,6 +26,12 @@
# When adding a new wait event, make sure it is placed in the appropriate
# ClassName section.
#
+# Wait events added in stable branches should be appended to the lists in
+# the "Backpatch:" region of their related ClassName section to preserve
+# ABI compatibility of the C code generated from this file's data, respecting
+# the order of any wait event already listed there. The "Backpatch:" regions
+# should remain empty on the master branch and on unreleased branches.
+#
# WaitEventLWLock and WaitEventLock have their own C code for their wait event
# enums and function names. Hence, for these, only the SGML documentation is
# generated.
@@ -61,6 +67,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
@@ -81,8 +88,10 @@ SSL_OPEN_SERVER "Waiting for SSL while attempting connection."
WAIT_FOR_STANDBY_CONFIRMATION "Waiting for WAL to be received and flushed by the physical standby."
WAIT_FOR_WAL_REPLAY "Waiting for a replay of the particular WAL position on the physical standby."
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."
+WAL_SENDER_WRITE_ZZZ "Waiting for wal sender write ZZ."
+WAL_SENDER_WRITE_DATA "Waiting for wal sender write data."
+Backpatch:
#
# Wait Events - IPC
@@ -150,6 +159,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
@@ -170,6 +180,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
@@ -257,6 +268,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
@@ -266,6 +278,7 @@ Section: ClassName - WaitEventBufferPin
BUFFER_PIN "Waiting to acquire an exclusive pin on a buffer."
+Backpatch:
#
# Wait Events - Extension
@@ -275,6 +288,8 @@ Section: ClassName - WaitEventExtension
Extension "Waiting in an extension."
+Backpatch:
+
#
# Wait Events - LWLock
#
@@ -379,6 +394,7 @@ SubtransSLRU "Waiting to access the sub-transaction SLRU cache."
XactSLRU "Waiting to access the transaction status SLRU cache."
ParallelVacuumDSA "Waiting for parallel vacuum dynamic shared memory allocation."
+# No "Backpatch" region here as code is generated automatically.
#
# Wait Events - Lock
@@ -401,3 +417,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."
+
+# No "Backpatch" region here as code is generated automatically.
--
2.43.0
From 05565c750d0f20ce12ebb579f4789ab1d275eab6 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Thu, 4 Apr 2024 15:35:27 +0900 Subject: [PATCH v3 2/2] Add some dummy tests --- src/backend/utils/activity/wait_event_names.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/backend/utils/activity/wait_event_names.txt b/src/backend/utils/activity/wait_event_names.txt index 5afa4d4839..8abc22981a 100644 --- a/src/backend/utils/activity/wait_event_names.txt +++ b/src/backend/utils/activity/wait_event_names.txt @@ -68,6 +68,11 @@ 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: +2_WAL_SENDER_WRITE_FO "Waiting for fo." +2_FOO_BAR_3 "Popo bar 3." +2_FOO_BAR_2 "Popo bar 2." +2_ZZZ "Waiting in zzz." +2_XXX "Waiting in XXX." # # Wait Events - Client @@ -92,6 +97,11 @@ WAL_SENDER_WRITE_ZZZ "Waiting for wal sender write ZZ." WAL_SENDER_WRITE_DATA "Waiting for wal sender write data." Backpatch: +WAL_SENDER_WRITE_FOO "Waiting for wal sender write foo." +FOO_BAR_3 "Waiting for foo bar 3." +FOO_BAR_2 "Waiting for foo bar 2." +ZZZ "Waiting for zzz." +XXX "Waiting for XXX." # # Wait Events - IPC -- 2.43.0
signature.asc
Description: PGP signature
