pg_primary_conninfo
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
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***************
*** 14098,14103 **** postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
--- 14098,14106 ----
<indexterm>
<primary>pg_last_xact_replay_timestamp</primary>
</indexterm>
+ <indexterm>
+ <primary>pg_primary_conninfo</primary>
+ </indexterm>
<para>
The functions shown in <xref
***************
*** 14168,14173 **** postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
--- 14171,14187 ----
the function returns NULL.
</entry>
</row>
+ <row>
+ <entry>
+ <literal><function>pg_primary_conninfo()</function></literal>
+ </entry>
+ <entry><type>text</type></entry>
+ <entry>Gets the connection string used to connect to the primary
+ when using streaming replication. When the server has been started
+ normally without recovery, or when file based recovery is in
+ progress, the function returns NULL.
+ </entry>
+ </row>
</tbody>
</tgroup>
</table>
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***************
*** 8829,8834 **** pg_last_xlog_replay_location(PG_FUNCTION_ARGS)
--- 8829,8857 ----
}
/*
+ * Report the connection info that the walreceiver is using to talk to the
+ * primary.
+ */
+ Datum
+ pg_primary_conninfo(PG_FUNCTION_ARGS)
+ {
+ /* use volatile pointer to prevent code rearrangement */
+ volatile WalRcvData *walrcv = WalRcv;
+ XLogRecPtr recptr;
+ char conninfo[MAXCONNINFO];
+
+ SpinLockAcquire(&walrcv->mutex);
+ recptr = walrcv->receivedUpto;
+ memcpy(conninfo, walrcv->conninfo, MAXCONNINFO);
+ SpinLockRelease(&walrcv->mutex);
+
+ if (recptr.xlogid == 0 && recptr.xrecoff == 0 && conninfo[0] != '\0')
+ PG_RETURN_NULL();
+
+ PG_RETURN_TEXT_P(cstring_to_text(conninfo));
+ }
+
+ /*
* Compute an xlog file name and decimal byte offset given a WAL location,
* such as is returned by pg_stop_backup() or pg_xlog_switch().
*
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***************
*** 3386,3392 **** DESCR("xlog filename, given an xlog location");
DATA(insert OID = 3810 ( pg_is_in_recovery PGNSP PGUID 12 1 0 0 f f f t f v 0 0 16 "" _null_ _null_ _null_ _null_ pg_is_in_recovery _null_ _null_ _null_ ));
DESCR("true if server is in recovery");
!
DATA(insert OID = 3820 ( pg_last_xlog_receive_location PGNSP PGUID 12 1 0 0 f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_last_xlog_receive_location _null_ _null_ _null_ ));
DESCR("current xlog flush location");
DATA(insert OID = 3821 ( pg_last_xlog_replay_location PGNSP PGUID 12 1 0 0 f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_last_xlog_replay_location _null_ _null_ _null_ ));
--- 3386,3393 ----
DATA(insert OID = 3810 ( pg_is_in_recovery PGNSP PGUID 12 1 0 0 f f f t f v 0 0 16 "" _null_ _null_ _null_ _null_ pg_is_in_recovery _null_ _null_ _null_ ));
DESCR("true if server is in recovery");
! DATA(insert OID = 3819 ( pg_primary_conninfo PGNSP PGUID 12 1 0 0 f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_primary_conninfo _null_ _null_ _null_ ));
! DESCR("connection string for primary");
DATA(insert OID = 3820 ( pg_last_xlog_receive_location PGNSP PGUID 12 1 0 0 f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_last_xlog_receive_location _null_ _null_ _null_ ));
DESCR("current xlog flush location");
DATA(insert OID = 3821 ( pg_last_xlog_replay_location PGNSP PGUID 12 1 0 0 f f f t f v 0 0 25 "" _null_ _null_ _null_ _null_ pg_last_xlog_replay_location _null_ _null_ _null_ ));
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
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/
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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/
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/
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
Magnus Hagander <magnus@hagander.net> writes:
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.
+1.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Wed, Dec 29, 2010 at 5:51 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
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
Simon has argued that we should allow those parameters to be set in
both recovery.conf and postgresql.conf for backward compatibility.
http://archives.postgresql.org/pgsql-hackers/2010-10/msg00017.php
So I'm thinking to make ProcessConfigFile() parse not only postgresql.conf
but also recovery.conf rather than move all the recovery parameters to
postgresql.conf.
Comments?
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
On Wed, Jan 12, 2011 at 11:52 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
So I'm thinking to make ProcessConfigFile() parse not only postgresql.conf
but also recovery.conf rather than move all the recovery parameters to
postgresql.conf.Comments?
+1.
Actually moving the settings can be done later in about 5 seconds if
we all agree it's a good idea, but let's not get bogged down in that
now.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company