Re: Autogenerate some wait events code and documentation

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Autogenerate some wait events code and documentation
Date: 2023-04-25 05:15:07
Message-ID: ZEdh2+46GuhCqW6b@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 24, 2023 at 09:03:53AM +0200, Drouvot, Bertrand wrote:
> Oh right, fixed.

I may tweak a few things if I put my hands on it, but that looks
pretty solid seen from here.. I have spotted a few extra issues.

One thing I have noticed with v4 is that the order of the tables
generated in wait_event_types.h and the SGML docs is inconsistent with
previous versions, and these are not in an alphabetical order. HEAD
orders them as Activity, BufferPin, Client, Extension, IO, IPC, Lock,
LWLock and Timeout. This patch switches the order to become
different, and note that the first table describing each of the wait
event type classes gets it right.

It seems to me that you should apply an extra ordering in
generate-waiteventnames.pl to make sure that the tables are printed in
the same order as previously, around here:
+# Generate the output files
+foreach $waitclass (keys %hashwe) {

(The table describing all the wait event types could be removed from
the SGML docs as well, at the cost of having their description in the
new .txt file. However, as these are long, it would make the .txt
file much messier, so not doing this extra part is OK for me.)

- * Use this category when a process is waiting because it has no work to do,
- * unless the "Client" or "Timeout" category describes the situation better.
- * Typically, this should only be used for background processes

wait_event.h includes a set of comments describing each category, that
this patch removes. Rather than removing this information, which is
helpful to have around, why not making them comments of
waiteventnames.txt instead? Losing this information would be sad.

+# src/backend/utils/activity/pgstat_wait_event.c
+# c functions to get the wait event name based on the enum
Nit. 'c' should be upper-case.

}
+
if (IsNewer(
'src/include/storage/lwlocknames.h',
Not wrong, but this is an unrelated diff.

+if %DIST%==1 if exist src\backend\utils\activity\pgstat_wait_event.c del /q src\backend\utils\activity\pgstat_wait_event.c
if %DIST%==1 if exist src\backend\storage\lmgr\lwlocknames.h del /q src\backend\storage\lmgr\lwlocknames.h
+if %DIST%==1 if exist src\backend\utils\activity\wait_event_types.h del /q src\backend\utils\activity\wait_event_types.h
The order here is off a bit. Missed that previously..

perltidy had better be run on generate-waiteventnames.pl (I can do
that myself, no worries), as a couple of lines' format don't seem
quite in line.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-04-25 05:17:58 Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
Previous Message Amit Kapila 2023-04-25 04:43:48 Re: Add two missing tests in 035_standby_logical_decoding.pl