Re: Extend postgres_fdw_get_connections to return remote backend pid

On 2025/02/19 2:06, Sagar Shedge wrote:
> Hi Fujii,
>
> On Tue, Feb 18, 2025 at 10:25 PM Fujii Masao
> <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
>>
>> I assume you're planning to extend postgres_fdw_get_connections() to
>> also return the result of PQbackendPID(entry->conn).
>> However, the patch you attached doesn't seem to include that change.
>> Did you attach the wrong patch?
>
> My bad!!
> You are right. I was going through an old discussion and attached the
> same old patch file.
>
> Please refer to the patch file to return the remote backend pid.

Thanks for the patch!

Here are my review comments:

The documentation needs to be updated.

+ OUT closed boolean, OUT remote_backend_pid INTEGER)

Naming is always tricky, but remote_backend_pid feels a bit too long.
Would remote_pid be sufficient?

* 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:

"three values" should be changed to "four values".

+ * - remote_backend_pid - return remote server backend pid

For consistency with other field comments, "return" seems unnecessary.
How about: "PID of the remote backend handling the connection" instead?

+ /*
+ * If a connection status is not closed and remote backend
+ * ID is valid, return remote backend ID. Otherwise, return NULL.
+ */
+ remote_backend_pid = PQbackendPID(entry->conn);
+ if ((is_conn_closed != 1) && (remote_backend_pid != 0))
+ values[i++] = remote_backend_pid;

Wouldn't it be better to return the result of PQbackendPID() instead of NULL
even when the connection is closed, for debugging purposes? This way,
users can see which remote backend previously handled the "closed" connection,
which might be helpful for troubleshooting.

The postgres_fdw regression test failed on my MacBook with the following diff:

--- /Users/postgres/pgsql/git/contrib/postgres_fdw/expected/postgres_fdw.out 2025-02-19 12:53:27
+++ /Users/postgres/pgsql/git/contrib/postgres_fdw/results/postgres_fdw.out 2025-02-19 17:40:04
@@ -12443,7 +12443,7 @@
FROM postgres_fdw_get_connections(true);
server_name | remote_conn_closed | remote_backend_pid
-------------+--------------------+--------------------
- loopback | false | available
+ loopback | true | available
(1 row)

-- After terminating the remote backend, since the connection is closed,
@@ -12458,7 +12458,7 @@
FROM postgres_fdw_get_connections(true);
server_name | remote_conn_closed | remote_backend_pid
-------------+--------------------+--------------------
- loopback | true | not available
+ loopback | true | available
(1 row)

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION