Re: WIP: new system catalog pg_wait_event
От | Drouvot, Bertrand |
---|---|
Тема | Re: WIP: new system catalog pg_wait_event |
Дата | |
Msg-id | d28ad85d-1deb-4648-92a0-f38273fc91ca@gmail.com обсуждение исходный текст |
Ответ на | Re: WIP: new system catalog pg_wait_event (Masahiro Ikeda <ikedamsh@oss.nttdata.com>) |
Ответы |
Re: WIP: new system catalog pg_wait_event
(Masahiro Ikeda <ikedamsh@oss.nttdata.com>)
|
Список | pgsql-hackers |
Hi, On 8/17/23 3:53 AM, Masahiro Ikeda wrote: > 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. > Thanks for looking at it! > 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 > Yeah, nice catch. v7 just shared up-thread replace "-" by "_" for such a case. But I'm not sure the pg_wait_event description field needs to be 100% aligned with the documentation: wouldn't be better to replace "-" by " " in such cases in pg_wait_event? > * The HTML tag match is not shortest match. > -frozenid Waiting to update pg_database.datfrozenxid and pg_database.datminmxid > +frozenid Waiting to update datminmxid > Nice catch, fixed in v7. > * There are two blanks before "about". This is coming from wait_event_names.txt, thanks for pointing out. I just submitted a patch [1] to fix that. > 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 > Not intended, fixed in v7. > * 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 > Do you mean remove the wait event name from the description in case of custom extension wait events? I'd prefer to keep it, if not the descriptions would be all the same for custom wait events. > 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 nameof 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. > Yeah, could make sense but I think that should be a dedicated patch for the documentation. > 3) > > Would index == els be better for the following Assert? > + Assert(index <= els); > Agree, done in v7. > 4) > > There is a typo. alll -> all > + /* Allocate enough space for alll entries */ > Thanks! Fixed in v7. [1]: https://www.postgresql.org/message-id/dd836027-2e9e-4df9-9fd9-7527cd1757e1%40gmail.com Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
В списке pgsql-hackers по дате отправления: