On 2025/02/28 15:10, Sagar Shedge wrote:
For now, I think it makes sense to keep postgres_fdw_get_connections()
aligned with the current implementation. Otherwise, explaining what
remote_backend_pid = NULL means could be confusing, especially since
pipeline mode isn't supported yet in postgres_fdw.

Make sense. I have created a patch according to the above suggestions.
Please review.

Thanks for updating the patch!

I made a few cosmetic adjustments and attached the revised version.
Unless there are any objections, I plan to commit this.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From e510af4e2e04d4d9500fa7d1ddc9ed0386bdca64 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fu...@postgresql.org>
Date: Fri, 28 Feb 2025 18:37:05 +0900
Subject: [PATCH v3] postgres_fdw: Extend postgres_fdw_get_connections to
 return remote backend PID.

This commit adds a new "remote_backend_pid" output column to
the postgres_fdw_get_connections function. It returns the process ID of
the remote backend, on the foreign server, handling the connection.

This enhancement is useful for troubleshooting, monitoring, and reporting.
For example, if a connection is unexpectedly closed by the foreign server,
the remote backend's PID can help diagnose the cause.

No extension version bump is needed, as commit c297a47c5f already
handled it for v18~.

Author: Sagar Dilip Shedge <sagar.shedg...@gmail.com>
Reviewed-by: Fujii Masao <masao.fu...@gmail.com>
Discussion: 
https://postgr.es/m/caphyiff25q5xuqwxetfkwhc0yva_6+tfg9kw4bcvcjpcwxy...@mail.gmail.com
---
 contrib/postgres_fdw/connection.c             | 11 +++--
 .../postgres_fdw/expected/postgres_fdw.out    | 46 ++++++++++++-------
 .../postgres_fdw/postgres_fdw--1.1--1.2.sql   |  2 +-
 contrib/postgres_fdw/sql/postgres_fdw.sql     | 26 ++++++++---
 doc/src/sgml/postgres-fdw.sgml                | 23 +++++++---
 5 files changed, 76 insertions(+), 32 deletions(-)

diff --git a/contrib/postgres_fdw/connection.c 
b/contrib/postgres_fdw/connection.c
index 8a8d3b4481f..8ef9702c05c 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -2111,8 +2111,8 @@ pgfdw_finish_abort_cleanup(List *pending_entries, List 
*cancel_requested,
 
 /* Number of output arguments (columns) for various API versions */
 #define POSTGRES_FDW_GET_CONNECTIONS_COLS_V1_1 2
-#define POSTGRES_FDW_GET_CONNECTIONS_COLS_V1_2 5
-#define POSTGRES_FDW_GET_CONNECTIONS_COLS      5       /* maximum of above */
+#define POSTGRES_FDW_GET_CONNECTIONS_COLS_V1_2 6
+#define POSTGRES_FDW_GET_CONNECTIONS_COLS      6       /* maximum of above */
 
 /*
  * Internal function used by postgres_fdw_get_connections variants.
@@ -2128,13 +2128,15 @@ pgfdw_finish_abort_cleanup(List *pending_entries, List 
*cancel_requested,
  *
  * For API version 1.2 and later, this function takes an input parameter
  * to check a connection status and returns the following
- * additional values along with the three values from version 1.1:
+ * additional values along with the four values from version 1.1:
  *
  * - user_name - the local user name of the active connection. In case the
  *   user mapping is dropped but the connection is still active, then the
  *   user name will be NULL in the output.
  * - used_in_xact - true if the connection is used in the current transaction.
  * - closed - true if the connection is closed.
+ * - remote_backend_pid - process ID of the remote backend, on the foreign
+ *   server, handling the connection.
  *
  * No records are returned when there are no cached connections at all.
  */
@@ -2273,6 +2275,9 @@ postgres_fdw_get_connections_internal(FunctionCallInfo 
fcinfo,
                                values[i++] = 
BoolGetDatum(pgfdw_conn_check(entry->conn) != 0);
                        else
                                nulls[i++] = true;
+
+                       /* Return process ID of remote backend */
+                       values[i++] = Int32GetDatum(PQbackendPID(entry->conn));
                }
 
                tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, 
values, nulls);
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out 
b/contrib/postgres_fdw/expected/postgres_fdw.out
index 8447b289cb7..c68119030ab 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10589,13 +10589,18 @@ drop cascades to foreign table ft7
 -- List all the existing cached connections. loopback and loopback3
 -- should be output as invalid connections. Also the server name and user name
 -- for loopback3 should be NULL because both server and user mapping were
--- dropped.
-SELECT server_name, user_name = CURRENT_USER as "user_name = CURRENT_USER", 
valid, used_in_xact, closed
-FROM postgres_fdw_get_connections() ORDER BY 1;
- server_name | user_name = CURRENT_USER | valid | used_in_xact | closed 
--------------+--------------------------+-------+--------------+--------
- loopback    | t                        | f     | t            | 
-             |                          | f     | t            | 
+-- dropped. In this test, the PIDs of remote backends can be gathered from
+-- pg_stat_activity, and remote_backend_pid should match one of those PIDs.
+SELECT server_name, user_name = CURRENT_USER as "user_name = CURRENT_USER",
+  valid, used_in_xact, closed,
+  remote_backend_pid = ANY(SELECT pid FROM pg_stat_activity
+  WHERE backend_type = 'client backend' AND pid <> pg_backend_pid())
+  as remote_backend_pid
+  FROM postgres_fdw_get_connections() ORDER BY 1;
+ server_name | user_name = CURRENT_USER | valid | used_in_xact | closed | 
remote_backend_pid 
+-------------+--------------------------+-------+--------------+--------+--------------------
+ loopback    | t                        | f     | t            |        | t
+             |                          | f     | t            |        | t
 (2 rows)
 
 -- The invalid connections get closed in pgfdw_xact_callback during commit.
@@ -12436,25 +12441,34 @@ SELECT 1 FROM ft1 LIMIT 1;
 
 -- Since the remote server is still connected, "closed" should be FALSE,
 -- or NULL if the connection status check is not available.
-SELECT CASE WHEN closed IS NOT true THEN 1 ELSE 0 END
+-- In this test, the remote backend handling this connection should have
+-- application_name set to "fdw_conn_check", so remote_backend_pid should
+-- match the PID from the pg_stat_activity entry with that application_name.
+SELECT server_name,
+  CASE WHEN closed IS NOT true THEN false ELSE true END AS closed,
+  remote_backend_pid = (SELECT pid FROM pg_stat_activity
+  WHERE application_name = 'fdw_conn_check') AS remote_backend_pid
   FROM postgres_fdw_get_connections(true);
- case 
-------
-    1
+ server_name | closed | remote_backend_pid 
+-------------+--------+--------------------
+ 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.
+-- is not available. Despite the termination, remote_backend_pid should
+-- still show the non-zero PID of the terminated remote backend.
 DO $$ BEGIN
 PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity
   WHERE application_name = 'fdw_conn_check';
 END $$;
-SELECT CASE WHEN closed IS NOT false THEN 1 ELSE 0 END
+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);
- case 
-------
-    1
+ server_name | closed | remote_backend_pid 
+-------------+--------+--------------------
+ loopback    | t      | t
 (1 row)
 
 -- Clean up
diff --git a/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql 
b/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql
index 81aad4fcdaa..511a3e5c2ef 100644
--- a/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql
+++ b/contrib/postgres_fdw/postgres_fdw--1.1--1.2.sql
@@ -12,7 +12,7 @@ DROP FUNCTION postgres_fdw_get_connections ();
 CREATE FUNCTION postgres_fdw_get_connections (
     IN check_conn boolean DEFAULT false, OUT server_name text,
     OUT user_name text, OUT valid boolean, OUT used_in_xact boolean,
-    OUT closed boolean)
+    OUT closed boolean, OUT remote_backend_pid int4)
 RETURNS SETOF record
 AS 'MODULE_PATHNAME', 'postgres_fdw_get_connections_1_2'
 LANGUAGE C STRICT PARALLEL RESTRICTED;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql 
b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 1598d9e0862..d45e9f8ab52 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3410,9 +3410,14 @@ DROP SERVER loopback3 CASCADE;
 -- List all the existing cached connections. loopback and loopback3
 -- should be output as invalid connections. Also the server name and user name
 -- for loopback3 should be NULL because both server and user mapping were
--- dropped.
-SELECT server_name, user_name = CURRENT_USER as "user_name = CURRENT_USER", 
valid, used_in_xact, closed
-FROM postgres_fdw_get_connections() ORDER BY 1;
+-- dropped. In this test, the PIDs of remote backends can be gathered from
+-- pg_stat_activity, and remote_backend_pid should match one of those PIDs.
+SELECT server_name, user_name = CURRENT_USER as "user_name = CURRENT_USER",
+  valid, used_in_xact, closed,
+  remote_backend_pid = ANY(SELECT pid FROM pg_stat_activity
+  WHERE backend_type = 'client backend' AND pid <> pg_backend_pid())
+  as remote_backend_pid
+  FROM postgres_fdw_get_connections() ORDER BY 1;
 -- The invalid connections get closed in pgfdw_xact_callback during commit.
 COMMIT;
 -- All cached connections were closed while committing above xact, so no
@@ -4288,17 +4293,26 @@ SELECT 1 FROM ft1 LIMIT 1;
 
 -- Since the remote server is still connected, "closed" should be FALSE,
 -- or NULL if the connection status check is not available.
-SELECT CASE WHEN closed IS NOT true THEN 1 ELSE 0 END
+-- In this test, the remote backend handling this connection should have
+-- application_name set to "fdw_conn_check", so remote_backend_pid should
+-- match the PID from the pg_stat_activity entry with that application_name.
+SELECT server_name,
+  CASE WHEN closed IS NOT true THEN false ELSE true END AS closed,
+  remote_backend_pid = (SELECT pid FROM pg_stat_activity
+  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.
+-- is not available. Despite the termination, remote_backend_pid should
+-- still show the non-zero PID of the terminated remote backend.
 DO $$ BEGIN
 PERFORM pg_terminate_backend(pid, 180000) FROM pg_stat_activity
   WHERE application_name = 'fdw_conn_check';
 END $$;
-SELECT CASE WHEN closed IS NOT false THEN 1 ELSE 0 END
+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);
 
 -- Clean up
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index d2998c13d5d..a7f2f5ca182 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -854,7 +854,7 @@ OPTIONS (ADD password_required 'false');
     <term><function>postgres_fdw_get_connections(
       IN check_conn boolean DEFAULT false, OUT server_name text,
       OUT user_name text, OUT valid boolean, OUT used_in_xact boolean,
-      OUT closed boolean)
+      OUT closed boolean, OUT remote_backend_pid int4)
       returns setof record</function></term>
     <listitem>
      <para>
@@ -882,11 +882,11 @@ OPTIONS (ADD password_required 'false');
       Example usage of the function:
 <screen>
 postgres=# SELECT * FROM postgres_fdw_get_connections(true);
- server_name | user_name | valid | used_in_xact | closed
--------------+-----------+-------+--------------+--------
- loopback1   | postgres  | t     | t            | f
- loopback2   | public    | t     | t            | f
- loopback3   |           | f     | t            | f
+ server_name | user_name | valid | used_in_xact | closed | remote_backend_pid
+-------------+-----------+-------+--------------+-----------------------------
+ loopback1   | postgres  | t     | t            | f      |            1353340
+ loopback2   | public    | t     | t            | f      |            1353120
+ loopback3   |           | f     | t            | f      |            1353156
 </screen>
       The output columns are described in
       <xref linkend="postgres-fdw-get-connections-columns"/>.
@@ -951,6 +951,17 @@ postgres=# SELECT * FROM 
postgres_fdw_get_connections(true);
          is not available on this platform.
         </entry>
        </row>
+       <row>
+        <entry><structfield>remote_backend_pid</structfield></entry>
+        <entry><type>int4</type></entry>
+        <entry>
+         Process ID of the remote backend, on the foreign server,
+         handling the connection.  If the remote backend is terminated and
+         the connection is closed (with <literal>closed</literal> set to
+         <literal>true</literal>), this still shows the process ID of
+         the terminated backend.
+        </entry>
+       </row>
       </tbody>
      </tgroup>
     </table>
-- 
2.47.0

Reply via email to