Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 03b7a1e

Browse filesBrowse files
MasaoFujiiMasaoFujii
committed
postgres_fdw: Fix connection leak.
In postgres_fdw, the cached connections to foreign servers will not be closed until the local session exits if the user mappings or foreign servers that those connections depend on are dropped. Those connections can be leaked. To fix that connection leak issue, after a change to a pg_foreign_server or pg_user_mapping catalog entry, this commit makes postgres_fdw close the connections depending on that entry immediately if current transaction has not used those connections yet. Otherwise, mark those connections as invalid and then 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. Back-patch to all supported branches. Author: Bharath Rupireddy Reviewed-by: Zhihong Yu, Zhijie Hou, Fujii Masao Discussion: https://postgr.es/m/CALj2ACVNcGH_6qLY-4_tXz8JLvA+4yeBThRfxMz7Oxbk1aHcpQ@mail.gmail.com
1 parent 3f920e8 commit 03b7a1e
Copy full SHA for 03b7a1e

File tree

Expand file treeCollapse file tree

3 files changed

+58
-7
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+58
-7
lines changed

‎contrib/postgres_fdw/connection.c

Copy file name to clipboardExpand all lines: contrib/postgres_fdw/connection.c
+26-7Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -820,12 +820,14 @@ pgfdw_xact_callback(XactEvent event, void *arg)
820820
entry->xact_depth = 0;
821821

822822
/*
823-
* If the connection isn't in a good idle state, discard it to
824-
* recover. Next GetConnection will open a new connection.
823+
* If the connection isn't in a good idle state or it is marked as
824+
* invalid, then discard it to recover. Next GetConnection will open a
825+
* new connection.
825826
*/
826827
if (PQstatus(entry->conn) != CONNECTION_OK ||
827828
PQtransactionStatus(entry->conn) != PQTRANS_IDLE ||
828-
entry->changing_xact_state)
829+
entry->changing_xact_state ||
830+
entry->invalidated)
829831
{
830832
elog(DEBUG3, "discarding connection %p", entry->conn);
831833
disconnect_pg_server(entry);
@@ -949,9 +951,12 @@ pgfdw_subxact_callback(SubXactEvent event, SubTransactionId mySubid,
949951
* Connection invalidation callback function
950952
*
951953
* After a change to a pg_foreign_server or pg_user_mapping catalog entry,
952-
* mark connections depending on that entry as needing to be remade.
953-
* We can't immediately destroy them, since they might be in the midst of
954-
* a transaction, but we'll remake them at the next opportunity.
954+
* close connections depending on that entry immediately if current transaction
955+
* has not used those connections yet. Otherwise, mark those connections as
956+
* invalid and then make pgfdw_xact_callback() close them at the end of current
957+
* transaction, since they cannot be closed in the midst of the transaction
958+
* using them. Closed connections will be remade at the next opportunity if
959+
* necessary.
955960
*
956961
* Although most cache invalidation callbacks blow away all the related stuff
957962
* regardless of the given hashvalue, connections are expensive enough that
@@ -982,7 +987,21 @@ pgfdw_inval_callback(Datum arg, int cacheid, uint32 hashvalue)
982987
entry->server_hashvalue == hashvalue) ||
983988
(cacheid == USERMAPPINGOID &&
984989
entry->mapping_hashvalue == hashvalue))
985-
entry->invalidated = true;
990+
{
991+
/*
992+
* Close the connection immediately if it's not used yet in this
993+
* transaction. Otherwise mark it as invalid so that
994+
* pgfdw_xact_callback() can close it at the end of this
995+
* transaction.
996+
*/
997+
if (entry->xact_depth == 0)
998+
{
999+
elog(DEBUG3, "discarding connection %p", entry->conn);
1000+
disconnect_pg_server(entry);
1001+
}
1002+
else
1003+
entry->invalidated = true;
1004+
}
9861005
}
9871006
}
9881007

‎contrib/postgres_fdw/expected/postgres_fdw.out

Copy file name to clipboardExpand all lines: contrib/postgres_fdw/expected/postgres_fdw.out
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6318,3 +6318,21 @@ AND ftoptions @> array['fetch_size=60000'];
63186318
(1 row)
63196319

63206320
ROLLBACK;
6321+
-- ===================================================================
6322+
-- test connection invalidation cases
6323+
-- ===================================================================
6324+
-- This test case is for closing the connection in pgfdw_xact_callback
6325+
BEGIN;
6326+
-- Connection xact depth becomes 1 i.e. the connection is in midst of the xact.
6327+
SELECT 1 FROM ft1 LIMIT 1;
6328+
?column?
6329+
----------
6330+
1
6331+
(1 row)
6332+
6333+
-- Connection is not closed at the end of the alter statement in
6334+
-- pgfdw_inval_callback. That's because the connection is in midst of this
6335+
-- xact, it is just marked as invalid.
6336+
ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off');
6337+
-- The invalid connection gets closed in pgfdw_xact_callback during commit.
6338+
COMMIT;

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Copy file name to clipboardExpand all lines: contrib/postgres_fdw/sql/postgres_fdw.sql
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1504,3 +1504,17 @@ WHERE ftrelid = 'table30000'::regclass
15041504
AND ftoptions @> array['fetch_size=60000'];
15051505

15061506
ROLLBACK;
1507+
1508+
-- ===================================================================
1509+
-- test connection invalidation cases
1510+
-- ===================================================================
1511+
-- This test case is for closing the connection in pgfdw_xact_callback
1512+
BEGIN;
1513+
-- Connection xact depth becomes 1 i.e. the connection is in midst of the xact.
1514+
SELECT 1 FROM ft1 LIMIT 1;
1515+
-- Connection is not closed at the end of the alter statement in
1516+
-- pgfdw_inval_callback. That's because the connection is in midst of this
1517+
-- xact, it is just marked as invalid.
1518+
ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off');
1519+
-- The invalid connection gets closed in pgfdw_xact_callback during commit.
1520+
COMMIT;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.