Message-ID: <0e9ea094-67df-487c-96f5-f9eabb5357c7@oss.nttdata.com>
Date: Sun, 23 Jan 2022 14:36:02 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.5.0
Subject: Re: [Proposal] Add foreign-server health checks infrastructure
Content-Language: en-US
To: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>,
 "zyu@yugabyte.com" <zyu@yugabyte.com>
Cc: "pgsql-hackers@lists.postgresql.org"
 <pgsql-hackers@lists.postgresql.org>,
 'Shinya Kato' <Shinya11.Kato@oss.nttdata.com>
References: <TYAPR01MB58662809E678253B90E82CE5F5889@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <54449d7bea79263e937266ad6ebc65b3@oss.nttdata.com>
 <TYAPR01MB58662CD4FD98AA475B3D10F9F59B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <4e64bb8a0a88f5b6ac5a8901377c81af@oss.nttdata.com>
 <TYAPR01MB58661992162BA7708191EC79F5619@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <CALNJ-vQWNK6AzpL=5iXLfpr4oRMYdJ72tDYhxLwd=iX5H5XUdQ@mail.gmail.com>
 <TYAPR01MB5866D73C56A4363039175E56F5669@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <CALNJ-vSDLaDnbJpDuKTKwugX+dUnoLD+eEXHdR-aBRXG1Dh3hA@mail.gmail.com>
 <b9076e887ec768463428ecccf4e63a26@oss.nttdata.com>
 <TYAPR01MB5866BC188389FA07C0E6CFE1F5769@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <db3533c9-42ea-0ef4-2d70-8dfc016e02b5@oss.nttdata.com>
 <TYAPR01MB58667F6B6773141CE81D6AA5F55A9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
 <TYAPR01MB58669361410DFD6B01DF45E3F55B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
In-Reply-To: <TYAPR01MB58669361410DFD6B01DF45E3F55B9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/0e9ea094-67df-487c-96f5-f9eabb5357c7%40oss.nttdata.com>
Precedence: bulk


On 2022/01/21 15:08, kuroda.hayato@fujitsu.com wrote:
> Dear Fujii-san, Zhihong,
> 
> I attached the latest version.

Thanks for updating the patch!

+int
+TryDisableRemoteServerCheckingTimeout(void)

When more than one FDWs are used, even if one FDW calls this function to disable the timeout, its remote-server-check-callback can still be called. Is this OK?
Please imagine the case where two FDWs are used and they registered their callbacks. Even when one FDW calls TryDisableRemoteServerCheckingTimeout(), if another FDW has not called that yet, the timeout is still being enabled. If the timeout is triggered during that period, even the callback registered by the FDW that has already called TryDisableRemoteServerCheckingTimeout() would be called.


+			if (remote_servers_connection_check_interval > 0)
+			{
+				CallCheckingRemoteServersCallbacks();
+				enable_timeout_after(CHECKING_REMOTE_SERVERS_TIMEOUT,
+									 remote_servers_connection_check_interval);

LockErrorCleanup() needs to be called before reporting the error, doesn't it?


This can cause an error even while DoingCommandRead == true. Is that safe?


> The biggest change is that callbacks are no longer un-registered at the end of transactions.
> FDW developer must enable or disable timeout instead, via new APIs.
> 
> The timer will be turned on when:
> * new GUC is >= 0, and

This can cause the timeout to be enabled even when no remote transaction is started?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION