Re: Autogenerate some wait events code and documentation
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-05-16T07:48:35Z
Lists: pgsql-hackers
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:
>>>> I'll look at v7 when the v17 branch opens and propose the separate patch
>>>> mentioned above at that time too.
>>>
>>> Thanks, again.
>>
>> By the way, while browsing through the patch, I have noticed two
>> things:
>> - The ordering of the items for Lock and LWLock is incorrect.
>
> Oh right, fixed in V8 attached (moved the sort on the third column
> instead of the second which has always the same content "WAIT_EVENT_DOCONLY"
> for Lock and LWLock).
Ah, I didn't notice that. Makes sense.
>> - We are missing some of the LWLock entries, like CommitTsBuffer,
>> XactBuffer or WALInsert, as of all the entries in
>> BuiltinTrancheNames.
>
> Yeah, my bad. Fixed in V8 attached.
BufFileTruncate and BufFileWrite have an incorrect order in HEAD's
monitoring.sgml (will fix in a minute for 16~). So your patch fixes
that.
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;
And it looks like you'd need to apply uc() on each [2] element. I
would add a comment about this detail, as well.
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?
>> My apologies for not noticing that earlier. This exists in v6 as much
>> as v7.
>
> No problem at all and thanks for the call out!
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
--
Michael
Commits
-
Add "ABI_compatibility" regions to wait_event_names.txt
- f98dbdeb51d1 17.0 landed
-
Remove column for wait event names in wait_event_names.txt
- 59cbf60c0f2b 17.0 landed
-
Use more consistent names for wait event objects and types
- 414f6c0fb79a 17.0 landed
-
Add information about line contents on parsing failure of wait_event_names.txt
- aea7fe33fb6a 17.0 landed
-
Remove double quotes from the second column of wait_event_names.txt
- 183a60a628fe 17.0 landed
-
Generate automatically code and documentation related to wait events
- fa88928470b5 17.0 landed
-
Refactor some code related to wait events "BufferPin" and "Extension"
- 2aeaf80e578e 17.0 landed
-
doc: Fix ordering of entries in wait event table for I/O type
- 44e977d7ecb9 16.0 landed