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


Reply via email to