Re: Logging which local address was connected to in log_line_prefix - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: Logging which local address was connected to in log_line_prefix |
Date | |
Msg-id | 3101493.1743976861@sss.pgh.pa.us Whole thread Raw |
In response to | Re: Logging which local address was connected to in log_line_prefix (Greg Sabino Mullane <htamfids@gmail.com>) |
Responses |
Re: Logging which local address was connected to in log_line_prefix
Re: Logging which local address was connected to in log_line_prefix |
List | pgsql-hackers |
Greg Sabino Mullane <htamfids@gmail.com> writes: > I have not attempted the caching change yet. After some thought I concluded that caching the local-address string in MyProcPort itself would be the most robust way of making that work. Otherwise you need some way to update the cache when MyProcPort is created (in case the process already emitted some messages before that), and the patch starts to spread into other places. I think 0004 attached is about committable, but there is one definitional point that is troubling me slightly: our choice to emit "[none]" when there's no port isn't consistent with the log_line_prefix documentation's statement that ... Some escapes are only recognized by session processes, and will be treated as empty by background processes such as the main server process. Since we've marked %L as "Session only" = yes, this implies that it should print as an empty string not "[none]". We could either 1. Ignore the inconsistency, commit 0004 as-is. 2. Change the output to be an empty string in background processes. This is consistent, but it goes against our upthread feeling that "[none]" would avoid confusion. 3. Mark %L as "Session only" = no. This seems a little weird, but it'd also be consistent. 4. Add something to the above-quoted text about %L being an exception. I don't really care for #3 or #4, but I'm ambivalent between #1 and #2. I think the worry about confusion originated when the patch would print "[local]" for either a Unix socket or a background process, and that certainly was confusing. "[local]" versus an empty string is not so ambiguous, so maybe it's fine. Thoughts? regards, tom lane diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index fea683cb49c..a8542fe41ce 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -7689,6 +7689,12 @@ local0.* /var/log/postgresql <entry>Remote host name or IP address</entry> <entry>yes</entry> </row> + <row> + <entry><literal>%L</literal></entry> + <entry>Local address (the IP address on the server that the + client connected to)</entry> + <entry>yes</entry> + </row> <row> <entry><literal>%b</literal></entry> <entry>Backend type</entry> diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index 8a6b6905079..d6299633ab7 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -67,6 +67,7 @@ #endif #include "access/xact.h" +#include "common/ip.h" #include "libpq/libpq.h" #include "libpq/pqformat.h" #include "mb/pg_wchar.h" @@ -3084,6 +3085,38 @@ log_status_format(StringInfo buf, const char *format, ErrorData *edata) appendStringInfoSpaces(buf, padding > 0 ? padding : -padding); break; + case 'L': + { + const char *local_host; + + if (MyProcPort) + { + if (MyProcPort->local_host[0] == '\0') + { + /* + * First time through: cache the lookup, since it + * might not have trivial cost. + */ + (void) pg_getnameinfo_all(&MyProcPort->laddr.addr, + MyProcPort->laddr.salen, + MyProcPort->local_host, + sizeof(MyProcPort->local_host), + NULL, 0, + NI_NUMERICHOST | NI_NUMERICSERV); + } + local_host = MyProcPort->local_host; + } + else + { + /* Background process, or connection not yet made */ + local_host = "[none]"; + } + if (padding != 0) + appendStringInfo(buf, "%*s", padding, local_host); + else + appendStringInfoString(buf, local_host); + } + break; case 'r': if (MyProcPort && MyProcPort->remote_host) { diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index ff56a1f0732..f154906c2fa 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -603,6 +603,7 @@ # %d = database name # %r = remote host and port # %h = remote host + # %L = local address # %b = backend type # %p = process ID # %P = process ID of parallel group leader diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 0d1f1838f73..d6e671a6382 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -139,6 +139,9 @@ typedef struct Port int remote_hostname_errcode; /* see above */ char *remote_port; /* text rep of remote port */ + /* local_host is filled only if needed (see log_status_format) */ + char local_host[64]; /* ip addr of local socket for client conn */ + /* * Information that needs to be saved from the startup packet and passed * into backend execution. "char *" fields are NULL if not set.
pgsql-hackers by date: