Re: Extend postgres_fdw_get_connections to return remote backend pid

On 2025/02/21 0:54, Sagar Shedge wrote:
> Hi Fujii,
>
>> Naming is always tricky, but remote_backend_pid feels a bit too long.
> Would remote_pid be sufficient?
> Point looks valid. I had another perspective is to align the naming
> convention to pg_backend_pid(). remote_pid is not helping to identify
> whether pid belongs to postgres backend or not. Does this make sense?
> Or I'm fine to go with concise name like `remote_pid`

I initially thought "remote_pid" was sufficient since the postgres_fdw
connection clearly corresponds to a backend process. However, I'm fine
with keeping "remote_backend_pid" as the column name for now. If we find
a better name later, we can rename it.

>> How about: "PID of the remote backend handling the connection" instead?
> Updated in v2.

Thanks for updating the patch!

You still need to update the documentation. Could you add descriptions
for postgres_fdw_get_connections()?

>> 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.
> Agree. Updated logic to return backend pid always except when pid is 0
> which indicates no backend attached to session. Returning 0 found
> misleading. What's your thought on this?

Your approach makes sense to me.

>> The postgres_fdw regression test failed on my MacBook with the following diff:
> I updated tests to make it more stable. Let me know if it's still
> failing on your setup.

Yes, the regression test passed successfully on my machine.

--- dropped.
-SELECT server_name, user_name = CURRENT_USER as "user_name = CURRENT_USER", valid, used_in_xact, closed
+-- dropped. remote_backend_pid will continue to return available as it fetch remote
+-- server backend pid from cached connections.
+SELECT server_name, user_name = CURRENT_USER as "user_name = CURRENT_USER", valid, used_in_xact, closed,
+CASE WHEN remote_backend_pid IS NOT NULL then 'available' ELSE 'not available' END AS remote_backend_pid

Instead of checking whether remote_backend_pid is NOT NULL, how about
verifying that it actually matches a remote backend's PID? For example:

remote_backend_pid = ANY(SELECT pid FROM pg_stat_activity WHERE backend_type = 'client backend' AND pid <> pg_backend_pid()) AS "remote_backend_pid = remote pg_stat_activity.pid"

-SELECT CASE WHEN closed IS NOT true THEN 1 ELSE 0 END
+SELECT server_name, CASE WHEN closed IS NOT true THEN 'false' ELSE 'true' END AS remote_conn_closed,
+CASE WHEN remote_backend_pid IS NOT NULL then 'available' ELSE 'not available' END AS remote_backend_pid

Similarly, instead of checking if remote_backend_pid is NOT NULL,
how about verifying it against pg_stat_activity?

remote_backend_pid = (SELECT pid FROM pg_stat_activity WHERE application_name = 'fdw_conn_check')

Regards,

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