Hi,
On Mon, Mar 18, 2024 at 08:02:24AM +0900, Michael Paquier wrote:
> On Sun, Mar 17, 2024 at 11:31:14AM -0700, Noah Misch wrote:
> > Adding an event
> > will renumber others, which can make an extension report the wrong event
> > until
> > recompiled. Extensions citus, pg_bulkload, and vector refer to static
> > events.
> > If a back-patch added WAIT_EVENT_MESSAGE_QUEUE_SOMETHING_NEW, an old-build
> > pg_bulkload report of WAIT_EVENT_PARALLEL_CREATE_INDEX_SCAN would show up in
> > pg_stat_activity as WAIT_EVENT_PARALLEL_BITMAP_SCAN. (WAIT_EVENT_EXTENSION
> > is
> > not part of a generated enum, fortunately.)
Nice catch, thanks!
> I see an option (4), similar to your (2) without the per-line
> annotation: add a new magic keyword like the existing "Section" that
> is used in the first lines of generate-wait_event_types.pl where we
> generate tab-separated lines with the section name as prefix of each
> line. So I can think of something like:
> Section: ClassName - WaitEventFoo
> FOO_1 "Waiting in foo1"
> FOO_2 "Waiting in foo2"
> Backpatch:
> BAR_1 "Waiting in bar1"
> BAR_2 "Waiting in bar2"
>
> Then force the ordering for the docs and keep the elements in the
> backpatch section at the end of the enums in the order in the txt.
Yeah I think that's a good idea.
> One thing that could make sense is to enforce that "Backpatch" is at
> the end of a section, meaning that we would need a second keyword like
> a "Section: EndBackpatch" or something like that. That's not strictly
> necessary IMO as the format of the txt is easy to follow.
I gave it a try in the POC patch attached. I did not use a "EndBackpatch"
section to keep the perl script as simple a possible though (but documented the
expectation instead).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
>From f93412201df9fa9156cee0b2492a25ac89ff0921 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot...@gmail.com>
Date: Mon, 18 Mar 2024 08:34:19 +0000
Subject: [PATCH v1] Add a "Backpatch" section 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"
section 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 | 10 +++++--
2 files changed, 33 insertions(+), 3 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..62c835df2e 100644
--- a/src/backend/utils/activity/wait_event_names.txt
+++ b/src/backend/utils/activity/wait_event_names.txt
@@ -24,7 +24,12 @@
# 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. If the wait event is backpatched to a version < 17 then
+# put it under a "Backpatch" delimiter at the end of the related ClassName
+# section.
+# Ensure that the wait events under the "Backpatch" delimiter are placed in the
+# same order as in the pre 17 wait_event_types.h (see VERSION_FILE_SYNC as an
+# example).
#
# WaitEventLWLock and WaitEventLock have their own C code for their wait event
# enums and function names. Hence, for these, only the SGML documentation is
@@ -239,7 +244,6 @@ TIMELINE_HISTORY_WRITE "Waiting for a write of a newly created timeline history
TWOPHASE_FILE_READ "Waiting for a read of a two phase state file."
TWOPHASE_FILE_SYNC "Waiting for a two phase state file to reach durable storage."
TWOPHASE_FILE_WRITE "Waiting for a write of a two phase state file."
-VERSION_FILE_SYNC "Waiting for the version file to reach durable storage while creating a database."
VERSION_FILE_WRITE "Waiting for the version file to be written while creating a database."
WALSENDER_TIMELINE_HISTORY_READ "Waiting for a read from a timeline history file during a walsender timeline command."
WAL_BOOTSTRAP_SYNC "Waiting for WAL to reach durable storage during bootstrapping."
@@ -256,6 +260,8 @@ 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:
+VERSION_FILE_SYNC "Waiting for the version file to reach durable storage while creating a database."
#
# Wait Events - Buffer Pin
--
2.34.1
