Dear Ted,

Thank you for reviewing! PSA new version.

> +       /* quick exit if connection cache has been not initialized yet. */
>
> been not initialized -> not been initialized

Fixed.

> +                                               
> (errcode(ERRCODE_CONNECTION_FAILURE),
> +                                                errmsg("could not connect to 
> server \"%s\"",
>
> Currently each server which is not connected would log a warning.
> Is it better to concatenate names for such servers and log one line ? This 
> would be cleaner when there are multiple such servers.

Sounds good, fixed as you said. The following shows the case that two 
disconnections
are detected by postgres_fdw_verify_connection_states_all().

```
postgres=*# select postgres_fdw_verify_connection_states_all ();
WARNING:  could not connect to server "my_external_server2", 
"my_external_server"
DETAIL:  Socket close is detected.
HINT:  Plsease check the health of server.
 postgres_fdw_verify_connection_states_all 
-------------------------------------------
 f
(1 row)
```

Currently, the name of servers is concatenated without doing unique checks. IIUC
a backend process cannot connect to the same foreign server by using different
user mapping, so there is no possibility that the same name appears twice.
If the user mapping is altered in the transaction, the cache entry is 
invalidated
and will not be checked.


Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment: v24-0001-Add-PQConncheck-and-PQCanConncheck-to-libpq.patch
Description: v24-0001-Add-PQConncheck-and-PQCanConncheck-to-libpq.patch

Attachment: v24-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch
Description: v24-0002-postgres_fdw-add-postgres_fdw_verify_connection_.patch

Attachment: v24-0003-add-test.patch
Description: v24-0003-add-test.patch

Reply via email to