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. Another approach to solve this problem could be that (based on Robert's idea[1]) automatic clean up of cache entries, but letting users decide on caching also seems to be good. Please note that documentation is still pending. Thoughts? Test Case: without patch: 1. Run the query on foreign table 2. Look for the backend/session opened on the remote postgres server, it exists till the local session remains active. with patch: 1. SET enable_connectioncache TO false; 2. Run the query on the foreign table 3. Look for the backend/session opened on the remote postgres server, it should not exist. [1] - https://www.postgresql.org/message-id/CA%2BTgmob_ksTOgmbXhno%2Bk5XXPOK%2B-JYYLoU3MpXuutP4bH7gzA%40mail.gmail.com With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
testcase
Description: Binary data
From d4594067b29ab1414e9741a7e6abd91daf078201 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Mon, 22 Jun 2020 10:07:53 +0530
Subject: [PATCH v1] enable_connectioncache GUC for postgres_fdw connection
caching
postgres_fdw connection caching - cause remote sessions linger
till the local session exit.
---
contrib/postgres_fdw/connection.c | 3 ++-
src/backend/utils/hash/dynahash.c | 11 +++++++++++
src/backend/utils/misc/guc.c | 15 +++++++++++++++
src/backend/utils/misc/postgresql.conf.sample | 2 +-
src/include/utils/hsearch.h | 4 ++++
5 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 52d1fe3563..de994f678b 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -869,7 +869,8 @@ pgfdw_xact_callback(XactEvent event, void *arg)
*/
if (PQstatus(entry->conn) != CONNECTION_OK ||
PQtransactionStatus(entry->conn) != PQTRANS_IDLE ||
- entry->changing_xact_state)
+ entry->changing_xact_state ||
+ scan.enableconncache == false)
{
elog(DEBUG3, "discarding connection %p", entry->conn);
disconnect_pg_server(entry);
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 2688e27726..a5448f4af4 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -276,6 +276,9 @@ static bool has_seq_scans(HTAB *hashp);
*/
static MemoryContext CurrentDynaHashCxt = NULL;
+/* parameter for enabling fdw connection hashing */
+bool enable_connectioncache = true;
+
static void *
DynaHashAlloc(Size size)
{
@@ -1383,6 +1386,14 @@ hash_seq_init(HASH_SEQ_STATUS *status, HTAB *hashp)
status->hashp = hashp;
status->curBucket = 0;
status->curEntry = NULL;
+ status->enableconncache = true;
+
+ /* 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)
+ status->enableconncache = false;
+
if (!hashp->frozen)
register_seq_scan(hashp);
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 75fc6f11d6..f7a57415fc 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -127,6 +127,7 @@ extern char *temp_tablespaces;
extern bool ignore_checksum_failure;
extern bool ignore_invalid_pages;
extern bool synchronize_seqscans;
+extern bool enable_connectioncache;
#ifdef TRACE_SYNCSCAN
extern bool trace_syncscan;
@@ -2041,6 +2042,20 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
+ {
+ {"enable_connectioncache", PGC_USERSET, RESOURCES_MEM,
+ gettext_noop("Enables postgres_fdw connection caching."),
+ gettext_noop("The postgres_fdw connections are cached and "
+ "reused within the same session by default. "
+ "Setting this parameter to false, discards the "
+ "remote connection at the end of the transaction."),
+ GUC_EXPLAIN
+ },
+ &enable_connectioncache,
+ true,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 3a25287a39..1a55177df2 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -184,7 +184,7 @@
#old_snapshot_threshold = -1 # 1min-60d; -1 disables; 0 is immediate
# (change requires restart)
#backend_flush_after = 0 # measured in pages, 0 disables
-
+#enable_connectioncache = on
#------------------------------------------------------------------------------
# WRITE-AHEAD LOG
diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h
index f1deb9beab..77546a7c16 100644
--- a/src/include/utils/hsearch.h
+++ b/src/include/utils/hsearch.h
@@ -114,6 +114,10 @@ typedef struct
HTAB *hashp;
uint32 curBucket; /* index of current bucket */
HASHELEMENT *curEntry; /* current entry in bucket */
+ /* if true, fdw connections in a session are cached, else
+ discarded at the end of every remote transaction.
+ */
+ bool enableconncache;
} HASH_SEQ_STATUS;
/*
--
2.25.1
