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