pgsql: Add connection messages for streaming replication.

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

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

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

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

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

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

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/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>

<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