diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 06673017bcf..3d2a8d0519d 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -1480,6 +1480,11 @@ pgfdw_inval_callback(Datum arg, SysCacheIdentifier cacheid, uint32 hashvalue) * Such connections can't safely be further used. Re-establishing the * connection would change the snapshot and roll back any writes already * performed, so that's not an option, either. Thus, we must abort. + * + * Note: there might be open cursors that use the connection, so even if the + * connection cache entry is marked as such, we will retain it until abort + * cleanup of the main transaction, to ensure such open cursors can safely + * refer to the PGconn for the connection. */ static void pgfdw_reject_incomplete_xact_state_change(ConnCacheEntry *entry) @@ -1490,15 +1495,12 @@ pgfdw_reject_incomplete_xact_state_change(ConnCacheEntry *entry) if (entry->conn == NULL || !entry->changing_xact_state) return; - /* make sure this entry is inactive */ - disconnect_pg_server(entry); - /* find server name to be shown in the message below */ server = GetForeignServer(entry->serverid); ereport(ERROR, (errcode(ERRCODE_CONNECTION_EXCEPTION), - errmsg("connection to server \"%s\" was lost", + errmsg("connection to server \"%s\" cannot be used due to abort cleanup failure", server->servername))); } diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 10e87acabef..aaffcf31271 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -12977,3 +12977,79 @@ SELECT server_name, -- Clean up \set VERBOSITY default RESET debug_discard_caches; +-- =================================================================== +-- test cleanup of failed connections on abort +-- =================================================================== +CREATE VIEW my_backend_pid (pid) AS SELECT pg_backend_pid(); +CREATE FOREIGN TABLE remote_backend_pid (pid int) + SERVER loopback OPTIONS (table_name 'my_backend_pid'); +CREATE FUNCTION wait_for_backend_termination(int) RETURNS void AS $$ + BEGIN + WHILE (SELECT count(*) FROM pg_stat_activity WHERE pid = $1) > 0 + LOOP + PERFORM pg_stat_clear_snapshot(); + END LOOP; + END +$$ LANGUAGE plpgsql; +SET client_min_messages = 'ERROR'; +BEGIN; +DECLARE c CURSOR FOR SELECT * FROM ft1 ORDER BY c1; +SAVEPOINT s; +SELECT pid AS remote_pid FROM remote_backend_pid \gset +SELECT pg_terminate_backend(:remote_pid); + pg_terminate_backend +---------------------- + t +(1 row) + +SELECT wait_for_backend_termination(:remote_pid); + wait_for_backend_termination +------------------------------ + +(1 row) + +ROLLBACK TO SAVEPOINT s; +SELECT pid FROM remote_backend_pid; +ERROR: connection to server "loopback" cannot be used due to abort cleanup failure +ROLLBACK TO SAVEPOINT s; +RELEASE SAVEPOINT s; +FETCH c; +ERROR: no connection to the server +CONTEXT: remote SQL command: DECLARE c1 CURSOR FOR +SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" ORDER BY "C 1" ASC NULLS LAST +ABORT; +BEGIN; +DECLARE c CURSOR FOR SELECT * FROM ft1 ORDER BY c1; +FETCH c; + c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 +----+----+-------------------+------------------------------+--------------------------+----+------------+----- + 1 | 2 | 00001_trig_update | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo +(1 row) + +SAVEPOINT s; +SELECT pid AS remote_pid FROM remote_backend_pid \gset +SELECT pg_terminate_backend(:remote_pid); + pg_terminate_backend +---------------------- + t +(1 row) + +SELECT wait_for_backend_termination(:remote_pid); + wait_for_backend_termination +------------------------------ + +(1 row) + +ROLLBACK TO SAVEPOINT s; +SELECT pid FROM remote_backend_pid; +ERROR: connection to server "loopback" cannot be used due to abort cleanup failure +ROLLBACK TO SAVEPOINT s; +RELEASE SAVEPOINT s; +CLOSE c; +ERROR: no connection to the server +CONTEXT: remote SQL command: CLOSE c1 +ABORT; +RESET client_min_messages; +DROP FUNCTION wait_for_backend_termination(int); +DROP FOREIGN TABLE remote_backend_pid; +DROP VIEW my_backend_pid; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 79ad5be8bf9..267d3c1a7e7 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -4620,3 +4620,54 @@ SELECT server_name, -- Clean up \set VERBOSITY default RESET debug_discard_caches; + +-- =================================================================== +-- test cleanup of failed connections on abort +-- =================================================================== + +CREATE VIEW my_backend_pid (pid) AS SELECT pg_backend_pid(); +CREATE FOREIGN TABLE remote_backend_pid (pid int) + SERVER loopback OPTIONS (table_name 'my_backend_pid'); +CREATE FUNCTION wait_for_backend_termination(int) RETURNS void AS $$ + BEGIN + WHILE (SELECT count(*) FROM pg_stat_activity WHERE pid = $1) > 0 + LOOP + PERFORM pg_stat_clear_snapshot(); + END LOOP; + END +$$ LANGUAGE plpgsql; + +SET client_min_messages = 'ERROR'; + +BEGIN; +DECLARE c CURSOR FOR SELECT * FROM ft1 ORDER BY c1; +SAVEPOINT s; +SELECT pid AS remote_pid FROM remote_backend_pid \gset +SELECT pg_terminate_backend(:remote_pid); +SELECT wait_for_backend_termination(:remote_pid); +ROLLBACK TO SAVEPOINT s; +SELECT pid FROM remote_backend_pid; +ROLLBACK TO SAVEPOINT s; +RELEASE SAVEPOINT s; +FETCH c; +ABORT; + +BEGIN; +DECLARE c CURSOR FOR SELECT * FROM ft1 ORDER BY c1; +FETCH c; +SAVEPOINT s; +SELECT pid AS remote_pid FROM remote_backend_pid \gset +SELECT pg_terminate_backend(:remote_pid); +SELECT wait_for_backend_termination(:remote_pid); +ROLLBACK TO SAVEPOINT s; +SELECT pid FROM remote_backend_pid; +ROLLBACK TO SAVEPOINT s; +RELEASE SAVEPOINT s; +CLOSE c; +ABORT; + +RESET client_min_messages; + +DROP FUNCTION wait_for_backend_termination(int); +DROP FOREIGN TABLE remote_backend_pid; +DROP VIEW my_backend_pid;