From: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
To: Drouvot, Bertrand <bertranddrouvot.pg@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: tgl@sss.pgh.pa.us
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: WIP: new system catalog pg_wait_event
Date: Thu, 17 Aug 2023 10:53:02 +0900
Message-ID: <735fbd560ae914c96faaa23cc8d9a118@oss.nttdata.com> (raw)
In-Reply-To: <f2204399-cfdb-40e7-9479-9373ec6ba0ab@gmail.com>
References: <0109438a-9672-322b-7638-04ad47c97d0f@gmail.com>
<1dc98591-0d06-f0da-bc88-21f529d6fb8e@gmail.com>
<20230808.115332.301701413576096139.horikyota.ntt@gmail.com>
<ZNGw9T2t5z5Kg1xd@paquier.xyz>
<b21e5706-b6dd-42f1-a744-3afffe87c2c9@gmail.com>
<c915434a-3d3a-4135-8f3a-fca0cf22c6ae@gmail.com>
<ZNNGq7DTv9ur5113@paquier.xyz>
<b71d1977-1a8b-4129-8c03-533031dbd004@gmail.com>
<ZNmvegQirGI9ifVw@paquier.xyz>
<2f84b397-d020-493c-924c-610b9dd18f8d@gmail.com>
<ZNxrMdmgrlovw1qD@paquier.xyz>
<f2204399-cfdb-40e7-9479-9373ec6ba0ab@gmail.com>
Hi,
Thank you for creating the patch!
I think it is a very useful view as a user.
I will share some thoughts about the v6 patch.
1)
The regular expression needs to be changed in
generate-wait_event_types.pl.
I have compared the documentation with the output of the pg_wait_event
view and found the following differences.
* For parameter names, the substitution for underscore is missing.
-ArchiveCleanupCommand Waiting for archive_cleanup_command to complete
-ArchiveCommand Waiting for archive_command to complete
+ArchiveCleanupCommand Waiting for archive-cleanup-command to complete
+ArchiveCommand Waiting for archive-command to complete
-RecoveryEndCommand Waiting for recovery_end_command to complete
+RecoveryEndCommand Waiting for recovery-end-command to complete
-RestoreCommand Waiting for restore_command to complete
+RestoreCommand Waiting for restore-command to complete
* The HTML tag match is not shortest match.
-frozenid Waiting to update pg_database.datfrozenxid and
pg_database.datminmxid
+frozenid Waiting to update datminmxid
* There are two blanks before "about". Also " for heavy weight is
removed (is it intended?)
-LockManager Waiting to read or update information about "heavyweight"
locks
+LockManager Waiting to read or update information about heavyweight
locks
* Do we need "worker_spi_main" in the description? The name column
shows the same one, so it could be omitted.
> pg_wait_event
> worker_spi_main Custom wait event "worker_spi_main" defined by
> extension
2)
Would it be better to add "extension" meaning unassigned?
> Extensions can add Extension and LWLock types to the list shown in
> Table 28.8 and Table 28.12. In some cases, the name of LWLock assigned
> by an extension will not be available in all server processes; It might
> be reported as just “extension” rather than the extension-assigned
> name.
3)
Would index == els be better for the following Assert?
+ Assert(index <= els);
4)
There is a typo. alll -> all
+ /* Allocate enough space for alll entries */
5)
BTW, although I think this is outside the scope of this patch,
it might be a good idea to be able to add a description to the
API for custom wait events.
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ikedamsh@oss.nttdata.com, bertranddrouvot.pg@gmail.com, michael@paquier.xyz, horikyota.ntt@gmail.com, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: WIP: new system catalog pg_wait_event
In-Reply-To: <735fbd560ae914c96faaa23cc8d9a118@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox