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

On Mon, Jun 22, 2020 at 11:26 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> When a query on foreign table is executed from a local session using
> postgres_fdw, as expected the local postgres backend opens a
> connection which causes a remote session/backend to be opened on the
> remote postgres server for query execution.
>
> One observation is that, even after the query is finished, the remote
> session/backend still persists on the remote postgres server. Upon
> researching, I found that there is a concept of Connection Caching for
> the remote connections made using postgres_fdw. Local backend/session
> can cache up to 8 different connections per backend. This caching is
> useful as it avoids the cost of reestablishing new connections per
> foreign query.
>
> However, at times, there may be situations where the long lasting
> local sessions may execute very few foreign queries and remaining all
> are local queries, in this scenario, the remote sessions opened by the
> local sessions/backends may not be useful as they remain idle and eat
> up the remote server connections capacity. This problem gets even
> worse(though this use case is a bit imaginary) if all of
> max_connections(default 100 and each backend caching 8 remote
> connections) local sessions open remote sessions and they are cached
> in the local backend.
>
> I propose to have a new session level GUC called
> "enable_connectioncache"(name can be changed if it doesn't correctly
> mean the purpose) with the default value being true which means that
> all the remote connections are cached. If set to false, the
> connections are not cached and so are remote sessions closed by the local backend/session at
> the end of each remote transaction.
>
> Attached the initial patch(based on commit
> 9550ea3027aa4f290c998afd8836a927df40b09d), test setup.

Few comments:

#backend_flush_after = 0 # measured in pages, 0 disables
-
+#enable_connectioncache = on
This guc could be placed in CONNECTIONS AND AUTHENTICATION section.

+
+ /* see if the cache was for postgres_fdw connections and
+ user chose to disable connection caching*/
+ if ((strcmp(hashp->tabname,"postgres_fdw connections") == 0) &&
+ !enable_connectioncache)

Should be changed to postgres style commenting like:
/*
* See if the cache was for postgres_fdw connections and
* user chose to disable connection caching.
*/

+ /* if true, fdw connections in a session are cached, else
+ discarded at the end of every remote transaction.
+ */
+ bool enableconncache;
Should be changed to postgres style commenting.

+/* parameter for enabling fdw connection hashing */
+bool enable_connectioncache = true;
+

Should this be connection caching?

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com