Re: WIP: new system catalog pg_wait_event

From: Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com>
To: "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, tgl(at)sss(dot)pgh(dot)pa(dot)us, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: WIP: new system catalog pg_wait_event
Date: 2023-08-17 01:53:02
Message-ID: 735fbd560ae914c96faaa23cc8d9a118@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-08-17 01:57:04 Re: WIP: new system catalog pg_wait_event
Previous Message Tatsuo Ishii 2023-08-17 01:37:58 Re: pgbench: allow to exit immediately when any client is aborted