RE: [Proposal] Add foreign-server health checks infrastructure

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>
To: 'Zhihong Yu' <zyu@yugabyte.com>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Shinya Kato <Shinya11.Kato@oss.nttdata.com>
Date: 2022-01-20T06:58:31Z
Lists: pgsql-hackers
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

Commits

  1. postgres_fdw: Extend postgres_fdw_get_connections to return user name.

  2. postgres_fdw: Fix bug in connection status check.

  3. postgres_fdw: Add connection status check to postgres_fdw_get_connections().

  4. postgres_fdw: Add "used_in_xact" column to postgres_fdw_get_connections().

  5. doc: Enhance documentation for postgres_fdw_get_connections() output columns.

  6. Avoid reference to nonexistent array element in ExecInitAgg().

  7. Add WL_SOCKET_CLOSED for socket shutdown events.

  8. postgres_fdw: reestablish new connection if cached one is detected as broken.