Simplify tests of postgres_fdw terminating connections

michaelpq · michaelpq · commit 93f41461449f · 2021-04-14T14:23:53.000+09:00
The tests introduced in 32a9c0b for connections broken and re-established rely on pg_terminate_backend() for their logic. When these were introduced, this function simply sent a signal to a backend without waiting for the operation to complete, and the tests repeatedly looked at pg_stat_activity to check if the operation was completed or not. Since aaf0432, it is possible to define a timeout to make pg_terminate_backend() wait for a certain duration, so make use of it, with a timeout reasonably large enough (3min) to give enough room for the tests to pass even on slow machines. Some measurements show that the tests of postgres_fdw are much faster with this change. For example, on my laptop, they now take 4s instead of 6s. Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACXGY_EfGrMTjKjHy2zi-u1u9rdeioU_fro0T6Jo8t56KQ@mail.gmail.com
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -9195,19 +9195,6 @@ WARNING:  there is no transaction in progress
 -- ===================================================================
 -- reestablish new connection
 -- ===================================================================
--- Terminate the backend having the specified application_name and wait for
--- the termination to complete.
-CREATE OR REPLACE PROCEDURE terminate_backend_and_wait(appname text) AS $$
-BEGIN
-    PERFORM pg_terminate_backend(pid) FROM pg_stat_activity
-    WHERE application_name = appname;
-    LOOP
-        PERFORM * FROM pg_stat_activity WHERE application_name = appname;
-        EXIT WHEN NOT FOUND;
-        PERFORM pg_sleep(1), pg_stat_clear_snapshot();
-    END LOOP;
-END;
-$$ LANGUAGE plpgsql;
 -- 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');
@@ -9217,8 +9204,14 @@ SELECT 1 FROM ft1 LIMIT 1;
         1
 (1 row)
 
--- Terminate the remote connection.
-CALL terminate_backend_and_wait('fdw_retry_check');
+-- 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';
+ pg_terminate_backend 
+----------------------
+ t
+(1 row)
+
 -- This query should detect the broken connection when starting new remote
 -- transaction, reestablish new connection, and then succeed.
 BEGIN;
@@ -9231,15 +9224,20 @@ SELECT 1 FROM ft1 LIMIT 1;
 -- If the query detects the broken connection when starting new remote
 -- subtransaction, it doesn't reestablish new connection and should fail.
 -- The text of the error might vary across platforms, so don't show it.
-CALL terminate_backend_and_wait('fdw_retry_check');
+-- 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';
+ pg_terminate_backend 
+----------------------
+ t
+(1 row)
+
 SAVEPOINT s;
 \set VERBOSITY sqlstate
 SELECT 1 FROM ft1 LIMIT 1;    -- should fail
 ERROR:  08006
 \set VERBOSITY default
 COMMIT;
--- Clean up
-DROP PROCEDURE terminate_backend_and_wait(text);
 -- =============================================================================
 -- test connection invalidation cases and postgres_fdw_get_connections function
 -- =============================================================================
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -2786,27 +2786,14 @@ ROLLBACK;
 -- reestablish new connection
 -- ===================================================================
 
--- Terminate the backend having the specified application_name and wait for
--- the termination to complete.
-CREATE OR REPLACE PROCEDURE terminate_backend_and_wait(appname text) AS $$
-BEGIN
-    PERFORM pg_terminate_backend(pid) FROM pg_stat_activity
-    WHERE application_name = appname;
-    LOOP
-        PERFORM * FROM pg_stat_activity WHERE application_name = appname;
-        EXIT WHEN NOT FOUND;
-        PERFORM pg_sleep(1), pg_stat_clear_snapshot();
-    END LOOP;
-END;
-$$ LANGUAGE plpgsql;
-
 -- 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');
 SELECT 1 FROM ft1 LIMIT 1;
 
--- Terminate the remote connection.
-CALL terminate_backend_and_wait('fdw_retry_check');
+-- 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.
@@ -2816,16 +2803,15 @@ SELECT 1 FROM ft1 LIMIT 1;
 -- If the query detects the broken connection when starting new remote
 -- subtransaction, it doesn't reestablish new connection and should fail.
 -- The text of the error might vary across platforms, so don't show it.
-CALL terminate_backend_and_wait('fdw_retry_check');
+-- 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';
 SAVEPOINT s;
 \set VERBOSITY sqlstate
 SELECT 1 FROM ft1 LIMIT 1;    -- should fail
 \set VERBOSITY default
 COMMIT;
 
--- Clean up
-DROP PROCEDURE terminate_backend_and_wait(text);
-
 -- =============================================================================
 -- test connection invalidation cases and postgres_fdw_get_connections function
 -- =============================================================================
