Re: WIP: new system catalog pg_wait_event

Masahiro Ikeda <ikedamsh@oss.nttdata.com>

From: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
To: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Date: 2023-08-17T01:53:02Z
Lists: pgsql-hackers
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



Commits

  1. doc: Improve example query related to pg_wait_events

  2. Add system view pg_wait_events