Re: [PATCH] Include application_name in "connection authorized" log message

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Don Seiler <don(at)seiler(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Include application_name in "connection authorized" log message
Date: 2018-09-24 21:10:30
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Greetings Don!

* Don Seiler (don(at)seiler(dot)us) wrote:
> On Tue, Aug 7, 2018 at 12:32 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Don Seiler <don(at)seiler(dot)us> writes:
> >
> > > 1. We want to make a generic, central ascii-lobotomizing function similar
> > > to check_application_name that we can re-use there and for other checks
> > (eg
> > > user name).
> > > 2. Change check_application_name to call this function (or just call this
> > > function instead of check_application_name()?)
> >
> > check_application_name's API is dictated by the GUC check-hook interface,
> > and doesn't really make sense for this other use. So the first part of
> > that, not the second.
> >
> > > 3. Call this function when storing the value in the port struct.
> >
> > I'm not sure where exactly is the most sensible place to call it,
> > but trying to minimize the number of places that know about this
> > kluge seems like a good principle.
> OK I created a new function called clean_ascii() in common/string.c. I call
> this from my new logic in postmaster.c as well as replacing the logic in
> guc.c's check_application_name() and check_cluster_name().

Since we're putting it into common/string.c (which seems pretty
reasonable to me, at least), I went ahead and changed it to be
'pg_clean_ascii'. I didn't see any other obvious cases where we could
use this function (though typecmds.c does have an interesting ASCII
check for type categories..).

Otherwise, I added some comments, added application_name to the
replication 'connection authorized' messages (seems like we really
should be consistent across all of them...), ran it through pgindent,
and updated a variable name or two here and there.

> I've been fighting my own confusion with git and rebasing and fighting the
> same conflicts over and over and over, but this patch should be what I
> want. If anyone has time to review my git process, I would appreciate it. I
> must be doing something wrong to have these same conflicts every time I
> rebase (or I completely misunderstand what it actually does).

I'd be happy to chat about it sometime, of course, just have to find
time when we both have a free moment. :)

Attached is the updated patch. If you get a chance to look over it
again and make sure it looks good to you, that'd be great. I did a bit
of testing of it myself but wouldn't complain if someone else wanted to

One thing I noticed while testing is that our 'disconnection' message
still emits 'database=' for replication connections even though the
'connection authorized' message doesn't (presumably because we realized
it's a bit silly to do so when we say 'replication connection'...).
Seems like it'd be nice to have the log_connection / log_disconnection
messages have some consistency about them but that's really a different
discussion from this.



Attachment Content-Type Size
add_appname_to_authmsg_v2.patch text/x-diff 8.6 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2018-09-24 21:11:17 Re: pgsql: Improve autovacuum logging for aggressive and anti-wraparound ru
Previous Message Sergei Kornilov 2018-09-24 20:55:41 Re: pgsql: Improve autovacuum logging for aggressive and anti-wraparound ru