From 2dabc150ee982d33662cbae5a34c7b2b9cd537a5 Mon Sep 17 00:00:00 2001 From: Fujii Masao Date: Fri, 12 Jun 2026 12:46:13 +0900 Subject: [PATCH v1] postgres_fdw: stabilize terminated-connection regression tests The regression test for postgres_fdw_get_connections(true) assumed that a terminated remote connection would still remain visible in the FDW connection cache long enough to be reported as closed with a nonzero remote_backend_pid. That assumption is not always valid. postgres_fdw_get_connections() reports only entries that are still present in ConnectionHash, while pgfdw_inval_callback() may immediately discard an idle cached connection (xact_depth == 0) when a relevant invalidation arrives. In CI, that can happen between terminating the remote backend and querying postgres_fdw_get_connections(true), causing the function to return no rows. Adjust the idle-connection test to accept either outcome: if the cache entry is still present, verify that it reports the expected server name, closed status, and nonzero remote backend PID; otherwise treat zero rows as a legitimate result. To preserve coverage of the terminated-backend reporting path, add a separate check inside an explicit transaction. In that case, concurrent invalidation may mark the connection invalid but cannot discard it before transaction end, so postgres_fdw_get_connections(true) should still report the terminated connection as in-use, closed, and associated with a nonzero remote backend PID. --- .../postgres_fdw/expected/postgres_fdw.out | 60 ++++++++++++++++--- contrib/postgres_fdw/sql/postgres_fdw.sql | 40 +++++++++++-- 2 files changed, 87 insertions(+), 13 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index e90289e4ab1..c882bf9accb 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -12969,23 +12969,67 @@ SELECT server_name, loopback | f | t (1 row) --- After terminating the remote backend, since the connection is closed, --- "closed" should be TRUE, or NULL if the connection status check --- is not available. Despite the termination, remote_backend_pid should --- still show the non-zero PID of the terminated remote backend. +-- After terminating the remote backend, if the connection entry is still in +-- the cache, "closed" should be TRUE, or NULL if the connection status check +-- is not available, and remote_backend_pid should still show the non-zero PID +-- of the terminated remote backend. Concurrent invalidation can remove the +-- idle cached connection before the next statement, in which case +-- postgres_fdw_get_connections(true) can legitimately return no rows. DO $$ BEGIN PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity WHERE application_name = 'fdw_conn_check'; END $$; -SELECT server_name, +WITH terminated_conn AS ( + SELECT server_name, + CASE WHEN closed IS NOT false THEN true ELSE false END AS closed, + remote_backend_pid <> 0 AS remote_backend_pid + FROM postgres_fdw_get_connections(true) +) +SELECT CASE + WHEN count(*) = 0 THEN true + WHEN count(*) = 1 THEN bool_and(server_name = 'loopback' + AND closed + AND remote_backend_pid) + ELSE false +END AS ok +FROM terminated_conn; + ok +---- + t +(1 row) + +-- In an explicit transaction, concurrent invalidation may mark the +-- connection invalid but cannot discard it before transaction end, so the +-- terminated connection should remain visible in the cache. +SELECT 1 FROM postgres_fdw_disconnect_all(); + ?column? +---------- + 1 +(1 row) + +SET client_min_messages = 'ERROR'; +BEGIN; +SELECT 1 FROM ft1 LIMIT 1; + ?column? +---------- + 1 +(1 row) + +DO $$ BEGIN +PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity + WHERE application_name = 'fdw_conn_check'; +END $$; +SELECT server_name, used_in_xact, CASE WHEN closed IS NOT false THEN true ELSE false END AS closed, remote_backend_pid <> 0 AS remote_backend_pid FROM postgres_fdw_get_connections(true); - server_name | closed | remote_backend_pid --------------+--------+-------------------- - loopback | t | t + server_name | used_in_xact | closed | remote_backend_pid +-------------+--------------+--------+-------------------- + loopback | t | t | t (1 row) +ABORT; +RESET client_min_messages; -- Clean up \set VERBOSITY default RESET debug_discard_caches; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index dfc58beb0d2..1f0e4461071 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -4617,18 +4617,48 @@ SELECT server_name, WHERE application_name = 'fdw_conn_check') AS remote_backend_pid FROM postgres_fdw_get_connections(true); --- After terminating the remote backend, since the connection is closed, --- "closed" should be TRUE, or NULL if the connection status check --- is not available. Despite the termination, remote_backend_pid should --- still show the non-zero PID of the terminated remote backend. +-- After terminating the remote backend, if the connection entry is still in +-- the cache, "closed" should be TRUE, or NULL if the connection status check +-- is not available, and remote_backend_pid should still show the non-zero PID +-- of the terminated remote backend. Concurrent invalidation can remove the +-- idle cached connection before the next statement, in which case +-- postgres_fdw_get_connections(true) can legitimately return no rows. DO $$ BEGIN PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity WHERE application_name = 'fdw_conn_check'; END $$; -SELECT server_name, +WITH terminated_conn AS ( + SELECT server_name, + CASE WHEN closed IS NOT false THEN true ELSE false END AS closed, + remote_backend_pid <> 0 AS remote_backend_pid + FROM postgres_fdw_get_connections(true) +) +SELECT CASE + WHEN count(*) = 0 THEN true + WHEN count(*) = 1 THEN bool_and(server_name = 'loopback' + AND closed + AND remote_backend_pid) + ELSE false +END AS ok +FROM terminated_conn; + +-- In an explicit transaction, concurrent invalidation may mark the +-- connection invalid but cannot discard it before transaction end, so the +-- terminated connection should remain visible in the cache. +SELECT 1 FROM postgres_fdw_disconnect_all(); +SET client_min_messages = 'ERROR'; +BEGIN; +SELECT 1 FROM ft1 LIMIT 1; +DO $$ BEGIN +PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity + WHERE application_name = 'fdw_conn_check'; +END $$; +SELECT server_name, used_in_xact, CASE WHEN closed IS NOT false THEN true ELSE false END AS closed, remote_backend_pid <> 0 AS remote_backend_pid FROM postgres_fdw_get_connections(true); +ABORT; +RESET client_min_messages; -- Clean up \set VERBOSITY default -- 2.53.0