Received: from malur.postgresql.org ([217.196.149.56])
	by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1pZMb2-0007sZ-Tt
	for pgsql-hackers@arkaria.postgresql.org; Tue, 07 Mar 2023 01:58:32 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.92)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1pZMb1-0001xb-BV
	for pgsql-hackers@arkaria.postgresql.org; Tue, 07 Mar 2023 01:58:31 +0000
Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29])
	by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <katsuragiy@oss.nttdata.com>)
	id 1pZMb1-0001xS-1Z
	for pgsql-hackers@lists.postgresql.org; Tue, 07 Mar 2023 01:58:31 +0000
Received: from oss.nttdata.com ([49.212.34.109])
	by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <katsuragiy@oss.nttdata.com>)
	id 1pZMax-0007Qn-Kk
	for pgsql-hackers@lists.postgresql.org; Tue, 07 Mar 2023 01:58:30 +0000
Received: from oss.nttdata.com (localhost [127.0.0.1])
	by oss.nttdata.com (Postfix) with ESMTPA id F3A2360199;
	Tue,  7 Mar 2023 10:58:21 +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: Tue, 07 Mar 2023 10:58:21 +0900
From: Katsuragi Yuta <katsuragiy@oss.nttdata.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: 'Ted Yu' <yuzhihong@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, vignesh C
 <vignesh21@gmail.com>, pgsql-hackers@lists.postgresql.org,
 =?UTF-8?Q?=C3=96nder_Kalac=C4=B1?= <onderkalaci@gmail.com>, Fujii Masao
 <masao.fujii@oss.nttdata.com>, Shinya11.Kato@oss.nttdata.com, Kyotaro
 Horiguchi <horikyota.ntt@gmail.com>
Subject: Re: [Proposal] Add foreign-server health checks infrastructure
In-Reply-To: <TYCPR01MB58700498838127C54BC9BD48F5B69@TYCPR01MB5870.jpnprd01.prod.outlook.com>
References: <TYAPR01MB58669EAAC02493BFF9F39B06F5D99@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <e0af630a74a02cdd031af52d7f17359e@oss.nttdata.com>
 <TYAPR01MB58669C95604A0EB7BCC1B58CF5A49@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <f09b19e3bf464eeeae21464c2051024a@oss.nttdata.com>
 <TYAPR01MB5866904CE4F2C30E1CEEFC04F5AA9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <3cb43b39672c17786566063b0aa1225d@oss.nttdata.com>
 <TYAPR01MB586674A3C56AECB9B150BC84F5B39@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <f94c09551891249fe316bfe55015b3e7@oss.nttdata.com>
 <TYCPR01MB58700498838127C54BC9BD48F5B69@TYCPR01MB5870.jpnprd01.prod.outlook.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <f723a26e8c34001ae32e3399951b8066@oss.nttdata.com>
X-Sender: katsuragiy@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/f723a26e8c34001ae32e3399951b8066%40oss.nttdata.com>
Precedence: bulk

Hi Kuroda-san,

Thank you for updating the patch!

I think we can update the status to ready for committer after
this fix, if there is no objection.

>> 7. the document of postgres_fdw_verify_connection_states_all
>> <literal>NULL</literal>
>> +      is returned if the local session does not have connection 
>> caches,
>> or this
>> +      function is not available on this platform.
>> 
>> I think there is a case where a connection cache exists but valid
>> connections do not exist and NULL is returned (disconnection case).
>> Almost the same document as the postgres_fdw_verify_connection_states
>> case (comment no.5) seems better.
> 
> Yes, but completely same statement cannot be used because these is not
> specified foreign server. How about:
> NULL is returned if there are no established connections or this 
> function ...

Yes, to align with the postgres_fdw_verify_connection_states()
case, how about writing the connection is not valid. Like the
following?
'NULL is returned if no valid connections are established or
this function...'


This is my comment for v35. Please check.
0002:
1. the comment of verify_cached_connections (I missed one minor point.)
+ * This function emits warnings if a disconnection is found. This 
returns true
+ * if disconnections cannot be found, otherwise returns false.

I think false is returned only if disconnections are found and
true is returned in all other cases. So, modifying the description
like the following seems better.
'This returns false if disconnections are found, otherwise
returns true.'


regards,

-- 
Katsuragi Yuta
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION