Received: from malur.postgresql.org ([217.196.149.56])
	by arkaria.postgresql.org with esmtps  (TLS1.3) tls
 TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
	(Exim 4.94.2)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1qQMmh-00DfQg-0R
	for pgsql-hackers@arkaria.postgresql.org; Mon, 31 Jul 2023 06:53:38 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.94.2)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1qQMmf-00CPSF-79
	for pgsql-hackers@arkaria.postgresql.org; Mon, 31 Jul 2023 06:53:37 +0000
Received: from makus.postgresql.org ([2001:4800:3e1:1::229])
	by malur.postgresql.org with esmtps  (TLS1.3) tls
 TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
	(Exim 4.94.2)
	(envelope-from <ikedamsh@oss.nttdata.com>)
	id 1qQMme-00CPS6-Tl
	for pgsql-hackers@lists.postgresql.org; Mon, 31 Jul 2023 06:53:36 +0000
Received: from oss.nttdata.com ([49.212.34.109])
	by makus.postgresql.org with esmtps  (TLS1.2) tls
 TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384
	(Exim 4.94.2)
	(envelope-from <ikedamsh@oss.nttdata.com>)
	id 1qQMmZ-002WFr-Ro
	for pgsql-hackers@lists.postgresql.org; Mon, 31 Jul 2023 06:53:35 +0000
Received: from oss.nttdata.com (localhost [127.0.0.1])
	by oss.nttdata.com (Postfix) with ESMTPA id 2DAD360416;
	Mon, 31 Jul 2023 15:53:27 +0900 (JST)
X-Virus-Status: Clean
X-Virus-Scanned: clamav-milter 0.103.8 at oss.nttdata.com
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Mon, 31 Jul 2023 15:53:27 +0900
From: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Tristan
 Partin <tristan@neon.tech>, andres@anarazel.de,
 pgsql-hackers@lists.postgresql.org
Subject: Re: Support to define custom wait events for extensions
In-Reply-To: <ZMcJ7F7nkGkIs8zP@paquier.xyz>
References: <ZK9BdDI6KMxmiG+5@paquier.xyz>
 <cb7d5257d1b1a25aa405c219732aaf8c@oss.nttdata.com>
 <ZK9iLbi2l2rrELfV@paquier.xyz>
 <97449ae1a3b2189934890f6d78b18216@oss.nttdata.com>
 <ca567bd4076dfc7e8b0d39afe5b0cc22@oss.nttdata.com>
 <CU69THFPG2CN.MK16RYW510YD@gonk> <ZMI1/tQp/TQ1/m3B@paquier.xyz>
 <cd09c86daa9e4c1c5f7903da39b75994@oss.nttdata.com>
 <ZMMUiR7kvzPWenhF@paquier.xyz>
 <CALj2ACVq=BogP=iQ0C=Pjzk4+rSTaKZU_+fdMRk4YyG8B60gAQ@mail.gmail.com>
 <ZMcJ7F7nkGkIs8zP@paquier.xyz>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <446d286f3272437df705ee69ad0d8793@oss.nttdata.com>
X-Sender: ikedamsh@oss.nttdata.com
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: 
 <https://www.postgresql.org/message-id/446d286f3272437df705ee69ad0d8793%40oss.nttdata.com>
Precedence: bulk

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