Thread: Re: [COMMITTERS] 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
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
Re: Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.
From
Fujii Masao
Date:
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
Re: Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.
From
Simon Riggs
Date:
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
Re: Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.
From
Fujii Masao
Date:
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
Re: Re: [COMMITTERS] pgsql: Add connection messages for streaming replication.
From
Craig Ringer
Date:
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