Hi,
While working on [1], I got to know that there is a new GUC
debug_invalidate_system_caches_always that has been introduced in v14.
It can be used to switch off cache invalidation in
CLOBBER_CACHE_ALWAYS builds which makes cache sensitive tests stable.
Using this GUC, it is quite possible to make cached connection
management function tests more meaningful by returning original
values(true/false, all the output columns) instead of SELECT 1. Note
that the commit f77717b29 stabilized the tests for those functions -
postgres_fdw_disconnect, postgres_fdw_disconnect_all and
postgres_fdw_get_connections by masking actual return value of the
functions.
Attaching a patch to use the new GUC to make the functions return actual output.
Thoughts?
[1] -
https://www.postgresql.org/message-id/CALj2ACVGSQsq68y-LmyXKZzbNVgSgsiVKSzsrVXzVgnsZTN26Q%40mail.gmail.com
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From a5ff26edbf54fca4d42833ef4aedf79bec9445bd Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Sat, 8 May 2021 19:51:58 +0530
Subject: [PATCH v1] postgres_fdw-make cached connection functions tests
meaningful
The new GUC debug_invalidate_system_caches_always which has been
introduced in v14 can be used to switch off cache invalidation in
CLOBBER_CACHE_ALWAYS builds which can make cache sensitive tests
stable. Using this GUC, it is quite possible to make cached
connection management function tests more meaningful by returning
original values(true/false, all the output columns) instead of
SELECT 1. Note that the commit f77717b29 stabilized the tests for
those functions - postgres_fdw_disconnect, postgres_fdw_disconnect_all
and postgres_fdw_get_connections by masking actual return value of
the functions.
---
.../postgres_fdw/expected/postgres_fdw.out | 135 ++++++++++--------
contrib/postgres_fdw/sql/postgres_fdw.sql | 58 ++++----
2 files changed, 100 insertions(+), 93 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 6f533c745d..67d993edb8 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -9198,17 +9198,26 @@ PREPARE TRANSACTION 'fdw_tpc';
ERROR: cannot PREPARE a transaction that has operated on postgres_fdw foreign tables
ROLLBACK;
WARNING: there is no transaction in progress
+-- Let's ensure to close all the existing cached connections so that they don't
+-- make the following tests unstable.
+SELECT 1 FROM postgres_fdw_disconnect_all();
+ ?column?
+----------
+ 1
+(1 row)
+
+-- If debug_invalidate_system_caches_always is active, it results in dropping
+-- remote connections after every transaction, making it impossible to test
+-- connection retry use case meaningfully. And also, the new functions
+-- introduced for cached connections management will be unstable with the flag
+-- on. So turn it off for these tests.
+SET debug_invalidate_system_caches_always = 0;
-- ===================================================================
-- reestablish new connection
-- ===================================================================
-- Change application_name of remote connection to special one
-- so that we can easily terminate the connection later.
ALTER SERVER loopback OPTIONS (application_name 'fdw_retry_check');
--- If debug_invalidate_system_caches_always is active, it results in
--- dropping remote connections after every transaction, making it
--- impossible to test termination meaningfully. So turn that off
--- for this test.
-SET debug_invalidate_system_caches_always = 0;
-- Make sure we have a remote connection.
SELECT 1 FROM ft1 LIMIT 1;
?column?
@@ -9250,21 +9259,13 @@ SELECT 1 FROM ft1 LIMIT 1; -- should fail
ERROR: 08006
\set VERBOSITY default
COMMIT;
-RESET debug_invalidate_system_caches_always;
-- =============================================================================
-- test connection invalidation cases and postgres_fdw_get_connections function
-- =============================================================================
--- Let's ensure to close all the existing cached connections.
-SELECT 1 FROM postgres_fdw_disconnect_all();
- ?column?
-----------
- 1
-(1 row)
-
-- No cached connections, so no records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name
--------------
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | valid
+-------------+-------
(0 rows)
-- This test case is for closing the connection in pgfdw_xact_callback
@@ -9284,11 +9285,11 @@ SELECT 1 FROM ft7 LIMIT 1;
-- List all the existing cached connections. loopback and loopback3 should be
-- output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name
--------------
- loopback
- loopback3
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | valid
+-------------+-------
+ loopback | t
+ loopback3 | t
(2 rows)
-- Connections are not closed at the end of the alter and drop statements.
@@ -9313,9 +9314,9 @@ SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
COMMIT;
-- All cached connections were closed while committing above xact, so no
-- records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name
--------------
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | valid
+-------------+-------
(0 rows)
-- =======================================================================
@@ -9338,11 +9339,11 @@ SELECT 1 FROM ft6 LIMIT 1;
-- List all the existing cached connections. loopback and loopback2 should be
-- output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name
--------------
- loopback
- loopback2
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | valid
+-------------+-------
+ loopback | t
+ loopback2 | t
(2 rows)
-- Issue a warning and return false as loopback connection is still in use and
@@ -9356,11 +9357,11 @@ WARNING: cannot close connection for server "loopback" because it is still in u
-- List all the existing cached connections. loopback and loopback2 should be
-- output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name
--------------
- loopback
- loopback2
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | valid
+-------------+-------
+ loopback | t
+ loopback2 | t
(2 rows)
-- Return false as connections are still in use, warnings are issued.
@@ -9375,15 +9376,15 @@ SELECT postgres_fdw_disconnect_all();
RESET client_min_messages;
COMMIT;
-- Ensure that loopback2 connection is closed.
-SELECT 1 FROM postgres_fdw_disconnect('loopback2');
- ?column?
-----------
- 1
+SELECT postgres_fdw_disconnect('loopback2');
+ postgres_fdw_disconnect
+-------------------------
+ t
(1 row)
-SELECT server_name FROM postgres_fdw_get_connections() WHERE server_name = 'loopback2';
- server_name
--------------
+SELECT * FROM postgres_fdw_get_connections() WHERE server_name = 'loopback2';
+ server_name | valid
+-------------+-------
(0 rows)
-- Return false as loopback2 connection is closed already.
@@ -9397,18 +9398,25 @@ SELECT postgres_fdw_disconnect('loopback2');
SELECT postgres_fdw_disconnect('unknownserver');
ERROR: server "unknownserver" does not exist
-- Let's ensure to close all the existing cached connections.
-SELECT 1 FROM postgres_fdw_disconnect_all();
- ?column?
-----------
- 1
+SELECT postgres_fdw_disconnect_all();
+ postgres_fdw_disconnect_all
+-----------------------------
+ t
(1 row)
-- No cached connections, so no records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name
--------------
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | valid
+-------------+-------
(0 rows)
+-- No cached connections, so false should be output.
+SELECT postgres_fdw_disconnect_all();
+ postgres_fdw_disconnect_all
+-----------------------------
+ f
+(1 row)
+
-- =============================================================================
-- test case for having multiple cached connections for a foreign server
-- =============================================================================
@@ -9436,25 +9444,25 @@ SELECT 1 FROM ft1 LIMIT 1;
RESET ROLE;
-- Should output two connections for loopback server
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name
--------------
- loopback
- loopback
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | valid
+-------------+-------
+ loopback | t
+ loopback | t
(2 rows)
COMMIT;
-- Let's ensure to close all the existing cached connections.
-SELECT 1 FROM postgres_fdw_disconnect_all();
- ?column?
-----------
- 1
+SELECT postgres_fdw_disconnect_all();
+ postgres_fdw_disconnect_all
+-----------------------------
+ t
(1 row)
-- No cached connections, so no records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name
--------------
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | valid
+-------------+-------
(0 rows)
-- Clean up
@@ -9477,12 +9485,13 @@ SELECT 1 FROM ft1 LIMIT 1;
(1 row)
-- No cached connections, so no records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name
--------------
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | valid
+-------------+-------
(0 rows)
ALTER SERVER loopback OPTIONS (SET keep_connections 'on');
+RESET debug_invalidate_system_caches_always;
-- ===================================================================
-- batch insert
-- ===================================================================
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 000e2534fc..f2a502f992 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -2788,32 +2788,32 @@ SELECT count(*) FROM ft1;
PREPARE TRANSACTION 'fdw_tpc';
ROLLBACK;
+-- Let's ensure to close all the existing cached connections so that they don't
+-- make the following tests unstable.
+SELECT 1 FROM postgres_fdw_disconnect_all();
+
+-- If debug_invalidate_system_caches_always is active, it results in dropping
+-- remote connections after every transaction, making it impossible to test
+-- connection retry use case meaningfully. And also, the new functions
+-- introduced for cached connections management will be unstable with the flag
+-- on. So turn it off for these tests.
+SET debug_invalidate_system_caches_always = 0;
+
-- ===================================================================
-- reestablish new connection
-- ===================================================================
-
-- Change application_name of remote connection to special one
-- so that we can easily terminate the connection later.
ALTER SERVER loopback OPTIONS (application_name 'fdw_retry_check');
-
--- If debug_invalidate_system_caches_always is active, it results in
--- dropping remote connections after every transaction, making it
--- impossible to test termination meaningfully. So turn that off
--- for this test.
-SET debug_invalidate_system_caches_always = 0;
-
-- Make sure we have a remote connection.
SELECT 1 FROM ft1 LIMIT 1;
-
-- Terminate the remote connection and wait for the termination to complete.
SELECT pg_terminate_backend(pid, 180000) FROM pg_stat_activity
WHERE application_name = 'fdw_retry_check';
-
-- This query should detect the broken connection when starting new remote
-- transaction, reestablish new connection, and then succeed.
BEGIN;
SELECT 1 FROM ft1 LIMIT 1;
-
-- If we detect the broken connection when starting a new remote
-- subtransaction, we should fail instead of establishing a new connection.
-- Terminate the remote connection and wait for the termination to complete.
@@ -2826,15 +2826,11 @@ SELECT 1 FROM ft1 LIMIT 1; -- should fail
\set VERBOSITY default
COMMIT;
-RESET debug_invalidate_system_caches_always;
-
-- =============================================================================
-- test connection invalidation cases and postgres_fdw_get_connections function
-- =============================================================================
--- Let's ensure to close all the existing cached connections.
-SELECT 1 FROM postgres_fdw_disconnect_all();
-- No cached connections, so no records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-- This test case is for closing the connection in pgfdw_xact_callback
BEGIN;
-- Connection xact depth becomes 1 i.e. the connection is in midst of the xact.
@@ -2842,7 +2838,7 @@ SELECT 1 FROM ft1 LIMIT 1;
SELECT 1 FROM ft7 LIMIT 1;
-- List all the existing cached connections. loopback and loopback3 should be
-- output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-- Connections are not closed at the end of the alter and drop statements.
-- That's because the connections are in midst of this xact,
-- they are just marked as invalid in pgfdw_inval_callback.
@@ -2856,7 +2852,7 @@ SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
COMMIT;
-- All cached connections were closed while committing above xact, so no
-- records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-- =======================================================================
-- test postgres_fdw_disconnect and postgres_fdw_disconnect_all functions
@@ -2868,13 +2864,13 @@ SELECT 1 FROM ft1 LIMIT 1;
SELECT 1 FROM ft6 LIMIT 1;
-- List all the existing cached connections. loopback and loopback2 should be
-- output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-- Issue a warning and return false as loopback connection is still in use and
-- can not be closed.
SELECT postgres_fdw_disconnect('loopback');
-- List all the existing cached connections. loopback and loopback2 should be
-- output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-- Return false as connections are still in use, warnings are issued.
-- But disable warnings temporarily because the order of them is not stable.
SET client_min_messages = 'ERROR';
@@ -2882,16 +2878,18 @@ SELECT postgres_fdw_disconnect_all();
RESET client_min_messages;
COMMIT;
-- Ensure that loopback2 connection is closed.
-SELECT 1 FROM postgres_fdw_disconnect('loopback2');
-SELECT server_name FROM postgres_fdw_get_connections() WHERE server_name = 'loopback2';
+SELECT postgres_fdw_disconnect('loopback2');
+SELECT * FROM postgres_fdw_get_connections() WHERE server_name = 'loopback2';
-- Return false as loopback2 connection is closed already.
SELECT postgres_fdw_disconnect('loopback2');
-- Return an error as there is no foreign server with given name.
SELECT postgres_fdw_disconnect('unknownserver');
-- Let's ensure to close all the existing cached connections.
-SELECT 1 FROM postgres_fdw_disconnect_all();
+SELECT postgres_fdw_disconnect_all();
-- No cached connections, so no records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
+-- No cached connections, so false should be output.
+SELECT postgres_fdw_disconnect_all();
-- =============================================================================
-- test case for having multiple cached connections for a foreign server
@@ -2906,19 +2904,17 @@ BEGIN;
SET ROLE regress_multi_conn_user1;
SELECT 1 FROM ft1 LIMIT 1;
RESET ROLE;
-
-- Will cache loopback connection with user mapping for regress_multi_conn_user2
SET ROLE regress_multi_conn_user2;
SELECT 1 FROM ft1 LIMIT 1;
RESET ROLE;
-
-- Should output two connections for loopback server
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
COMMIT;
-- Let's ensure to close all the existing cached connections.
-SELECT 1 FROM postgres_fdw_disconnect_all();
+SELECT postgres_fdw_disconnect_all();
-- No cached connections, so no records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-- Clean up
DROP USER MAPPING FOR regress_multi_conn_user1 SERVER loopback;
@@ -2936,9 +2932,11 @@ ALTER SERVER loopback OPTIONS (keep_connections 'off');
-- as keep_connections was set to off.
SELECT 1 FROM ft1 LIMIT 1;
-- No cached connections, so no records should be output.
-SELECT server_name FROM postgres_fdw_get_connections() ORDER BY 1;
+SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
ALTER SERVER loopback OPTIONS (SET keep_connections 'on');
+RESET debug_invalidate_system_caches_always;
+
-- ===================================================================
-- batch insert
-- ===================================================================
--
2.25.1
