From 932d0c781ee60d8cc98cda35a23eba553df6e611 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Fri, 27 Nov 2020 07:25:13 +0530
Subject: [PATCH v2] postgres_fdw connection cache discard tests and
 documentation

---
 .../postgres_fdw/expected/postgres_fdw.out    | 169 +++++++++++++++++-
 contrib/postgres_fdw/sql/postgres_fdw.sql     |  96 ++++++++++
 doc/src/sgml/postgres-fdw.sgml                | 125 +++++++++++++
 3 files changed, 389 insertions(+), 1 deletion(-)

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 2d88d06358..de95a6f886 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,170 @@ ERROR:  08006
 COMMIT;
 -- Clean up
 DROP PROCEDURE terminate_backend_and_wait(text);
+-- ===================================================================
+-- disconnect connections that are cached/kept by the local session
+-- ===================================================================
+-- 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)
+
+-- 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)
+
+-- 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)
+
+-- 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
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 7581c5417b..2b1472c3c2 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -2697,3 +2697,99 @@ COMMIT;
 
 -- Clean up
 DROP PROCEDURE terminate_backend_and_wait(text);
+
+-- ===================================================================
+-- disconnect connections that are cached/kept by the local session
+-- ===================================================================
+
+-- 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;
+
+-- 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';
+
+-- 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();
+
+-- 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');
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index e6fd2143c1..60b9d2bf7b 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -477,6 +477,104 @@ 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_disconnect</function> ( <parameter>servername</parameter> <type>text</type> )
+   which takes foreign server name as input. When called in the local session,
+   it discards the 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 connections
+   previously made to the foreign servers and returns <literal>true</literal>.
+   If there are no previous connections kept by the local session, 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 +588,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

