Hi,
On 2020-11-06 18:56, Anastasia Lubennikova wrote:
Status update for a commitfest entry.
This thread was inactive for a while and from the latest messages, I
see that the patch needs some further work.
So I move it to "Waiting on Author".
The new status of this patch is: Waiting on Author
I had a look on the initial patch and discussed options [1] to proceed
with this issue. I agree with Bruce about idle_session_timeout, it would
be a nice to have in-core feature on its own. However, this should be a
cluster-wide option and it will start dropping all idle connection not
only foreign ones. So it may be not an option for some cases, when the
same foreign server is used for another load as well.
Regarding the initial issue I prefer point #3, i.e. foreign server
option. It has a couple of benefits IMO: 1) it may be set separately on
per foreign server basis, 2) it will live only in the postgres_fdw
contrib without any need to touch core. I would only supplement this
postgres_fdw foreign server option with a GUC, e.g.
postgres_fdw.keep_connections, so one could easily define such behavior
for all foreign servers at once or override server-level option by
setting this GUC on per session basis.
Attached is a small POC patch, which implements this contrib-level
postgres_fdw.keep_connections GUC. What do you think?
[1]
https://www.postgresql.org/message-id/CALj2ACUFNydy0uo0JL9A1isHQ9pFe1Fgqa_HVanfG6F8g21nSQ%40mail.gmail.com
Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index ab3226287d..64f0e96635 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -28,6 +28,8 @@
#include "utils/memutils.h"
#include "utils/syscache.h"
+#include "postgres_fdw.h"
+
/*
* Connection cache hash table entry
*
@@ -948,6 +950,7 @@ pgfdw_xact_callback(XactEvent event, void *arg)
*/
if (PQstatus(entry->conn) != CONNECTION_OK ||
PQtransactionStatus(entry->conn) != PQTRANS_IDLE ||
+ !keep_connections ||
entry->changing_xact_state)
{
elog(DEBUG3, "discarding connection %p", entry->conn);
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 9c5aaacc51..4cd5f71223 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -45,6 +45,8 @@
#include "utils/sampling.h"
#include "utils/selfuncs.h"
+#include "postgres_fdw.h"
+
PG_MODULE_MAGIC;
/* Default CPU cost to start up a foreign query. */
@@ -301,6 +303,8 @@ typedef struct
List *already_used; /* expressions already dealt with */
} ec_member_foreign_arg;
+bool keep_connections = true;
+
/*
* SQL functions
*/
@@ -505,6 +509,15 @@ static void merge_fdw_options(PgFdwRelationInfo *fpinfo,
const PgFdwRelationInfo *fpinfo_o,
const PgFdwRelationInfo *fpinfo_i);
+void
+_PG_init(void)
+{
+ DefineCustomBoolVariable("postgres_fdw.keep_connections",
+ "Enables postgres_fdw connection caching.",
+ "When off postgres_fdw will close connections at the end of transaction.",
+ &keep_connections, true, PGC_USERSET, 0, NULL,
+ NULL, NULL);
+}
/*
* Foreign-data wrapper handler function: return a struct with pointers
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index eef410db39..7f1bdb96d6 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -124,9 +124,12 @@ typedef struct PgFdwRelationInfo
int relation_index;
} PgFdwRelationInfo;
+extern bool keep_connections;
+
/* in postgres_fdw.c */
extern int set_transmission_modes(void);
extern void reset_transmission_modes(int nestlevel);
+extern void _PG_init(void);
/* in connection.c */
extern PGconn *GetConnection(UserMapping *user, bool will_prep_stmt);
