Thread: pgsql: Change replication connection log format to allow for a database

pgsql: Change replication connection log format to allow for a database

From
sriggs@postgresql.org (Simon Riggs)
Date:
Log Message:
-----------
Change replication connection log format to allow for a database
called replication. Add host and port details, following format
of messages in BackendInitialize().

Modified Files:
--------------
    pgsql/src/backend/utils/init:
        postinit.c (r1.206 -> r1.207)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/init/postinit.c?r1=1.206&r2=1.207)

Re: pgsql: Change replication connection log format to allow for a database

From
Heikki Linnakangas
Date:
Simon Riggs wrote:
> Log Message:
> -----------
> Change replication connection log format to allow for a database
> called replication. Add host and port details, following format
> of messages in BackendInitialize().

Thanks!

I'm afraid this doesn't translate well:

> --- 222,232 ----
>        */
>       if (am_walsender)
>           ereport(LOG,
> !                 (errmsg("replication connection authorized: user=%s host=%s%s%s",
> !                         port->user_name,
> !                         port->remote_host, port->remote_port[0] ? " port=" : "",
> !                         port->remote_port)));
> !
>       else if (Log_connections)
>           ereport(LOG,
>                   (errmsg("connection authorized: user=%s database=%s",

If you want display the port (I'm not sure if it's of much interest),
I'd suggest something like:

if (port->remote_port[0])
  ereport(LOG,
    (errmsg("replication connection authorized: user=%s host=%s port=%s",
            port->user_name,
            port->remote_host,
            port->remote_port[0])));
else
  ereport(LOG,
    (errmsg("replication connection authorized: user=%s host=%s",
            port->user_name,
            port->remote_host)));


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

Re: pgsql: Change replication connection log format to allow for a database

From
Heikki Linnakangas
Date:
Heikki Linnakangas wrote:
> I'm afraid this doesn't translate well:
>
>> --- 222,232 ----
>>        */
>>       if (am_walsender)
>>           ereport(LOG,
>> !                 (errmsg("replication connection authorized: user=%s host=%s%s%s",
>> !                         port->user_name,
>> !                         port->remote_host, port->remote_port[0] ? " port=" : "",
>> !                         port->remote_port)));
>> !
>>       else if (Log_connections)
>>           ereport(LOG,
>>                   (errmsg("connection authorized: user=%s database=%s",

Ok, I just realized that this was copy-pasted from BackendInitialize, so
I guess it's OK as it is...

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> Heikki Linnakangas wrote:
>> I'm afraid this doesn't translate well:
>>
>>> --- 222,232 ----
>>> */
>>> if (am_walsender)
>>> ereport(LOG,
>>> !                 (errmsg("replication connection authorized: user=%s host=%s%s%s",
>>> !                         port->user_name,
>>> !                         port->remote_host, port->remote_port[0] ? " port=" : "",
>>> !                         port->remote_port)));
>>> !
>>> else if (Log_connections)
>>> ereport(LOG,
>>> (errmsg("connection authorized: user=%s database=%s",

> Ok, I just realized that this was copy-pasted from BackendInitialize, so
> I guess it's OK as it is...

Two flat-out violations of our message style guidelines doesn't make it
right.  I agree with your proposed change for both.

            regards, tom lane

Re: pgsql: Change replication connection log format to allow for a database

From
Simon Riggs
Date:
On Thu, 2010-03-25 at 13:00 -0400, Tom Lane wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:

> > Ok, I just realized that this was copy-pasted from BackendInitialize, so
> > I guess it's OK as it is...
>
> Two flat-out violations of our message style guidelines doesn't make it
> right.  I agree with your proposed change for both.

The proposed change makes sense and I'm happy to do it.

--
 Simon Riggs           www.2ndQuadrant.com


Re: pgsql: Change replication connection log format to allow for a database

From
Alvaro Herrera
Date:
Heikki Linnakangas wrote:
> Heikki Linnakangas wrote:
> > I'm afraid this doesn't translate well:
> >
> >> --- 222,232 ----
> >>        */
> >>       if (am_walsender)
> >>           ereport(LOG,
> >> !                 (errmsg("replication connection authorized: user=%s host=%s%s%s",
> >> !                         port->user_name,
> >> !                         port->remote_host, port->remote_port[0] ? " port=" : "",
> >> !                         port->remote_port)));
> >> !
> >>       else if (Log_connections)
> >>           ereport(LOG,
> >>                   (errmsg("connection authorized: user=%s database=%s",
>
> Ok, I just realized that this was copy-pasted from BackendInitialize, so
> I guess it's OK as it is...

Actually it would be good to correct both per your previous suggestion.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.