Thread: Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.

Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.

From
Fujii Masao
Date:
On Sat, Mar 20, 2010 at 4:19 AM, Simon Riggs <sriggs@postgresql.org> wrote:
> Log Message:
> -----------
> Add connection messages for streaming replication. log_connections
> was broken for a replication connection and no messages were
> displayed on either standby or primary, at any debug level.
> Connection messages needed to diagnose session drop/reconnect
> events. Use LOG mode for now, discuss lowering in later releases.

LOG:  connection authorized: user=foo database=replication

Currently, when the primary accepts the connection from the standby,
it emits the above message. But "database=replication" is not accurate
because no "database" is supplied by the standby unless it's explicitly
specified in primary_conninfo parameter. So, how about changing the
message as follow?

LOG:  replication connection authorized: user=foo

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.

From
Simon Riggs
Date:
On Wed, 2010-03-24 at 10:52 +0900, Fujii Masao wrote:
> On Sat, Mar 20, 2010 at 4:19 AM, Simon Riggs <sriggs@postgresql.org> wrote:
> > Log Message:
> > -----------
> > Add connection messages for streaming replication. log_connections
> > was broken for a replication connection and no messages were
> > displayed on either standby or primary, at any debug level.
> > Connection messages needed to diagnose session drop/reconnect
> > events. Use LOG mode for now, discuss lowering in later releases.
> 
> LOG:  connection authorized: user=foo database=replication
> 
> Currently, when the primary accepts the connection from the standby,
> it emits the above message. But "database=replication" is not accurate
> because no "database" is supplied by the standby unless it's explicitly
> specified in primary_conninfo parameter. So, how about changing the
> message as follow?
> 
> LOG:  replication connection authorized: user=foo

The main thing for me was that it logged something. The above two ways
occurred to me and figured we'd end up discussing it.

The first way is slightly confusing for the reason stated, agreed. By
using the same form of words as is used currently, all existing scripts
that search for connection details will all still work. The second way
is more informative, if you don't know "replication" is a
pseudo-database, but it will break all existing scripts.

My own feeling was that breaking existing scripts was not a price worth
paying for the extra information in the second form of the message,
since its just the same words re-arranged.

-- Simon Riggs           www.2ndQuadrant.com



Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.

From
Fujii Masao
Date:
On Wed, Mar 24, 2010 at 2:25 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> The main thing for me was that it logged something. The above two ways
> occurred to me and figured we'd end up discussing it.
>
> The first way is slightly confusing for the reason stated, agreed. By
> using the same form of words as is used currently, all existing scripts
> that search for connection details will all still work. The second way
> is more informative, if you don't know "replication" is a
> pseudo-database, but it will break all existing scripts.
>
> My own feeling was that breaking existing scripts was not a price worth
> paying for the extra information in the second form of the message,
> since its just the same words re-arranged.

Since the meaning of the first message is different between 8.4 and 9.0
(in 8.4, the normal connection to the database 'replication', in 9.0, the
connection for replication from the standby server), we would still need
to change the existing scripts. No?

What is worse is that we can connect to the real database 'replication'
in 9.0. So we might be unable to discern that normal connection from the
replication connection by seeing the first message.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.

From
Simon Riggs
Date:
On Wed, 2010-03-24 at 17:36 +0900, Fujii Masao wrote:
> On Wed, Mar 24, 2010 at 2:25 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> > The main thing for me was that it logged something. The above two ways
> > occurred to me and figured we'd end up discussing it.
> >
> > The first way is slightly confusing for the reason stated, agreed. By
> > using the same form of words as is used currently, all existing scripts
> > that search for connection details will all still work. The second way
> > is more informative, if you don't know "replication" is a
> > pseudo-database, but it will break all existing scripts.
> >
> > My own feeling was that breaking existing scripts was not a price worth
> > paying for the extra information in the second form of the message,
> > since its just the same words re-arranged.
> 
> Since the meaning of the first message is different between 8.4 and 9.0
> (in 8.4, the normal connection to the database 'replication', in 9.0, the
> connection for replication from the standby server), we would still need
> to change the existing scripts. No?
> 
> What is worse is that we can connect to the real database 'replication'
> in 9.0. So we might be unable to discern that normal connection from the
> replication connection by seeing the first message.

So we are allowing a database to be called "REPLICATION"? Surely there
are some significant problems in that case. How will access control to
that database work in the pg_hba.conf?

-- Simon Riggs           www.2ndQuadrant.com



Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.

From
Fujii Masao
Date:
On Wed, Mar 24, 2010 at 7:29 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> So we are allowing a database to be called "REPLICATION"?

Yes.

> Surely there
> are some significant problems in that case. How will access control to
> that database work in the pg_hba.conf?

We can do that by enclosing the database field of pg_hba.conf in double
quotes as follows.
   TYPE   DATABASE        USER  CIDR-ADDRESS  METHOD   host   "replication"   foo   192.168.0.5   md5

In pg_hba.conf, double-quoted keyword like "all", "sameuser", "samerole"
or "replication" matches a database with that name.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.

From
Simon Riggs
Date:
On Wed, 2010-03-24 at 19:49 +0900, Fujii Masao wrote:
> On Wed, Mar 24, 2010 at 7:29 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> > So we are allowing a database to be called "REPLICATION"?
> 
> Yes.
> 
> > Surely there
> > are some significant problems in that case. How will access control to
> > that database work in the pg_hba.conf?
> 
> We can do that by enclosing the database field of pg_hba.conf in double
> quotes as follows.
> 
>     TYPE   DATABASE        USER  CIDR-ADDRESS  METHOD
>     host   "replication"   foo   192.168.0.5   md5
> 
> In pg_hba.conf, double-quoted keyword like "all", "sameuser", "samerole"
> or "replication" matches a database with that name.

So we might have a pg_hba.conf that looks like this

TYPE   DATABASE        USER  CIDR-ADDRESS  METHOD
host   "replication"   foo   192.168.0.5   md5
host   replication     foo   192.168.0.5   md5

Which looks pretty strange.
I think we should change that, though if not we should at least document
it.

That probably tips the balance towards having the alternate wording:
LOG:  replication connection authorized: user=foo



It also highlights another problem: it's possible to have database names
that contain spaces. There are no double quotes around most of the
things that get logged. So if we do

CREATE DATABASE "oh my god";

will cause things like this to be logged without quotes

LOG:  connection authorized: user=foo database=oh my god

-- Simon Riggs           www.2ndQuadrant.com



Re: Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.

From
Heikki Linnakangas
Date:
Simon Riggs wrote:
> So we might have a pg_hba.conf that looks like this
> 
> TYPE   DATABASE        USER  CIDR-ADDRESS  METHOD
> host   "replication"   foo   192.168.0.5   md5
> host   replication     foo   192.168.0.5   md5
> 
> Which looks pretty strange.
> I think we should change that, though if not we should at least document
> it.

The default pg_hba.conf says:

> # Database and user names containing spaces, commas, quotes and other
> # special characters must be quoted.  Quoting one of the keywords
> # "all", "sameuser", "samerole" or "replication" makes the name lose
> # its special character, and just match a database or username with
> # that name.

but I don't see any mention of that in the docs. How about:

*** client-auth.sgml    24 Mar 2010 09:44:06 +0200    1.134
--- client-auth.sgml    24 Mar 2010 13:21:16 +0200    
***************
*** 77,84 ****    a set of records, one per line. Blank lines are ignored, as is any    text after the
<literal>#</literal>comment character. A record is made    up of a number of fields which are separated by spaces
and/ortabs.
 
!    Fields can contain white space if the field value is quoted. Records
!    cannot be continued across lines.   </para>
   <para>
--- 77,87 ----    a set of records, one per line. Blank lines are ignored, as is any    text after the
<literal>#</literal>comment character. A record is made    up of a number of fields which are separated by spaces
and/ortabs.
 
!    Fields can contain white space if the field value is quoted.
!    Quoting one of the keywords in database or username field (e.g "all"
!    or "replication") makes the name lose its special character, and just
!    match a database or username with that name. Records cannot be
!    continued across lines.   </para>
   <para>


> That probably tips the balance towards having the alternate wording:
> LOG:  replication connection authorized: user=foo

+1

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


On Wed, Mar 24, 2010 at 8:22 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> but I don't see any mention of that in the docs. How about:
>
> *** client-auth.sgml    24 Mar 2010 09:44:06 +0200      1.134
> --- client-auth.sgml    24 Mar 2010 13:21:16 +0200
> ***************
> *** 77,84 ****
>     a set of records, one per line. Blank lines are ignored, as is any
>     text after the <literal>#</literal> comment character. A record is made
>     up of a number of fields which are separated by spaces and/or tabs.
> !    Fields can contain white space if the field value is quoted. Records
> !    cannot be continued across lines.
>    </para>
>
>    <para>
> --- 77,87 ----
>     a set of records, one per line. Blank lines are ignored, as is any
>     text after the <literal>#</literal> comment character. A record is made
>     up of a number of fields which are separated by spaces and/or tabs.
> !    Fields can contain white space if the field value is quoted.
> !    Quoting one of the keywords in database or username field (e.g "all"
> !    or "replication") makes the name lose its special character, and just
> !    match a database or username with that name. Records cannot be
> !    continued across lines.
>    </para>

+1

>> That probably tips the balance towards having the alternate wording:
>> LOG:  replication connection authorized: user=foo
>
> +1

+1

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


On Wed, 2010-03-24 at 20:30 +0900, Fujii Masao wrote:
> On Wed, Mar 24, 2010 at 8:22 PM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com> wrote:
> > but I don't see any mention of that in the docs. How about:

> +1

Yes, plus a mention in the rep docs.

> >> That probably tips the balance towards having the alternate wording:
> >> LOG:  replication connection authorized: user=foo
> >
> > +1
> 
> +1

Will do.


What's the word on when you guys will be finished with the open items
list for SR? Feels like we're both using the other one as an excuse not
to complete our remaining actions.

-- Simon Riggs           www.2ndQuadrant.com



On Wed, Mar 24, 2010 at 8:52 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> What's the word on when you guys will be finished with the open items
> list for SR?

Sorry, I'm not sure when.

Now, I'm trying to address the open item "Walreceiver is not
interruptible on win32".
It might take time to create the patch since I'm not familiar with
tweaking Makefile
or something.
http://archives.postgresql.org/pgsql-hackers/2010-03/msg00413.php

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


On 24/03/2010 6:29 PM, Simon Riggs wrote:

> So we are allowing a database to be called "REPLICATION"? Surely there
> are some significant problems in that case. How will access control to
> that database work in the pg_hba.conf?

Surely it should be consistent with "template0" and "postgres":

template1=# create database postgres;
ERROR:  database "postgres" already exists
template1=# create database "postgres";
ERROR:  database "postgres" already exists
template1=# create database "POSTGRES";
CREATE DATABASE
template1=# create database template0;
ERROR:  database "template0" already exists
template1=# create database "template0";
ERROR:  database "template0" already exists
template1=# create database "TEMPLATE0";
CREATE DATABASE

--
Craig Ringer