On Fri, Dec 4, 2020 at 1:46 PM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
>
> On Fri, Dec 4, 2020 at 11:49 AM Fujii Masao <masao.fu...@oss.nttdata.com>
> wrote:
> >
> > > Attaching the v2 patch set. Please review it further.
> >
> > Regarding the 0001 patch, we should add the function that returns
> > the information of cached connections like dblink_get_connections(),
> > together with 0001 patch? Otherwise it's not easy for users to
> > see how many cached connections are and determine whether to
> > disconnect them or not. Sorry if this was already discussed before.
> >
>
> Thanks for bringing this up. Exactly this is what I was thinking a few
> days back. Say the new function postgres_fdw_get_connections() which
> can return an array of server names whose connections exist in the
> cache. Without this function, the user may not know how many
> connections this backend has until he checks it manually on the remote
> server.
>
> Thoughts? If okay, I can code the function in the 0001 patch.
>
Added a new function postgres_fdw_get_connections() into 0001 patch,
which returns a list of server names for which there exists an
existing open and active connection.
Attaching v3 patch set, please review it further.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From 0ededddba3ee80a968d51df8283525ff962440da Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Fri, 4 Dec 2020 16:22:06 +0530
Subject: [PATCH v3] postgres_fdw connection cache discard tests and
documentation
---
.../postgres_fdw/expected/postgres_fdw.out | 202 +++++++++++++++++-
contrib/postgres_fdw/sql/postgres_fdw.sql | 118 ++++++++++
doc/src/sgml/postgres-fdw.sgml | 132 ++++++++++++
3 files changed, 451 insertions(+), 1 deletion(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 2d88d06358..692da1c606 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -8911,7 +8911,7 @@ DO $d$
END;
$d$;
ERROR: invalid option "password"
-HINT: Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size
+HINT: Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, requirepeer, ssl_min_protocol_version, ssl_max_protocol_version, gssencmode, krbsrvname, gsslib, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size, keep_connection
CONTEXT: SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')"
PL/pgSQL function inline_code_block line 3 at EXECUTE
-- If we add a password for our user mapping instead, we should get a different
@@ -9035,3 +9035,203 @@ ERROR: 08006
COMMIT;
-- Clean up
DROP PROCEDURE terminate_backend_and_wait(text);
+-- ===================================================================
+-- disconnect connections that are cached/kept by the local session
+-- ===================================================================
+-- postgres_fdw_get_connections returns an array with elements in a
+-- machine-dependent ordering, so we must resort to unnesting and sorting for a
+-- stable result.
+CREATE FUNCTION fdw_unnest(anyarray) RETURNS SETOF anyelement
+LANGUAGE SQL STRICT IMMUTABLE AS $$
+SELECT $1[i] FROM generate_series(array_lower($1,1), array_upper($1,1)) AS i
+$$;
+-- Change application names of remote connections to special ones so that we
+-- can easily check for their existence.
+ALTER SERVER loopback OPTIONS (SET application_name 'fdw_disconnect_cached_conn_1');
+ALTER SERVER loopback2 OPTIONS (application_name 'fdw_disconnect_cached_conn_2');
+-- By default, the connections associated with foreign server are cached i.e.
+-- keep_connection option is on. Set it to off.
+ALTER SERVER loopback OPTIONS (keep_connection 'off');
+-- loopback server connection is closed by the local session at the end of xact
+-- as the keep_connection was set to off.
+SELECT 1 FROM ft1 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+-- Connection should not exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_1';
+ application_name
+------------------
+(0 rows)
+
+-- loopback2 server connection is cached by the local session as the
+-- keep_connection is on.
+SELECT 1 FROM ft6 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+-- Connection should exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_2';
+ application_name
+------------------------------
+ fdw_disconnect_cached_conn_2
+(1 row)
+
+-- By default, keep_connections GUC is on i.e. local session caches all the
+-- foreign server connections.
+SHOW postgres_fdw.keep_connections;
+ postgres_fdw.keep_connections
+-------------------------------
+ on
+(1 row)
+
+-- Set it off i.e. the cached connections which are used after this setting are
+-- disconnected at the end of respective xacts.
+SET postgres_fdw.keep_connections TO off;
+-- loopback2 server connection is closed at the end of xact.
+SELECT 1 FROM ft6 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+-- Connection should not exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_2';
+ application_name
+------------------
+(0 rows)
+
+-- Connections from hereafter are cached.
+SET postgres_fdw.keep_connections TO on;
+-- loopback server connection is cached.
+ALTER SERVER loopback OPTIONS (SET keep_connection 'on');
+-- Connections are cached.
+SELECT 1 FROM ft1 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+SELECT 1 FROM ft6 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+-- List all the existing cached connections. Should return loopback and
+-- loopback2.
+SELECT * FROM fdw_unnest(postgres_fdw_get_connections()) ORDER BY 1;
+ fdw_unnest
+------------
+ loopback
+ loopback2
+(2 rows)
+
+-- loopback server connection is disconnected and true is returned. loopback2
+-- server connection still exists.
+SELECT postgres_fdw_disconnect('loopback');
+ postgres_fdw_disconnect
+-------------------------
+ t
+(1 row)
+
+-- Connection should not exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_1';
+ application_name
+------------------
+(0 rows)
+
+-- Connection should exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_2';
+ application_name
+------------------------------
+ fdw_disconnect_cached_conn_2
+(1 row)
+
+-- List all the existing cached connections. Should return loopback2.
+SELECT * FROM fdw_unnest(postgres_fdw_get_connections()) ORDER BY 1;
+ fdw_unnest
+------------
+ loopback2
+(1 row)
+
+-- Cache exists, but the loopback server connection is not present in it,
+-- so false is returned.
+SELECT postgres_fdw_disconnect('loopback');
+ postgres_fdw_disconnect
+-------------------------
+ f
+(1 row)
+
+-- Make loopback server connection again. Now, both loopback and loopback2
+-- server connections exist in the local session.
+SELECT 1 FROM ft1 LIMIT 1;
+ ?column?
+----------
+ 1
+(1 row)
+
+-- Connection should exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_1';
+ application_name
+------------------------------
+ fdw_disconnect_cached_conn_1
+(1 row)
+
+-- Discard all the connections. True is returned.
+SELECT postgres_fdw_disconnect();
+ postgres_fdw_disconnect
+-------------------------
+ t
+(1 row)
+
+-- Both the connections should not exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_1';
+ application_name
+------------------
+(0 rows)
+
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_2';
+ application_name
+------------------
+(0 rows)
+
+-- Cache does not exist. Try to discard, false is returned.
+SELECT postgres_fdw_disconnect();
+ postgres_fdw_disconnect
+-------------------------
+ f
+(1 row)
+
+-- List all the existing cached connections. Should return NULL, as there are
+-- none exist.
+SELECT * FROM postgres_fdw_get_connections();
+ postgres_fdw_get_connections
+------------------------------
+
+(1 row)
+
+-- Cache does not exist, but the loopback server exists, so false is returned.
+SELECT postgres_fdw_disconnect('loopback');
+ postgres_fdw_disconnect
+-------------------------
+ f
+(1 row)
+
+-- The server name provided doesn't exist, an error is expected.
+SELECT postgres_fdw_disconnect('unknownserver');
+ERROR: foreign server "unknownserver" does not exist
+-- Clean up
+DROP FUNCTION fdw_unnest;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 7581c5417b..137edae4bb 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -2697,3 +2697,121 @@ COMMIT;
-- Clean up
DROP PROCEDURE terminate_backend_and_wait(text);
+
+-- ===================================================================
+-- disconnect connections that are cached/kept by the local session
+-- ===================================================================
+
+-- postgres_fdw_get_connections returns an array with elements in a
+-- machine-dependent ordering, so we must resort to unnesting and sorting for a
+-- stable result.
+CREATE FUNCTION fdw_unnest(anyarray) RETURNS SETOF anyelement
+LANGUAGE SQL STRICT IMMUTABLE AS $$
+SELECT $1[i] FROM generate_series(array_lower($1,1), array_upper($1,1)) AS i
+$$;
+
+-- Change application names of remote connections to special ones so that we
+-- can easily check for their existence.
+ALTER SERVER loopback OPTIONS (SET application_name 'fdw_disconnect_cached_conn_1');
+ALTER SERVER loopback2 OPTIONS (application_name 'fdw_disconnect_cached_conn_2');
+
+-- By default, the connections associated with foreign server are cached i.e.
+-- keep_connection option is on. Set it to off.
+ALTER SERVER loopback OPTIONS (keep_connection 'off');
+
+-- loopback server connection is closed by the local session at the end of xact
+-- as the keep_connection was set to off.
+SELECT 1 FROM ft1 LIMIT 1;
+
+-- Connection should not exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_1';
+
+-- loopback2 server connection is cached by the local session as the
+-- keep_connection is on.
+SELECT 1 FROM ft6 LIMIT 1;
+
+-- Connection should exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_2';
+
+-- By default, keep_connections GUC is on i.e. local session caches all the
+-- foreign server connections.
+SHOW postgres_fdw.keep_connections;
+
+-- Set it off i.e. the cached connections which are used after this setting are
+-- disconnected at the end of respective xacts.
+SET postgres_fdw.keep_connections TO off;
+
+-- loopback2 server connection is closed at the end of xact.
+SELECT 1 FROM ft6 LIMIT 1;
+
+-- Connection should not exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_2';
+
+-- Connections from hereafter are cached.
+SET postgres_fdw.keep_connections TO on;
+
+-- loopback server connection is cached.
+ALTER SERVER loopback OPTIONS (SET keep_connection 'on');
+
+-- Connections are cached.
+SELECT 1 FROM ft1 LIMIT 1;
+SELECT 1 FROM ft6 LIMIT 1;
+
+-- List all the existing cached connections. Should return loopback and
+-- loopback2.
+SELECT * FROM fdw_unnest(postgres_fdw_get_connections()) ORDER BY 1;
+
+-- loopback server connection is disconnected and true is returned. loopback2
+-- server connection still exists.
+SELECT postgres_fdw_disconnect('loopback');
+
+-- Connection should not exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_1';
+
+-- Connection should exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_2';
+
+-- List all the existing cached connections. Should return loopback2.
+SELECT * FROM fdw_unnest(postgres_fdw_get_connections()) ORDER BY 1;
+
+-- Cache exists, but the loopback server connection is not present in it,
+-- so false is returned.
+SELECT postgres_fdw_disconnect('loopback');
+
+-- Make loopback server connection again. Now, both loopback and loopback2
+-- server connections exist in the local session.
+SELECT 1 FROM ft1 LIMIT 1;
+
+-- Connection should exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_1';
+
+-- Discard all the connections. True is returned.
+SELECT postgres_fdw_disconnect();
+
+-- Both the connections should not exist.
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_1';
+SELECT application_name FROM pg_stat_activity
+ WHERE application_name = 'fdw_disconnect_cached_conn_2';
+
+-- Cache does not exist. Try to discard, false is returned.
+SELECT postgres_fdw_disconnect();
+
+-- List all the existing cached connections. Should return NULL, as there are
+-- none exist.
+SELECT * FROM postgres_fdw_get_connections();
+
+-- Cache does not exist, but the loopback server exists, so false is returned.
+SELECT postgres_fdw_disconnect('loopback');
+
+-- The server name provided doesn't exist, an error is expected.
+SELECT postgres_fdw_disconnect('unknownserver');
+
+-- Clean up
+DROP FUNCTION fdw_unnest;
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index e6fd2143c1..f3f4988454 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -477,6 +477,111 @@ OPTIONS (ADD password_required 'false');
</para>
</sect3>
+
+ <sect3>
+ <title>Connection Management Options</title>
+
+ <para>
+ By default the foreign server connections made with
+ <filename>postgres_fdw</filename> are kept in local session for re-use.
+ This may be overridden using the following
+ <xref linkend="sql-createserver"/> option:
+ </para>
+
+ <variablelist>
+
+ <varlistentry>
+ <term><literal>keep_connection</literal></term>
+ <listitem>
+ <para>
+ This option controls whether <filename>postgres_fdw</filename> keeps the
+ server connection that is made with a specific foreign server. It can be
+ specified for a foreign server. Default is <literal>on</literal>. When
+ set to <literal>off</literal>, the associated foreign server connection
+ is discarded at the end of the transaction.
+ </para>
+
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+ </sect3>
+
+ </sect2>
+
+<sect2>
+ <title>Functions</title>
+
+ <para>
+ <function>postgres_fdw_get_connections</function> ( ) which takes no input.
+ When called in the local session, it returns an array of the foreign server
+ names of all the open connections that are previously made to the foreign
+ servers. If there are no open connections, then <literal>NULL</literal> is
+ returned.
+ </para>
+
+ <para>
+ <function>postgres_fdw_disconnect</function> ( <parameter>servername</parameter> <type>text</type> )
+ which takes foreign server name as input. When called in the local session,
+ it discards the open connection previously made to the foreign server and
+ returns <literal>true</literal>. If there is no associated connection exists
+ for the given foreign server, then <literal>false</literal> is returned. If
+ no foreign server exists with the given name, then an error is emitted.
+ </para>
+
+ <para>
+ <function>postgres_fdw_disconnect</function> ( ) which takes no input.
+ When called in the local session, it discards all the open connections that
+ are previously made to the foreign servers and returns <literal>true</literal>.
+ If there are no open connections, then <literal>false</literal> is returned.
+ </para>
+
+</sect2>
+
+ <sect2>
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+
+ <term>
+ <varname>postgres_fdw.keep_connections</varname> (<type>boolean</type>)
+ <indexterm>
+ <primary><varname>postgres_fdw.keep_connections</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+
+ <listitem>
+
+ <para>
+ Allows <filename>postgres_fdw</filename> to keep or discard the
+ connection made to the foreign server by the local session. Default is
+ <literal>on</literal>. When set to <literal>off</literal> the local
+ session doesn't keep the connections made to the foreign servers. Each
+ connection is discarded at the end of transaction in which it is used.
+ </para>
+
+ <para>
+ <varname>postgres_fdw.keep_connections</varname> configuration parameter
+ overrides the server level <literal>keep_connection</literal> option.
+ Which means that when the configuration parameter is set to
+ <literal>on</literal>, irrespective of the server option
+ <literal>keep_connection</literal> setting, the connections are discarded.
+ </para>
+
+ <para>
+ Note that setting <varname>postgres_fdw.keep_connections</varname> to
+ <literal>off</literal> does not discard any previously made and still open
+ connections immediately. They will be closed only at the end of future
+ transactions, which operated on them. To close all connections
+ immediately use <function>postgres_fdw_disconnect</function> function.
+ </para>
+
+ </listitem>
+
+ </varlistentry>
+ </variablelist>
+
</sect2>
<sect2>
@@ -490,6 +595,33 @@ OPTIONS (ADD password_required 'false');
multiple user identities (user mappings) are used to access the foreign
server, a connection is established for each user mapping.
</para>
+
+ <para>
+ Since the <filename>postgres_fdw</filename> keeps the connections to remote
+ servers in the local session, the corresponding sessions that are opened on
+ the remote servers are kept idle until they are re-used by the local session.
+ This may waste resources if those connections are not frequently used by the
+ local session. To address this, the <filename>postgres_fdw</filename>
+ provides following ways to remove the connections to the remote servers and
+ so the remote sessions:
+
+ A server level option, <literal>keep_connection</literal> that is used with
+ <xref linkend="sql-createserver"/>. Default being <literal>on</literal>,
+ when set to <literal>off</literal> the local session doesn't keep remote
+ connection associated with the foreign server. The connection is
+ discarded at the end of the transaction.
+
+ A configuration parameter, <varname>postgres_fdw.keep_connections</varname>,
+ default being <literal>on</literal>, when set to <literal>off</literal>, the
+ local session doesn't keep remote connections that are made to the foreign
+ servers. Each connection is discarded at the end of transaction in which it
+ is used.
+
+ <function>postgres_fdw_disconnect()</function> to discard all the
+ connections or <function>postgres_fdw_disconnect(text)</function>
+ to discard the connection associated with the given foreign server.
+
+ </para>
</sect2>
<sect2>
--
2.25.1
From f890ebb605c8a2aa27dcf87e1b36cbb48594ce59 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Fri, 4 Dec 2020 16:24:57 +0530
Subject: [PATCH v3] postgres_fdw function to discard cached connections
This patch introduces new function postgres_fdw_disconnect() when
called with a foreign server name discards the associated
connections with the server name. When called without any argument,
discards all the existing cached connections.
This patch also adds another function postgres_fdw_get_connections()
to get the list of all cached connections by corresponding foreign
server names.
---
contrib/postgres_fdw/connection.c | 164 +++++++++++++++++++++
contrib/postgres_fdw/postgres_fdw--1.0.sql | 15 ++
2 files changed, 179 insertions(+)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index ab3226287d..65695ebb67 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -14,6 +14,7 @@
#include "access/htup_details.h"
#include "access/xact.h"
+#include "catalog/pg_foreign_server.h"
#include "catalog/pg_user_mapping.h"
#include "commands/defrem.h"
#include "mb/pg_wchar.h"
@@ -22,6 +23,7 @@
#include "postgres_fdw.h"
#include "storage/fd.h"
#include "storage/latch.h"
+#include "utils/builtins.h"
#include "utils/datetime.h"
#include "utils/hsearch.h"
#include "utils/inval.h"
@@ -73,6 +75,12 @@ static unsigned int prep_stmt_number = 0;
/* tracks whether any work is needed in callback functions */
static bool xact_got_connection = false;
+/*
+ * SQL functions
+ */
+PG_FUNCTION_INFO_V1(postgres_fdw_get_connections);
+PG_FUNCTION_INFO_V1(postgres_fdw_disconnect);
+
/* prototypes of private functions */
static void make_new_connection(ConnCacheEntry *entry, UserMapping *user);
static PGconn *connect_pg_server(ForeignServer *server, UserMapping *user);
@@ -94,6 +102,7 @@ static bool pgfdw_exec_cleanup_query(PGconn *conn, const char *query,
static bool pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime,
PGresult **result);
static bool UserMappingPasswordRequired(UserMapping *user);
+static bool disconnect_cached_connections(uint32 hashvalue, bool all);
/*
* Get a PGconn which can be used to execute queries on the remote PostgreSQL
@@ -1325,3 +1334,158 @@ exit: ;
*result = last_res;
return timed_out;
}
+
+/*
+ * List all cached connections by corresponding foreign server names.
+ *
+ * Takes no paramaters as input. Returns an array of all foreign server names
+ * with which connections exist. Returns NULL, in case there are no cached
+ * connections at all.
+ */
+Datum
+postgres_fdw_get_connections(PG_FUNCTION_ARGS)
+{
+ ArrayBuildState *astate = NULL;
+
+ if (ConnectionHash)
+ {
+ HASH_SEQ_STATUS scan;
+ ConnCacheEntry *entry;
+
+ hash_seq_init(&scan, ConnectionHash);
+ while ((entry = (ConnCacheEntry *) hash_seq_search(&scan)))
+ {
+ Form_pg_user_mapping umap;
+ HeapTuple umaptup;
+ Form_pg_foreign_server fsrv;
+ HeapTuple fsrvtup;
+
+ /* We only look for active and open remote connections. */
+ if (entry->invalidated || !entry->conn)
+ continue;
+
+ umaptup = SearchSysCache1(USERMAPPINGOID,
+ ObjectIdGetDatum(entry->key));
+
+ if (!HeapTupleIsValid(umaptup))
+ elog(ERROR, "cache lookup failed for user mapping with OID %u",
+ entry->key);
+
+ umap = (Form_pg_user_mapping) GETSTRUCT(umaptup);
+
+ fsrvtup = SearchSysCache1(FOREIGNSERVEROID,
+ ObjectIdGetDatum(umap->umserver));
+
+ if (!HeapTupleIsValid(fsrvtup))
+ elog(ERROR, "cache lookup failed for foreign server with OID %u",
+ umap->umserver);
+
+ fsrv = (Form_pg_foreign_server) GETSTRUCT(fsrvtup);
+
+ /* stash away current value */
+ astate = accumArrayResult(astate,
+ CStringGetTextDatum(NameStr(fsrv->srvname)),
+ false, TEXTOID, CurrentMemoryContext);
+
+ ReleaseSysCache(umaptup);
+ ReleaseSysCache(fsrvtup);
+ }
+ }
+
+ if (astate)
+ PG_RETURN_ARRAYTYPE_P(makeArrayResult(astate,
+ CurrentMemoryContext));
+ else
+ PG_RETURN_NULL();
+}
+
+/*
+ * Disconnects the cached connections.
+ *
+ * If server name is provided as input, it disconnects the associated cached
+ * connections. Otherwise all the cached connections are disconnected and the
+ * cache is destroyed.
+ *
+ * Returns false if the cache is empty or if the cache is non empty and server
+ * name is provided and it exists but it has no associated entry in the cache.
+ * An error is emitted when the given foreign server does not exist.
+ * In all other cases true is returned.
+ */
+Datum
+postgres_fdw_disconnect(PG_FUNCTION_ARGS)
+{
+ bool result = false;
+
+ if (PG_NARGS() == 1)
+ {
+ char *servername = NULL;
+ ForeignServer *server = NULL;
+
+ servername = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ server = GetForeignServerByName(servername, true);
+
+ if (server)
+ {
+ uint32 hashvalue;
+
+ hashvalue =
+ GetSysCacheHashValue1(FOREIGNSERVEROID,
+ ObjectIdGetDatum(server->serverid));
+
+ if (ConnectionHash)
+ result = disconnect_cached_connections(hashvalue, false);
+ }
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_CONNECTION_DOES_NOT_EXIST),
+ errmsg("foreign server \"%s\" does not exist", servername)));
+ }
+ else
+ {
+ if (ConnectionHash)
+ result = disconnect_cached_connections(0, true);
+ }
+
+ PG_RETURN_BOOL(result);
+}
+
+/*
+ * Workhorse to disconnect the cached connections.
+ *
+ * If all is true, all the cached connections are disconnected and cache is
+ * destroyed. Otherwise, the entries with the given hashvalue are disconnected.
+ *
+ * Returns true in the following cases: either the cache is destroyed now or
+ * at least a single entry with the given hashvalue exists. In all other cases
+ * false is returned.
+ */
+bool disconnect_cached_connections(uint32 hashvalue, bool all)
+{
+ bool result = false;
+ HASH_SEQ_STATUS scan;
+ ConnCacheEntry *entry;
+
+ /*
+ * We do not come here if the ConnectionHash is NULL. We handle it in the
+ * caller.
+ */
+ hash_seq_init(&scan, ConnectionHash);
+ while ((entry = (ConnCacheEntry *) hash_seq_search(&scan)))
+ {
+ if ((all || entry->server_hashvalue == hashvalue) &&
+ entry->conn)
+ {
+ disconnect_pg_server(entry);
+ result = true;
+ }
+ }
+
+ if (all)
+ {
+ hash_destroy(ConnectionHash);
+ ConnectionHash = NULL;
+ result = true;
+ }
+
+ return result;
+}
diff --git a/contrib/postgres_fdw/postgres_fdw--1.0.sql b/contrib/postgres_fdw/postgres_fdw--1.0.sql
index a0f0fc1bf4..edbb5911d2 100644
--- a/contrib/postgres_fdw/postgres_fdw--1.0.sql
+++ b/contrib/postgres_fdw/postgres_fdw--1.0.sql
@@ -16,3 +16,18 @@ LANGUAGE C STRICT;
CREATE FOREIGN DATA WRAPPER postgres_fdw
HANDLER postgres_fdw_handler
VALIDATOR postgres_fdw_validator;
+
+CREATE FUNCTION postgres_fdw_get_connections ()
+RETURNS text[]
+AS 'MODULE_PATHNAME','postgres_fdw_get_connections'
+LANGUAGE C STRICT PARALLEL RESTRICTED;
+
+CREATE FUNCTION postgres_fdw_disconnect ()
+RETURNS bool
+AS 'MODULE_PATHNAME','postgres_fdw_disconnect'
+LANGUAGE C STRICT PARALLEL RESTRICTED;
+
+CREATE FUNCTION postgres_fdw_disconnect (text)
+RETURNS bool
+AS 'MODULE_PATHNAME','postgres_fdw_disconnect'
+LANGUAGE C STRICT PARALLEL RESTRICTED;
--
2.25.1
From 1b3571df2d1b1e8675aa20d59e2fe9aff11bbcf0 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Fri, 27 Nov 2020 06:02:54 +0530
Subject: [PATCH v2] postgres_fdw add keep_connections GUC to not cache
connections
This patch adds a new GUC postgres_fdw.keep_connections, default
being on, when set to off no remote connections are cached by the
local session.
---
contrib/postgres_fdw/connection.c | 17 +++++++++++++----
contrib/postgres_fdw/postgres_fdw.c | 16 ++++++++++++++++
contrib/postgres_fdw/postgres_fdw.h | 3 +++
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 975d7c648e..85217188b5 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -807,6 +807,7 @@ pgfdw_xact_callback(XactEvent event, void *arg)
while ((entry = (ConnCacheEntry *) hash_seq_search(&scan)))
{
PGresult *res;
+ bool used_in_current_xact = false;
/* Ignore cache entry if no open connection right now */
if (entry->conn == NULL)
@@ -817,6 +818,8 @@ pgfdw_xact_callback(XactEvent event, void *arg)
{
bool abort_cleanup_failure = false;
+ used_in_current_xact = true;
+
elog(DEBUG3, "closing remote transaction on connection %p",
entry->conn);
@@ -950,15 +953,21 @@ pgfdw_xact_callback(XactEvent event, void *arg)
entry->xact_depth = 0;
/*
- * If the connection isn't in a good idle state, discard it to
- * recover. Next GetConnection will open a new connection.
+ * If the connection isn't in a good idle state, discard it to recover.
+ * Next GetConnection will open a new connection when required.
+ *
+ * If the keep_connections GUC is false and this connection is used in
+ * current xact, then also discard it.
*/
- if (PQstatus(entry->conn) != CONNECTION_OK ||
+ if ((PQstatus(entry->conn) != CONNECTION_OK ||
PQtransactionStatus(entry->conn) != PQTRANS_IDLE ||
- entry->changing_xact_state)
+ entry->changing_xact_state) ||
+ (used_in_current_xact &&
+ !keep_connections))
{
elog(DEBUG3, "discarding connection %p", entry->conn);
disconnect_pg_server(entry);
+ used_in_current_xact = false;
}
}
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index b6c72e1d1e..160f17972b 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -302,6 +302,8 @@ typedef struct
List *already_used; /* expressions already dealt with */
} ec_member_foreign_arg;
+bool keep_connections = true;
+
/*
* SQL functions
*/
@@ -506,6 +508,20 @@ 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);
--
2.25.1
From ec2b2d7d78a5ce9d75eb253ae3db45cc4a9d7a13 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Fri, 27 Nov 2020 06:18:42 +0530
Subject: [PATCH v2] postgres_fdw server level option, keep_connection to not
cache connection
This patch adds a new server level option, keep_connection,
default being on, when set to off, the local session doesn't cache
the connections associated with the foreign server.
---
contrib/postgres_fdw/connection.c | 22 +++++++++++++++++++---
contrib/postgres_fdw/option.c | 9 ++++++++-
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 85217188b5..c13e1a3cc8 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -60,6 +60,8 @@ typedef struct ConnCacheEntry
bool invalidated; /* true if reconnect is pending */
uint32 server_hashvalue; /* hash value of foreign server OID */
uint32 mapping_hashvalue; /* hash value of user mapping OID */
+ /* Keep or discard this connection at the end of xact */
+ bool keep_connection;
} ConnCacheEntry;
/*
@@ -120,6 +122,8 @@ GetConnection(UserMapping *user, bool will_prep_stmt)
ConnCacheEntry *entry;
ConnCacheKey key;
MemoryContext ccxt = CurrentMemoryContext;
+ ListCell *lc;
+ ForeignServer *server;
/* First time through, initialize connection cache hashtable */
if (ConnectionHash == NULL)
@@ -260,6 +264,15 @@ GetConnection(UserMapping *user, bool will_prep_stmt)
begin_remote_xact(entry);
}
+ server = GetForeignServer(user->serverid);
+ foreach(lc, server->options)
+ {
+ DefElem *def = (DefElem *) lfirst(lc);
+
+ if (strcmp(def->defname, "keep_connection") == 0)
+ entry->keep_connection = defGetBoolean(def);
+ }
+
/* Remember if caller will prepare statements */
entry->have_prep_stmt |= will_prep_stmt;
@@ -283,6 +296,7 @@ make_new_connection(ConnCacheEntry *entry, UserMapping *user)
entry->have_error = false;
entry->changing_xact_state = false;
entry->invalidated = false;
+ entry->keep_connection = true;
entry->server_hashvalue =
GetSysCacheHashValue1(FOREIGNSERVEROID,
ObjectIdGetDatum(server->serverid));
@@ -956,14 +970,16 @@ pgfdw_xact_callback(XactEvent event, void *arg)
* If the connection isn't in a good idle state, discard it to recover.
* Next GetConnection will open a new connection when required.
*
- * If the keep_connections GUC is false and this connection is used in
- * current xact, then also discard it.
+ * Also discard the connection if it is used in current xact and if the
+ * GUC is set to off or if the GUC is on but the server level option is
+ * set to off. Note that keep_connections GUC overrides the server
+ * level keep_connection option.
*/
if ((PQstatus(entry->conn) != CONNECTION_OK ||
PQtransactionStatus(entry->conn) != PQTRANS_IDLE ||
entry->changing_xact_state) ||
(used_in_current_xact &&
- !keep_connections))
+ (!keep_connections || !entry->keep_connection)))
{
elog(DEBUG3, "discarding connection %p", entry->conn);
disconnect_pg_server(entry);
diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c
index 1a03e02263..0fe2eff878 100644
--- a/contrib/postgres_fdw/option.c
+++ b/contrib/postgres_fdw/option.c
@@ -107,7 +107,8 @@ postgres_fdw_validator(PG_FUNCTION_ARGS)
* Validate option value, when we can do so without any context.
*/
if (strcmp(def->defname, "use_remote_estimate") == 0 ||
- strcmp(def->defname, "updatable") == 0)
+ strcmp(def->defname, "updatable") == 0 ||
+ strcmp(def->defname, "keep_connection") == 0)
{
/* these accept only boolean values */
(void) defGetBoolean(def);
@@ -213,6 +214,12 @@ InitPgFdwOptions(void)
{"sslcert", UserMappingRelationId, true},
{"sslkey", UserMappingRelationId, true},
+ /*
+ * If true, cache the connection associated with this server, otherwise
+ * remove it at the end of the xact. Default is true.
+ */
+ {"keep_connection", ForeignServerRelationId, false},
+
{NULL, InvalidOid, false}
};
--
2.25.1
