Re: Introduce WAIT_EVENT_EXTENSION and WAIT_EVENT_BUFFER_PIN - Mailing list pgsql-hackers

On Mon, May 15, 2023 at 06:01:02PM -0700, Andres Freund wrote:
> On 2023-05-16 09:38:54 +0900, Michael Paquier wrote:
>> On Mon, May 15, 2023 at 05:17:16PM -0700, Andres Freund wrote:
>>> IMO the submission should include why automating requires these changes (yours
>>> doesn't really either). I can probably figure it out if I stare a bit at the
>>> code and read the other thread - but I shouldn't need to.
>>
>> Hm?  My previous message includes two reasons..
>
> It explained the motivation, but not why that requires the specific
> changes. At least not in a way that I could quickly undestand.
>
>> The extensions and buffer pin parts need a few internal tweaks to make
>> the other changes much easier to do, which is what the patch of this
>> thread is doing.
>
> Why those tweaks are necessary is precisely what I am asking for.

Not sure how to answer to that without copy-pasting some code, and
that's what is written in the patch.  Anyway, I am used to the context
of what's wanted, so here you go with more details.  As a whole, the
patch reshapes the code to be more consistent for all the wait event
classes, so as it is possible to generate the code refactored by the
patch of this thread in a single way for all the classes.

The first change is related to the functions associated to each class,
used to fetch a specific wait event.  On HEAD, all the wait event classes
use a dedicated function (activity, IPC, etc.) like that:
        case PG_WAIT_BLAH:
    {
            WaitEventBlah w = (WaitEventBlah) wait_event_info;
            event_name = pgstat_get_wait_blah(w);
            break;
    }

There are two exceptions to that, the wait event classes for extension
and buffer pin, that just do that because these classes have only one
wait event currently:
        case PG_WAIT_EXTENSION:
           event_name = "Extension";
           break
        [...]
        case PG_WAIT_BUFFER_PIN:
            event_name = "BufferPin";
            break;
The first thing changed is to introduce two new functions for these
two classes, to work the same way as the other classes.  The script in
charge of generating the code from the wait event .txt file will just
build the internals of these functions.

The second change is to rework the enum structures for extension and
buffer pin, to be consistent with the other classes, so as all the
classes have structures shaped like that:
typedef enum
{
   WAIT_EVENT_1 = PG_WAIT_BLAH,
   [...]
   WAIT_EVENT_N
} WaitEventBlah;

Then the perl script generates the same structures for all the wait
event classes, with all the events associated to that.  There may be a
point in keeping extension and buffer pin out of this refactoring, but
reworking these like that moves in a single place all the wait event
definitions, making future additions easier.  Buffer pin actually
needed a small rename to stick with the naming rules of the other
classes.

These are the two things refactored in the patch, explaining the what.
The reason behind the why is to make the script in charge of
generating all these structures and functions consistent for all the
wait event classes, simply.  Treating all the wait event classes
together eases greatly the generation of the documentation, so that it
is possible to enforce an ordering of the tables of each class used to
list each wait event type attached to them.  Does this explanation
make sense?

Lock and LWLock are funkier because of the way they grab wait events
for the inner function, still these had better have their
documentation generated so as all the SGML tables created for all the
wait event tables are ordered alphabetically, in the same way as
HEAD.
--
Michael