pg_primary_conninfo

Started by Magnus Haganderover 15 years ago23 messageshackers
Jump to latest
#1Magnus Hagander
magnus@hagander.net

Attached patch implements a function called pg_primary_conninfo() that
returns, well, the primary_conninfo used on the standby when in
streaming replication mode (otherwise NULL).

Objections?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Attachments:

pg_primary_conninfo.patchtext/x-patch; charset=US-ASCII; name=pg_primary_conninfo.patchDownload+40-3
#2Robert Haas
robertmhaas@gmail.com
In reply to: Magnus Hagander (#1)
Re: pg_primary_conninfo

On Tue, Dec 28, 2010 at 8:31 AM, Magnus Hagander <magnus@hagander.net> wrote:

Attached patch implements a function called pg_primary_conninfo() that
returns, well, the primary_conninfo used on the standby when in
streaming replication mode (otherwise NULL).

+1. Let's make sure to explicitly document what this function returns
when recovery was previous in progress, but we are now in normal
running.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3Magnus Hagander
magnus@hagander.net
In reply to: Robert Haas (#2)
Re: pg_primary_conninfo

On Tue, Dec 28, 2010 at 14:38, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Dec 28, 2010 at 8:31 AM, Magnus Hagander <magnus@hagander.net> wrote:

Attached patch implements a function called pg_primary_conninfo() that
returns, well, the primary_conninfo used on the standby when in
streaming replication mode (otherwise NULL).

+1.  Let's make sure to explicitly document what this function returns
when recovery was previous in progress, but we are now in normal
running.

Oh, didn't think of that scenario.

Is that intended behaviour though? I tend to think that it is (since
you can check with pg_is_in_recovery) as long as it's documented, but
might it make more sense to have it return NULL in this case?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#1)
Re: pg_primary_conninfo

Magnus Hagander <magnus@hagander.net> writes:

Attached patch implements a function called pg_primary_conninfo() that
returns, well, the primary_conninfo used on the standby when in
streaming replication mode (otherwise NULL).

Objections?

What's the use case? And aren't there security reasons to NOT expose
that? It might contain a password for instance.

+ 	if (recptr.xlogid == 0 && recptr.xrecoff == 0 && conninfo[0] != '\0')
+ 		PG_RETURN_NULL();

This test seems a bit incoherent.

regards, tom lane

#5Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#4)
Re: pg_primary_conninfo

On Dec 28, 2010 3:58 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

Attached patch implements a function called pg_primary_conninfo() that
returns, well, the primary_conninfo used on the standby when in
streaming replication mode (otherwise NULL).

Objections?

What's the use case? And aren't there security reasons to NOT expose
that? It might contain a password for instance.

Good point - should be made superuser only.

+ if (recptr.xlogid == 0 && recptr.xrecoff == 0 && conninfo[0] !=

'\0')

+ PG_RETURN_NULL();

This test seems a bit incoherent.

I used that to test that streaming repl is enabled at all. Is there a better
way?

/Magnus

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#5)
Re: pg_primary_conninfo

Magnus Hagander <magnus@hagander.net> writes:

On Dec 28, 2010 3:58 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

What's the use case? And aren't there security reasons to NOT expose
that? It might contain a password for instance.

Good point - should be made superuser only.

I'm still wondering what's the actual use-case for exposing this inside
SQL. Those with a legitimate need-to-know can look at the slave
server's config files, no?

regards, tom lane

#7Guillaume Lelarge
guillaume@lelarge.info
In reply to: Tom Lane (#6)
Re: pg_primary_conninfo

Le 28/12/2010 16:34, Tom Lane a �crit :

Magnus Hagander <magnus@hagander.net> writes:

On Dec 28, 2010 3:58 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

What's the use case? And aren't there security reasons to NOT expose
that? It might contain a password for instance.

Good point - should be made superuser only.

I'm still wondering what's the actual use-case for exposing this inside
SQL. Those with a legitimate need-to-know can look at the slave
server's config files, no?

This is something I wanted to have in 9.0 when I coded in pgAdmin some
features related to the HotStandby. Knowing on which IP is the master
can help pgAdmin offer the user to register the master node.

It's also interesting to get lag between master and slave. As soon as
I'm connected to a slave, I can connect to the master and get the lag
between them. Something I can't do right now in pgAdmin.

--
Guillaume
http://www.postgresql.fr
http://dalibo.com

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Guillaume Lelarge (#7)
Re: pg_primary_conninfo

Guillaume Lelarge <guillaume@lelarge.info> writes:

Le 28/12/2010 16:34, Tom Lane a �crit :

I'm still wondering what's the actual use-case for exposing this inside
SQL. Those with a legitimate need-to-know can look at the slave
server's config files, no?

This is something I wanted to have in 9.0 when I coded in pgAdmin some
features related to the HotStandby. Knowing on which IP is the master
can help pgAdmin offer the user to register the master node.

It's also interesting to get lag between master and slave. As soon as
I'm connected to a slave, I can connect to the master and get the lag
between them. Something I can't do right now in pgAdmin.

The proposed primary_conninfo seems like a pretty awful solution to
those problems, though.

1. It'll have to be restricted to superusers, therefore ordinary
users on the slave can't actually make use of it.

2. It's not what you want, since you don't want to connect as the
replication user. Therefore, you'd have to start by parsing out
the parts you do need. Expecting every client to include conninfo
parsing logic doesn't seem cool to me.

I can see the point of, say, a primary_host_address() function returning
inet, which would be way better on both those dimensions than the
current proposal. But I'm not sure what else would be needed.

regards, tom lane

#9Guillaume Lelarge
guillaume@lelarge.info
In reply to: Tom Lane (#8)
Re: pg_primary_conninfo

Le 28/12/2010 17:36, Tom Lane a �crit :

Guillaume Lelarge <guillaume@lelarge.info> writes:

Le 28/12/2010 16:34, Tom Lane a �crit :

I'm still wondering what's the actual use-case for exposing this inside
SQL. Those with a legitimate need-to-know can look at the slave
server's config files, no?

This is something I wanted to have in 9.0 when I coded in pgAdmin some
features related to the HotStandby. Knowing on which IP is the master
can help pgAdmin offer the user to register the master node.

It's also interesting to get lag between master and slave. As soon as
I'm connected to a slave, I can connect to the master and get the lag
between them. Something I can't do right now in pgAdmin.

The proposed primary_conninfo seems like a pretty awful solution to
those problems, though.

I would say "not the best one, but better than what I have now" :)

1. It'll have to be restricted to superusers, therefore ordinary
users on the slave can't actually make use of it.

pgAdmin's users usually connect as superusers.

2. It's not what you want, since you don't want to connect as the
replication user. Therefore, you'd have to start by parsing out
the parts you do need. Expecting every client to include conninfo
parsing logic doesn't seem cool to me.

I can see the point of, say, a primary_host_address() function returning
inet, which would be way better on both those dimensions than the
current proposal. But I'm not sure what else would be needed.

Yeah, it would be better that way. I'm actually interested in Magnus's
patch because, during 9.0 development phase, I had in mind to parse the
primary_conninfo till I found I could not get this value with SHOW or
current_setting().

But, actually, what I really need is host and port. This way, I could
connect to the master node, with the same user and password that was
used on the slave node.

--
Guillaume
http://www.postgresql.fr
http://dalibo.com

#10Gurjeet Singh
singh.gurjeet@gmail.com
In reply to: Tom Lane (#8)
Re: pg_primary_conninfo

On Tue, Dec 28, 2010 at 11:36 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I can see the point of, say, a primary_host_address() function returning
inet, which would be way better on both those dimensions than the
current proposal. But I'm not sure what else would be needed.

+1, since it bypasses security risks associated with exposing
username/password.

Ability to see port number will be a useful addition.

Another case to consider is what if slave is connected to a local server
over unix-domain sockets? Returning NULL might make it ambiguous with the
case where the instance has been promoted out of standby.

Regards,
--
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.EnterpriseDB.com

singh.gurjeet@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet

Mail sent from my BlackLaptop device

#11Guillaume Lelarge
guillaume@lelarge.info
In reply to: Gurjeet Singh (#10)
Re: pg_primary_conninfo

Le 28/12/2010 17:50, Gurjeet Singh a �crit :

On Tue, Dec 28, 2010 at 11:36 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I can see the point of, say, a primary_host_address() function returning
inet, which would be way better on both those dimensions than the
current proposal. But I'm not sure what else would be needed.

+1, since it bypasses security risks associated with exposing
username/password.

Ability to see port number will be a useful addition.

Another case to consider is what if slave is connected to a local server
over unix-domain sockets? Returning NULL might make it ambiguous with the
case where the instance has been promoted out of standby.

The host should be the socket file path.

--
Guillaume
http://www.postgresql.fr
http://dalibo.com

#12Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#6)
Re: pg_primary_conninfo

On Dec 28, 2010, at 10:34 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm still wondering what's the actual use-case for exposing this inside
SQL. Those with a legitimate need-to-know can look at the slave
server's config files, no?

SQL access is frequently more convenient, though. Although maybe now that we've made recovery.conf use the GUC lexer we oughta continue in that vein and expose those parameters as PGC_INTERNAL GUCs rather than inventing a new function for it...

...Robert

#13Guillaume Lelarge
guillaume@lelarge.info
In reply to: Robert Haas (#12)
Re: pg_primary_conninfo

Le 28/12/2010 18:12, Robert Haas a �crit :

On Dec 28, 2010, at 10:34 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm still wondering what's the actual use-case for exposing this inside
SQL. Those with a legitimate need-to-know can look at the slave
server's config files, no?

SQL access is frequently more convenient, though. Although maybe now that we've made recovery.conf use the GUC lexer we oughta continue in that vein and expose those parameters as PGC_INTERNAL GUCs rather than inventing a new function for it...

That was the first thing I wanted. Knowing the trigger file for example
would be quite useful for pgAdmin and pgPool for example.

--
Guillaume
http://www.postgresql.fr
http://dalibo.com

#14Gurjeet Singh
singh.gurjeet@gmail.com
In reply to: Robert Haas (#12)
Re: pg_primary_conninfo

On Tue, Dec 28, 2010 at 12:12 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Dec 28, 2010, at 10:34 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm still wondering what's the actual use-case for exposing this inside
SQL. Those with a legitimate need-to-know can look at the slave
server's config files, no?

SQL access is frequently more convenient, though. Although maybe now that
we've made recovery.conf use the GUC lexer we oughta continue in that vein
and expose those parameters as PGC_INTERNAL GUCs rather than inventing a new
function for it...

+1 for SQL access, but exposing it via pg_settings opens up the security
problem as there might be sensitive info in those GUCs.

Regards,
--
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.EnterpriseDB.com

singh.gurjeet@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet

Mail sent from my BlackLaptop device

#15Tom Lane
tgl@sss.pgh.pa.us
In reply to: Gurjeet Singh (#14)
Re: pg_primary_conninfo

Gurjeet Singh <singh.gurjeet@gmail.com> writes:

On Tue, Dec 28, 2010 at 12:12 PM, Robert Haas <robertmhaas@gmail.com> wrote:

SQL access is frequently more convenient, though. Although maybe now that
we've made recovery.conf use the GUC lexer we oughta continue in that vein
and expose those parameters as PGC_INTERNAL GUCs rather than inventing a new
function for it...

+1 for SQL access, but exposing it via pg_settings opens up the security
problem as there might be sensitive info in those GUCs.

IIRC we do have a GUC property that hides the value from non-superusers,
so we could easily have a GUC that is equivalent to the proposed
pg_primary_conninfo function. Of course this does nothing for my
objections to the function. Also, I'm not sure how we'd deal with the
state-dependency aspect of it (ie, value changes once you exit recovery
mode).

regards, tom lane

#16Guillaume Lelarge
guillaume@lelarge.info
In reply to: Tom Lane (#15)
Re: pg_primary_conninfo

Le 28/12/2010 19:30, Tom Lane a �crit :

Gurjeet Singh <singh.gurjeet@gmail.com> writes:

On Tue, Dec 28, 2010 at 12:12 PM, Robert Haas <robertmhaas@gmail.com> wrote:

SQL access is frequently more convenient, though. Although maybe now that
we've made recovery.conf use the GUC lexer we oughta continue in that vein
and expose those parameters as PGC_INTERNAL GUCs rather than inventing a new
function for it...

+1 for SQL access, but exposing it via pg_settings opens up the security
problem as there might be sensitive info in those GUCs.

IIRC we do have a GUC property that hides the value from non-superusers,
so we could easily have a GUC that is equivalent to the proposed
pg_primary_conninfo function. Of course this does nothing for my
objections to the function. Also, I'm not sure how we'd deal with the
state-dependency aspect of it (ie, value changes once you exit recovery
mode).

We already have superuser GUC.

b1=> show data_directory;
ERROR: must be superuser to examine "data_directory"

We only need to do the same for primary_conninfo and trigger_file (as I
remember it, there are the only ones needing this).

--
Guillaume
http://www.postgresql.fr
http://dalibo.com

#17Gurjeet Singh
singh.gurjeet@gmail.com
In reply to: Tom Lane (#15)
Re: pg_primary_conninfo

On Tue, Dec 28, 2010 at 1:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Gurjeet Singh <singh.gurjeet@gmail.com> writes:

On Tue, Dec 28, 2010 at 12:12 PM, Robert Haas <robertmhaas@gmail.com>

wrote:

SQL access is frequently more convenient, though. Although maybe now

that

we've made recovery.conf use the GUC lexer we oughta continue in that

vein

and expose those parameters as PGC_INTERNAL GUCs rather than inventing a

new

function for it...

+1 for SQL access, but exposing it via pg_settings opens up the security
problem as there might be sensitive info in those GUCs.

IIRC we do have a GUC property that hides the value from non-superusers,
so we could easily have a GUC that is equivalent to the proposed
pg_primary_conninfo function. Of course this does nothing for my
objections to the function. Also, I'm not sure how we'd deal with the
state-dependency aspect of it (ie, value changes once you exit recovery
mode).

I would vote for making host:port part visible to non-superusers. This info
is definitely usable in combination with pg_current_xlog_location() and
pg_last_xlog_receive_location() to allow non-superusers to monitor streaming
replication.

Given that primary_conninfo is already parsed by libpq, how difficult would
it be to extract and store/display those host:port components.

Regards,
--
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.EnterpriseDB.com

singh.gurjeet@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet

Mail sent from my BlackLaptop device

#18Magnus Hagander
magnus@hagander.net
In reply to: Guillaume Lelarge (#9)
Re: pg_primary_conninfo

On Tue, Dec 28, 2010 at 17:43, Guillaume Lelarge <guillaume@lelarge.info> wrote:

Le 28/12/2010 17:36, Tom Lane a écrit :

Guillaume Lelarge <guillaume@lelarge.info> writes:

Le 28/12/2010 16:34, Tom Lane a écrit :

1. It'll have to be restricted to superusers, therefore ordinary
users on the slave can't actually make use of it.

pgAdmin's users usually connect as superusers.

It would be a function for DBAs, of course. I don't see why "normal
users" would be intersted in it, really.

2. It's not what you want, since you don't want to connect as the
replication user.  Therefore, you'd have to start by parsing out
the parts you do need.  Expecting every client to include conninfo
parsing logic doesn't seem cool to me.

I can see the point of, say, a primary_host_address() function returning
inet, which would be way better on both those dimensions than the
current proposal.  But I'm not sure what else would be needed.

Yeah, it would be better that way. I'm actually interested in Magnus's
patch because, during 9.0 development phase, I had in mind to parse the
primary_conninfo till I found I could not get this value with SHOW or
current_setting().

But, actually, what I really need is host and port. This way, I could
connect to the master node, with the same user and password that was
used on the slave node.

I agree it might well be more useful to have it split up for us. We'd
need the host name (though it would have to be text and not inet,
since we'd need the unix socket path for a local connection) and port.
And username. But certainly not password, and probably none of the
other parameters.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#19Magnus Hagander
magnus@hagander.net
In reply to: Robert Haas (#12)
Re: pg_primary_conninfo

On Tue, Dec 28, 2010 at 18:12, Robert Haas <robertmhaas@gmail.com> wrote:

On Dec 28, 2010, at 10:34 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I'm still wondering what's the actual use-case for exposing this inside
SQL.  Those with a legitimate need-to-know can look at the slave
server's config files, no?

SQL access is frequently more convenient, though.

Yes. Reading it in the files does not scale with $LOTS of servers, be
them slaves or masters or both. You can't assume that people have
direct filesystem access to the server (or at least it's data
directory) - particularly when the organisation is large enough that
you have different teams running the db's and the OS's, not to mention
when you have some on-call group who verifies the things in the middle
of the night...

Unless you mean reading them with pg_read_file() and then parsing it
manually, but that just requires everybody to re-invent the wheel we
already have in the parser.

 Although maybe now that we've made recovery.conf use the GUC lexer we oughta continue in that vein and expose those parameters as PGC_INTERNAL GUCs rather than inventing a new function for it...

That's definitely another option that I wouldn't object to if people
prefer that way.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

#20Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Magnus Hagander (#19)
Re: pg_primary_conninfo

On 29.12.2010 10:36, Magnus Hagander wrote:

On Tue, Dec 28, 2010 at 18:12, Robert Haas<robertmhaas@gmail.com> wrote:

Although maybe now that we've made recovery.conf use the GUC lexer we oughta continue in that vein and expose those parameters as PGC_INTERNAL GUCs rather than inventing a new function for it...

That's definitely another option that I wouldn't object to if people
prefer that way.

I recall from previous discussions that we have a consensus that we
should unite recovery.conf and postgresql.conf, so that they're all GUCs
and you can put all the settings in postgresql.conf. Let's do that.

http://archives.postgresql.org/pgsql-hackers/2010-10/msg00033.php

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

#21Dimitri Fontaine
dimitri@2ndQuadrant.fr
In reply to: Magnus Hagander (#19)
#22Fujii Masao
masao.fujii@gmail.com
In reply to: Heikki Linnakangas (#20)
#23Robert Haas
robertmhaas@gmail.com
In reply to: Fujii Masao (#22)