diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index f382203b8f..781c526b83 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -812,12 +812,14 @@ pgfdw_xact_callback(XactEvent event, void *arg) entry->xact_depth = 0; /* - * If the connection isn't in a good idle state, discard it to - * recover. Next GetConnection will open a new connection. + * If the connection isn't in a good idle state or it is marked as + * invalid, then discard it to recover. Next GetConnection will open a + * new connection. */ if (PQstatus(entry->conn) != CONNECTION_OK || PQtransactionStatus(entry->conn) != PQTRANS_IDLE || - entry->changing_xact_state) + entry->changing_xact_state || + entry->invalidated) { elog(DEBUG3, "discarding connection %p", entry->conn); disconnect_pg_server(entry); @@ -941,9 +943,12 @@ pgfdw_subxact_callback(SubXactEvent event, SubTransactionId mySubid, * Connection invalidation callback function * * After a change to a pg_foreign_server or pg_user_mapping catalog entry, - * mark connections depending on that entry as needing to be remade. - * We can't immediately destroy them, since they might be in the midst of - * a transaction, but we'll remake them at the next opportunity. + * close connections depending on that entry immediately if current transaction + * has not used those connections yet. Otherwise, mark those connections as + * invalid and then make pgfdw_xact_callback() close them at the end of current + * transaction, since they cannot be closed in the midst of the transaction + * using them. Closed connections will be remade at the next opportunity if + * necessary. * * Although most cache invalidation callbacks blow away all the related stuff * regardless of the given hashvalue, connections are expensive enough that @@ -974,7 +979,21 @@ pgfdw_inval_callback(Datum arg, int cacheid, uint32 hashvalue) entry->server_hashvalue == hashvalue) || (cacheid == USERMAPPINGOID && entry->mapping_hashvalue == hashvalue)) - entry->invalidated = true; + { + /* + * Close the connection immediately if it's not used yet in this + * transaction. Otherwise mark it as invalid so that + * pgfdw_xact_callback() can close it at the end of this + * transaction. + */ + if (entry->xact_depth == 0) + { + elog(DEBUG3, "discarding connection %p", entry->conn); + disconnect_pg_server(entry); + } + else + entry->invalidated = true; + } } } diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 8c5aa2df21..252b8dab60 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -8842,3 +8842,21 @@ SELECT b, avg(a), max(a), count(*) FROM pagg_tab GROUP BY b HAVING sum(a) < 700 -- Clean-up RESET enable_partitionwise_aggregate; +-- =================================================================== +-- test connection invalidation cases +-- =================================================================== +-- 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. +SELECT 1 FROM ft1 LIMIT 1; + ?column? +---------- + 1 +(1 row) + +-- Connection is not closed at the end of the alter statement in +-- pgfdw_inval_callback. That's because the connection is in midst of this +-- xact, it is just marked as invalid. +ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off'); +-- The invalid connection gets closed in pgfdw_xact_callback during commit. +COMMIT; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index f16684b365..a957ef1b1d 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -2516,3 +2516,17 @@ SELECT b, avg(a), max(a), count(*) FROM pagg_tab GROUP BY b HAVING sum(a) < 700 -- Clean-up RESET enable_partitionwise_aggregate; + +-- =================================================================== +-- test connection invalidation cases +-- =================================================================== +-- 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. +SELECT 1 FROM ft1 LIMIT 1; +-- Connection is not closed at the end of the alter statement in +-- pgfdw_inval_callback. That's because the connection is in midst of this +-- xact, it is just marked as invalid. +ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off'); +-- The invalid connection gets closed in pgfdw_xact_callback during commit. +COMMIT;