RE: [PATCH] postgres_fdw connection caching - cause remote sessions linger till the local session exit

Hi

> Discussion here is on the point - whether to show up the invalidated
> connections in the output of the new postgres_fdw_get_connections()
> function? If we were to show, then because of the solution we proposed for
> the connection leak problem in [1], will the invalidated entries be shown
> every time?

IMO, we introduced the function postgres_fdw_get_connections to decide
whether there are too many connections exists and we should disconnect them.

If User decide to disconnect, we have two cases:
1. user decide to disconnect one of them,
I think it’s ok for user to disconnect invalidated connection, so we'd better list the invalidated connections.

2. User decide to disconnect all of them. In this case,
It seems postgres_fdw_disconnect will disconnect both invalidated and not connections,
And we should let user realize what connections they are disconnecting, so we should list the invalidated connections.

Based on the above two cases, Personlly, I think we can list the invalidated connections.

-----
I took a look into the patch, and have a little issue:

+bool disconnect_cached_connections(uint32 hashvalue, bool all)
+ if (all)
+ {
+ hash_destroy(ConnectionHash);
+ ConnectionHash = NULL;
+ result = true;
+ }

If disconnect_cached_connections is called to disconnect all the connections,
should we reset the 'xact_got_connection' flag ?

> [1] -
> https://www.postgresql.org/message-id/flat/CALj2ACVNcGH_6qLY-4_tXz8JLv
> A%2B4yeBThRfxMz7Oxbk1aHcpQ%40mail.gmail.com

The patch about connection leak looks good to me.
And I have a same issue about the new 'have_invalid_connections' flag,
If we disconnect all the connections, should we reset the flag ?

Best regards,
houzj