Re: Autogenerate some wait events code and documentation

On Sun, Mar 17, 2024 at 11:31:14AM -0700, Noah Misch wrote:
> I like the new developer experience of adding a wait event. After release of
> v17, how should we approach back-patching an event, like was done in commits
> 8fa4a1a 1396b5c 78c0f85? Each of those commits put the new event at the end
> of its released-branch wait_event.h enum. In v17,
> generate-wait_event_types.pl sorts events to position them.

Indeed, that would be a bad idea.

> 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.) Some options:
>
> 1. Don't back-patch wait events to v17+. Use the closest existing event.
> 2. Let wait_event_names.txt back-patches control the enum order. For example,
> a line could have an annotation that controls its position relative to the
> auto-sorted lines. For another example, the generator could stop sorting.
> 3. Accept the renumbering, because the consequence isn't that horrible.
>
> Option (3) is worse than (1), but I don't have a recommendation between (1)
> and (2). I tend to like (1), a concern being the ease of accidental
> violations. If we had the ABI compliance checking that
> https://postgr.es/m/flat/CAH2-Wzk7tvgLXzOZ8a22aF-gmO5gHojWTYRvAk5ZgOvTrcEQeg(at)mail(dot)gmail(dot)com
> explored, (1) would be plenty safe. Should anything change here, or not?

(1) would be annoying, we have backpatched scaling problems in the
past even if that does not happen often. And in some cases I can
understand why one would want to add a new wait event to track that
a patch does what is expected of it. (2) to stop the automated
sorting would bring back the problems that this thread has spent time
to solve: people tend to not add wait events correctly, so I would
suspect issues on HEAD. I've seen that too many times on older
branches.

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.
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.
--
Michael