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