Re: Enhance pg_stat_wal_receiver view to display connected host

On Wed, Mar 28, 2018 at 3:09 PM, Michael Paquier <michael@paquier.xyz> wrote:
> On Wed, Mar 28, 2018 at 03:41:33PM +1100, Haribabu Kommi wrote:
>> On Wed, Mar 28, 2018 at 12:54 PM, Michael Paquier <michael@paquier.xyz>
>> wrote:
>> Updated patch attached.

Thanks for the patch! I'd like to commit this feature for v11.

@@ -753,4 +753,6 @@ CREATE VIEW pg_stat_wal_receiver AS
             s.latest_end_time,
             s.slot_name,
+            s.remote_server,
+            s.remote_port,

As the column names, aren't sender_host and sender_port more intuitive
rather than remote_server and remote_port?

+ ret = PQhost(conn->streamConn);
+ if (ret)
+ *remote_server = pstrdup(ret);

When the connection has an error, PQhost() and PQport() return an empty string.
In this case, pg_stat_wal_receiver reports an empty string in remote_server and
NULL in remote_port. Which looks inconsistent to me. In that case, both of them
should be reported NULL, I think. So I think that the above "if (ret)" condition
should be "if (ret & strcmp(ret, "") == 0)". Thought?

Of course, currently it's basically impossible that PQhost() and PQport() return
an empty string in libpqrcv_get_remoteserver_info() because it's called just
after the replication connection is successfully established. But it's better to
handle also that case for robustness of the code.

Regards,

-- 
Fujii Masao