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
