Re: [Proposal] Add foreign-server health checks infrastructure - Mailing list pgsql-hackers

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-callbackcan 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
thetimeout 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