Re: Autogenerate some wait events code and documentation

From: "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Autogenerate some wait events code and documentation
Date: 2023-05-17 06:31:53
Message-ID: 976f4eb1-cbc0-9616-ec26-ebb4a252315c@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 5/16/23 9:48 AM, Michael Paquier wrote:
> On Mon, May 15, 2023 at 06:45:23PM +0200, Drouvot, Bertrand wrote:
>> On 5/6/23 4:37 AM, Michael Paquier wrote:
>>> On Sat, May 06, 2023 at 11:23:17AM +0900, Michael Paquier wrote:

>
> BufFileTruncate and BufFileWrite have an incorrect order in HEAD's
> monitoring.sgml (will fix in a minute for 16~). So your patch fixes
> that.

Oh nice catch!

>
> PgStatsDSA and PgStatsData are reversed in your patch compared to
> HEAD, actually, based on the way perl sees fit to do its ordering by
> giving priority to upper-case characters. Same for RelCacheInit and
> RelationMapping, or even SInvalRead/SInvalWrite being now before the
> "Serial" family. Worse, the tables LWLock and Lock are in an
> incorrect order as well with the patch. We'd better be a bit more
> verbose with the sort step, I think.. perl does not handle well
> sorting with collations from what I recall, but we could use uc() with
> a block sort to force the operation to be case-insensitive, like "sort
> {uc($a) cmp uc($b)}". That needs to be applied here, I guess:
> +# Sort the lines based on the third column
> +my @lines_sorted =
> + sort { (split(/\t/, $a))[2] cmp(split(/\t/, $b))[2] } @lines;
>

Oh right, nice catch.

> And it looks like you'd need to apply uc() on each [2] element. I
> would add a comment about this detail, as well.
>

Did it that way in V9 attached and the sorting does look like what we expect now.

> No entries are missing, after comparing what's generated by the patch
> with the contents of HEAD.
>
> Small nit-ish question: waiteventnames.sgml or wait_event_types.sgml?
> Same for generate-waiteventtypes.pl?
>

Agree, it's more consistent. Done that way in V9.

>
> FWIW, I would have posted two patches, one with the refactoring of
> done in [1], and a second that switches to the automation, to make
> clear the preparatory step.
>
> [1]: https://www.postgresql.org/message-id/c6f35117-4b20-4c78-1df5-d3056010dcf5@gmail.com
> --

Agree, V9 does now apply on top of v2-0001-Introducing-WAIT_EVENT_EXTENSION-and-WAIT_EVENT_B.patch
(just shared in [1]).

[1]: https://www.postgresql.org/message-id/a82c2660-64b4-1c59-3eef-bf82b86fb99a%40gmail.com

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v9-0001-Generating-wait_event_types.h-pgstat_wait_event.c.patch text/plain 120.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2023-05-17 06:47:47 Re: Possible regression setting GUCs on \connect
Previous Message Drouvot, Bertrand 2023-05-17 06:29:42 Re: Introduce WAIT_EVENT_EXTENSION and WAIT_EVENT_BUFFER_PIN