Re: Support to define custom wait events for extensions

On 2023-07-31 10:10, Michael Paquier wrote:
> Attached is a new version.

Thanks for all the improvements.
I have some comments for v10.

(1)

<note>
<para>
- Extensions can add <literal>LWLock</literal> types to the list
shown in
- <xref linkend="wait-event-lwlock-table"/>. In some cases, the
name
+ Extensions can add <literal>Extension</literal> and
+ <literal>LWLock</literal> types
+ to the list shown in <xref linkend="wait-event-extension-table"/>
and
+ <xref linkend="wait-event-lwlock-table"/>. In some cases, the name
assigned by an extension will not be available in all server
processes;
- so an <literal>LWLock</literal> wait event might be reported as
- just <quote><literal>extension</literal></quote> rather than the
+ so an <literal>LWLock</literal> or <literal>Extension</literal>
wait
+ event might be reported as just
+ <quote><literal>extension</literal></quote> rather than the
extension-assigned name.
</para>
</note>

I think the order in which they are mentioned should be matched. I mean
that
- so an <literal>LWLock</literal> or <literal>Extension</literal>
wait
+ so an <literal>Extension</literal> or <literal>LWLock</literal>
wait

(2)

/* This should only be called for user-defined wait event. */
if (eventId < NUM_BUILTIN_WAIT_EVENT_EXTENSION)
ereport(ERROR,
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("invalid wait event ID %u", eventId));

I was just wondering if it should also check the eventId
that has been allocated though it needs to take the spinlock
and GetWaitEventExtensionIdentifier() doesn't take it into account.

Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION