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

Dear Osumi-san,

Thanks for reviewing! PSA v17 patchset.

> (1) v16-0001 : definition of a new structure
> 
> CheckingRemoteServersCallbackItem can be added to typedefs.list.

Added.

> (2) v16-0001 : UnRegisterCheckingRemoteServersCallback's header comment
> 
> +void
> +UnRegisterCheckingRemoteServersCallback(CheckingRemoteServersCallback
> callback,
> +
> void *arg)
> +{
> 
> Looks require a header comment for this function,
> because in this file, all other functions have each one.

Added. Additionally, I renamed this function to Unregister...,
this follows other unregister functions.


> (3) v16-0002 : better place to declare new variables
> 
> New variables 'old' and 'server' in GetConnection() can be moved to
> a scope of 'if' statement where those are used.
> 
> @@ -138,6 +149,8 @@ GetConnection(UserMapping *user, bool will_prep_stmt,
> PgFdwConnState **state)
>         ConnCacheEntry *entry;
>         ConnCacheKey key;
>         MemoryContext ccxt = CurrentMemoryContext;
> +       MemoryContext old;
> +       ForeignServer *server = GetForeignServer(user->serverid);

Fixed.

> (4) v16-0002 : typo in pgfdw_connection_check_internal()
> 
> 
> +       /* return false is if the socket seems to be closed. */
> 
> It should be "return false if the socket seems to be closed" ?

Fixed.

> (5) v16-0002 : pgfdw_connection_check's message
> 
> When I tested the new feature, I got a below message.
> 
> "ERROR:  Foreign Server my_external_server might be down."
> 
> I think we can wrap the server name with double quotes
> so that the message is more aligned with existing error message
> like connect_pg_server.

Fixed.
...I'm not sure whether this message is still need, because
the disconnection was delegated to XactCallback, and the function did following output:

```
WARNING:  FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
       This probably means the server terminated abnormally
       before or while processing the request.
```

> (6) v16-003 : typo
> 
> +      Authors needs to register the callback function via following API.
> 
> Should be "Authors need to register the ...".

Fixed.

> (7) v16-0004 : unnecessary file
> 
> I think we can remove a new file which looks like a log file.
> 
> .../postgres_fdw/expected/postgres_fdw_1.out

This is needed to pass the test on the windows platform. See:
https://www.postgresql.org/docs/devel/regress-variant.html

Best Regards,
Hayato Kuroda
FUJITSU LIMITED