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

Hi,

This patch introduces new function postgres_fdw_disconnect() when
called with a foreign server name discards the associated
connections with the server name.

I think the following would read better:

This patch introduces *a* new function postgres_fdw_disconnect(). When
called with a foreign server name, it discards the associated
connections with the server.

Please note the removal of the 'name' at the end - connection is with
server, not server name.

+ if (is_in_use)
+ ereport(WARNING,
+ (errmsg("cannot close the connection because it is
still in use")));

It would be better to include servername in the message.

+ ereport(WARNING,
+ (errmsg("cannot close all connections because some
of them are still in use")));

I think showing the number of active connections would be more informative.
This can be achieved by changing active_conn_exists from bool to int (named
active_conns, e.g.):

+ if (entry->conn && !active_conn_exists)
+ active_conn_exists = true;

Instead of setting the bool value, active_conns can be incremented.

Cheers

On Sat, Jan 16, 2021 at 11:39 PM Bharath Rupireddy <
bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:

> On Sat, Jan 16, 2021 at 10:36 AM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > > > Please consider the v9 patch set for further review.
> > >
> > > Thanks for updating the patch! I reviewed only 0001 patch.
>
> I addressed the review comments and attached v10 patch set. Please
> consider it for further review.
>
>
> With Regards,
> Bharath Rupireddy.
> EnterpriseDB: http://www.enterprisedb.com
>