Re: Autogenerate some wait events code and documentation

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-04-22T13:36:05Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Add "ABI_compatibility" regions to wait_event_names.txt

  2. Remove column for wait event names in wait_event_names.txt

  3. Use more consistent names for wait event objects and types

  4. Add information about line contents on parsing failure of wait_event_names.txt

  5. Remove double quotes from the second column of wait_event_names.txt

  6. Generate automatically code and documentation related to wait events

  7. Refactor some code related to wait events "BufferPin" and "Extension"

  8. doc: Fix ordering of entries in wait event table for I/O type