Dear Zhihong,
Thank you for reviewing! And I have to apologize for the late.
I'll post new patchset later.
> + UnregisterAllCheckingRemoteServersCallback();
>
> UnregisterAllCheckingRemoteServersCallback
> -> UnregisterAllCheckingRemoteServersCallbacks
Will fix.
> + CheckingRemoteServersCallbackItem *item;
> + item = fdw_callbacks;
>
> The above two lines can be combined.
Will fix.
> +UnregisterCheckingRemoteServersCallback(CheckingRemoteServersCallback
> callback,
> + void *arg)
>
> Is the above func called anywhere ?
Currently not, I just kept the API because of any other FDW extensions.
But I cannot find any use cases, so will remove.
> + if (item->callback == callback && item->arg == arg)
>
> Is comparing argument pointers stable ? What if the same argument is cloned
> ?
This part is no longer needed. Will remove.
> + CallCheckingRemoteServersCallbacks();
> +
> + if (remote_servers_connection_check_interval > 0)
> + enable_timeout_after(CHECKING_REMOTE_SERVERS_TIMEOUT,
> +
> remote_servers_connection_check_interval);
>
> Should the call to CallCheckingRemoteServersCallbacks() be placed under the
> if block checking remote_servers_connection_check_interval ?
> If remote_servers_connection_check_interval is 0, it seems the callbacks
> shouldn't be called.
Agreed. We force stopping timeout when the GUC sets to 0 in assign_hook,
so your saying is consistent. Will move.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED