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-04-22 13:36:05
Message-ID: 2e649ed7-6c66-5d6a-16d4-5d5367d40e69@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 4/20/23 3:09 AM, Michael Paquier wrote:
> On Wed, Mar 29, 2023 at 02:51:27PM +0200, Drouvot, Bertrand wrote:
>> Just realized that more polishing was needed.
>>
>> Done in V2 attached.
>
> That would be pretty cool to get that done in an automated way, I've
> wanted that for a few years now. And I guess that a few others have
> the same feeling after missing to update these docs when adding a new
> wait event, or just to enforce this alphabetically, so let's do
> something about it in v17.

Thanks for the feedback!

> About the alphabetical order, could we have the script enforce a sort
> of the elements parsed from waiteventnames.txt, based on the second
> column? This now relies on the order of the items in the file, but
> my history with this stuff has proved that forcing an ordering rule
> would be a very good thing long-term.

Not having the lines in order would not have been a problem for the perl script
(as it populated the hash table based on the category column while reading the
text file).

That said I do agree that enforcing an order is a good idea, as it's "easier" to read
the generated output files (their content is now somehow "ordered").

This is done in V3 attached.

> Seeing waiteventnames.txt, I think that we should have something
> closer to errcodes.txt. Well, seeing the patch, I assume that this is
> inspired by errcodes.txt, but this new file should be able to do more
> IMO:
> - Supporting the parsing of comments, by ignoring them in
> generate-waiteventnames.pl.
> - Ignore empty likes.
> - Add a proper header, copyright, the output generated from it, etc.
> - Document the format lines of the file.
>

Fully agree, it's done in V3 attached.

> It is clear that the format of the file is:
> - category
> - C symbol in enums.
> - Format in the system views.
> - Description in the docs.
> Or perhaps it would be better to divide this file by sections (like
> errcodes.txt) for each category so as we eliminate entirely the first
> column?
>

Yeah, that could be an option. V3 is still using the category as the first column
but I'm ok to change it by a section if you prefer (though I don't really see the need).

> Perhaps waiteventnames.c should be named pgstat_wait_event.c?

Agree, done.

> Similarly,
> wait_event_types.h would be a better name for the set of enums?
>

Also agree, done.

> utils/adt/jsonpath_scan.c \
> + utils/activity/waiteventnames.c \
> + utils/activity/waiteventnames.h \
> + utils/adt/jsonpath_scan.c \
>
> Looks like a copy-pasto.

Why do you think so? both files have to be removed.

Regards,

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

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2023-04-22 14:12:06 Re: run pgindent on a regular basis / scripted manner
Previous Message Magnus Hagander 2023-04-22 13:23:59 Re: run pgindent on a regular basis / scripted manner