Changing recovery.conf parameters into GUCs

Started by Simon Riggsabout 13 years ago19 messageshackers
Jump to latest
#1Simon Riggs
simon@2ndQuadrant.com

What we want to do is make recovery parameters into GUCs, allowing
them to be reset by SIGHUP and also to allow all users to see the
parameters in use, from any session.

The existing mechanism for recovery is that
1. we put parameters in a file called recovery.conf
2. we use the existence of a recovery.conf file to trigger archive
recovery/replication

I also wish to see backwards compatibility maintained, so am proposing
the following:

a) recovery parameters are made into GUCs (for which we have a patch
from Fujii)
b) all processes automatically read recovery.conf as the last step in
reading configuration files, if it exists, even if data_directory
parameter is in use (attached patch)
c) we trigger archive recovery by the presence of either
recovery.conf or recovery.trigger in the data directory. At the end,
we rename to recovery.done just as we do now. This means that any
parameters put into recovery.conf will not be re-read when we SIGHUP
after end of recovery. Note that recovery.trigger will NOT be read for
parameters and is assumed to be zero-length.
(minor patch required)

This allows these use cases

1. Existing users create $PGDATA/recovery.conf and everything works as
before. No servers crash, because the HA instructions in the wiki
still work. Users can now see the parameters in pg_settings and we can
use SIGHUP without restarting server. Same stuff, new benefits.

2. New users wish to move their existing recovery.conf file to the
config directory. Same file contents, same file name (if desired),
same behaviour, just more convenient location for config management
tools. Recovery is triggered by recovery.trigger in $PGDATA. Trigger
and configuration are now separate, if desired.

3. Split mode. We can put things like trigger_file into the
postgresql.conf directory and also put other parameters (for example
PITR settings) into recovery.conf. Where multiple tools are in use, we
support both APIs.

Specific details...

* primary_conninfo, trigger_file and standby_mode are added to
postgresql.conf.sample
* all ex-recovery.conf parameters are SIGHUP, so no errors if
recovery.conf has changed to .done

If desired, this behaviour could be enabled by a parameter called
recovery_conf_enabled = on (default). When set to off, this would
throw an error if recovery.conf exists. (I don't see a need for this)

The patch to implement this is very small (attached). This works
standalone, but obviously barfs at the actual parameter parsing stage.
Just in case it wasn't clear, this patch is intended to go with the
parts of Fujji's patch that relate to GUC changes.

If we agree, I will merge and re-post before commit.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

read_recovery.conf_from_data_directory.v1.patchapplication/octet-stream; name=read_recovery.conf_from_data_directory.v1.patchDownload+55-15
#2Michael Paquier
michael@paquier.xyz
In reply to: Simon Riggs (#1)
Re: Changing recovery.conf parameters into GUCs

Hi,

The main argument on which this proposal is based on is to keep
backward-compatibility.
This has been discussed before many times and the position of each people
is well-known,
so I am not going back to that...

So, based on *only* what I see in this thread...

On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

What we want to do is make recovery parameters into GUCs, allowing
them to be reset by SIGHUP and also to allow all users to see the
parameters in use, from any session.

The existing mechanism for recovery is that
1. we put parameters in a file called recovery.conf
2. we use the existence of a recovery.conf file to trigger archive
recovery/replication

I also wish to see backwards compatibility maintained, so am proposing
the following:

a) recovery parameters are made into GUCs (for which we have a patch
from Fujii)

b) all processes automatically read recovery.conf as the last step in

reading configuration files, if it exists, even if data_directory
parameter is in use (attached patch)
c) we trigger archive recovery by the presence of either
recovery.conf or recovery.trigger in the data directory. At the end,
we rename to recovery.done just as we do now. This means that any
parameters put into recovery.conf will not be re-read when we SIGHUP
after end of recovery. Note that recovery.trigger will NOT be read for
parameters and is assumed to be zero-length.
(minor patch required)

This allows these use cases

1. Existing users create $PGDATA/recovery.conf and everything works as
before. No servers crash, because the HA instructions in the wiki
still work. Users can now see the parameters in pg_settings and we can
use SIGHUP without restarting server. Same stuff, new benefits.

Forcing hardcoding of "include_if_exists recovery.conf" at the bottom of
postgresql.conf
is not a good thing for the user as it makes postgresql.conf processing
more opaque and
complicates parameter reloading. IMO, simplicity and transparency of
operations are
important when processing parameters.

2. New users wish to move their existing recovery.conf file to the

config directory. Same file contents, same file name (if desired),
same behaviour, just more convenient location for config management
tools. Recovery is triggered by recovery.trigger in $PGDATA. Trigger
and configuration are now separate, if desired.

So, people could be able to also define a recovery.trigger file? What about
the case where both recovery.conf and recovery.trigger are found in the
base folder?
Priority needs to be given to one way of processing or the other. Is it
really our goal
to confuse the user with internal priority mechanisms at least for GUC
handling?

3. Split mode. We can put things like trigger_file into the

postgresql.conf directory and also put other parameters (for example
PITR settings) into recovery.conf. Where multiple tools are in use, we
support both APIs.

Specific details...

* primary_conninfo, trigger_file and standby_mode are added to
postgresql.conf.sample

Not adding all the recovery_target_* parameters would confuse the user.
With this proposal it is actually possible to define a recovery target
inside
postgresql.conf even if the parameter is not plainly written in
postgresql.conf.sample.

* all ex-recovery.conf parameters are SIGHUP, so no errors if
recovery.conf has changed to .done

Drop of recovery.trigger at the same time? And what about the case
where both files are specified, that the server can only remove one of the
trigger files, and is then restarted with only 1 trigger file present?

If desired, this behaviour could be enabled by a parameter called

recovery_conf_enabled = on (default). When set to off, this would
throw an error if recovery.conf exists. (I don't see a need for this)

Me neither, the less GUCs the better.

The patch to implement this is very small (attached). This works
standalone, but obviously barfs at the actual parameter parsing stage.
Just in case it wasn't clear, this patch is intended to go with the
parts of Fujji's patch that relate to GUC changes.

If we agree, I will merge and re-post before commit.

If an agreement is reached based on this proposal, I highly recommend that
you use one of the latest updated version I sent. Fujii's version had some
bugs, one of them being that as standbyModeRequested can be set to true if
specified in postgresql.conf, a portion of the code using in xlog.c can be
triggered even if ArchiveRecoveryRequested is not set to true. I also added
fixes related to documentation.

Comments from others are welcome.
--
Michael

#3Simon Riggs
simon@2ndQuadrant.com
In reply to: Michael Paquier (#2)
Re: Changing recovery.conf parameters into GUCs

On 29 March 2013 01:17, Michael Paquier <michael.paquier@gmail.com> wrote:

The main argument on which this proposal is based on is to keep
backward-compatibility.

The main objective is to get recovery parameters as GUCs, as I said....

On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

What we want to do is make recovery parameters into GUCs, allowing
them to be reset by SIGHUP and also to allow all users to see the
parameters in use, from any session.

From the user's perspective, we are making no changes. All recovery
parameters will work exactly the same as they always did, just now we
get to see their values more easily and we can potentially place them
in a different file if we wish. The user will have no idea that we
plan to do some internal refactoring of how we process the parameters.
So IMHO simplicity means continuing to work the way it always did
work. We simply announce "PostgreSQL now supports configuration
directories. All parameters, including recovery parameters, can be
placed in any configuration file, or in $PGDATA/recovery.conf, as
before".

We introduced "pg_ctl promote" with a new API without removing
existing ones, and some people are still in favour of keeping both
APIs. Doing the same thing here makes sense and reduces conceptual
change.

Early discussions had difficulties because of the lack of config
directories, include_if_exists and this patch. We now have the
technical capability to meet every request. Circumstances have changed
and outcomes may change also.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Michael Paquier
michael@paquier.xyz
In reply to: Simon Riggs (#3)
Re: Changing recovery.conf parameters into GUCs

On Fri, Mar 29, 2013 at 9:59 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 29 March 2013 01:17, Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs <simon@2ndquadrant.com>

wrote:
Early discussions had difficulties because of the lack of config
directories, include_if_exists and this patch. We now have the
technical capability to meet every request. Circumstances have changed
and outcomes may change also.

Thanks for the clarifications. The following questions are still unanswered:
1) If recovery.trigger and recovery.conf are specified. To which one the
priority is given?
2) If both recovery.trigger and recovery.conf are used, let's imagine that
the server removes recovery.trigger but fails in renaming recovery.conf but
a reason or another. Isn't there a risk of inconsistency if both triggering
methods are used at the same time?
3) Forcing a harcode of include_is_exists = 'recovery.conf' at the bottom
of postgresql.conf doesn't look like a hack?
4) Based on your proposal, are all the parameters included in
postgresql.conf.sample or not? Or only primary_conninfo, trigger_file and
standby_mode?
--
Michael

#5Simon Riggs
simon@2ndQuadrant.com
In reply to: Michael Paquier (#4)
Re: Changing recovery.conf parameters into GUCs

On 29 March 2013 13:24, Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, Mar 29, 2013 at 9:59 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 29 March 2013 01:17, Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs <simon@2ndquadrant.com>
wrote:

Early discussions had difficulties because of the lack of config
directories, include_if_exists and this patch. We now have the
technical capability to meet every request. Circumstances have changed
and outcomes may change also.

Thanks for the clarifications. The following questions are still unanswered:

Minor points only. We can implement this differently if you have
alternate proposals.

1) If recovery.trigger and recovery.conf are specified. To which one the
priority is given?

Neither. No priority is required. If either is present we are triggered.

2) If both recovery.trigger and recovery.conf are used, let's imagine that
the server removes recovery.trigger but fails in renaming recovery.conf but
a reason or another. Isn't there a risk of inconsistency if both triggering
methods are used at the same time?

No. If writes to the filesystem fail, you have much bigger problems.

3) Forcing a harcode of include_is_exists = 'recovery.conf' at the bottom of
postgresql.conf doesn't look like a hack?

Well, that's just an emotive term to describe something you don't
like. There are no significant downsides, just a few lines of code,
like we have in many places for various purposes, such as the support
of multiple APIs for triggering standbys.

4) Based on your proposal, are all the parameters included in
postgresql.conf.sample or not? Or only primary_conninfo, trigger_file and
standby_mode?

Other values are specific to particular situations (e.g. PITR) and if
set in the wrong context could easily break replication. We could add
them if people wish it, but it would be commented out with a clear
"don't touch these" message, so it seems more sensible to avoid them
IMHO.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Bruce Momjian
bruce@momjian.us
In reply to: Simon Riggs (#5)
Re: Changing recovery.conf parameters into GUCs

On Fri, Mar 29, 2013 at 01:56:50PM +0000, Simon Riggs wrote:

On 29 March 2013 13:24, Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, Mar 29, 2013 at 9:59 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 29 March 2013 01:17, Michael Paquier <michael.paquier@gmail.com> wrote:

On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs <simon@2ndquadrant.com>
wrote:

Early discussions had difficulties because of the lack of config
directories, include_if_exists and this patch. We now have the
technical capability to meet every request. Circumstances have changed
and outcomes may change also.

Thanks for the clarifications. The following questions are still unanswered:

Minor points only. We can implement this differently if you have
alternate proposals.

Seems we are doing design on this long after the final 9.3 commit fest
should have closed. I think we need to punt this to 9.4.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Simon Riggs
simon@2ndQuadrant.com
In reply to: Michael Paquier (#2)
Re: Changing recovery.conf parameters into GUCs

On 29 March 2013 01:17, Michael Paquier <michael.paquier@gmail.com> wrote:

I highly recommend that
you use one of the latest updated version I sent. Fujii's version had some
bugs, one of them being that as standbyModeRequested can be set to true if
specified in postgresql.conf, a portion of the code using in xlog.c can be
triggered even if ArchiveRecoveryRequested is not set to true. I also added
fixes related to documentation.

Yes, that is what I meant by "Fujii's patch". He would still be
credited first, having done the main part of the work. I'll call it
Michael's updated version to avoid any further confusion.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Josh Berkus
josh@agliodbs.com
In reply to: Simon Riggs (#3)
Re: Changing recovery.conf parameters into GUCs

Simon, All,

The new approach seems fine to me; I haven't looked at the code. If Tom
doesn't feel like it's overly complicated, then this seems like a good
compromise.

The desire to move recovery.conf/trigger to a different directory is
definitely wanted by our Debian contingent. Right now, the fact that
Debian has all .confs in /etc/, but that it doesn't work to relocate
recovery.conf, is a constant source of irritation.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Josh Berkus (#8)
Re: Changing recovery.conf parameters into GUCs

Josh Berkus <josh@agliodbs.com> writes:

The desire to move recovery.conf/trigger to a different directory is
definitely wanted by our Debian contingent. Right now, the fact that
Debian has all .confs in /etc/, but that it doesn't work to relocate
recovery.conf, is a constant source of irritation.

It seems like this is confusing two different problems.

If we get rid of recovery.conf per se in favor of folding the settings
into GUCs in the regular config file, then the first aspect of the issue
goes away, no? The second aspect is where to put the trigger file, and
I'm not at all convinced that anybody would want the trigger file to be
in the same place they put external config files, mainly because the
trigger has to be in a server-writable directory.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Robert Haas
robertmhaas@gmail.com
In reply to: Simon Riggs (#1)
Re: Changing recovery.conf parameters into GUCs

On Thu, Mar 28, 2013 at 11:48 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

What we want to do is make recovery parameters into GUCs, allowing
them to be reset by SIGHUP and also to allow all users to see the
parameters in use, from any session.

The existing mechanism for recovery is that
1. we put parameters in a file called recovery.conf
2. we use the existence of a recovery.conf file to trigger archive
recovery/replication

I also wish to see backwards compatibility maintained, so am proposing
the following:

a) recovery parameters are made into GUCs (for which we have a patch
from Fujii)
b) all processes automatically read recovery.conf as the last step in
reading configuration files, if it exists, even if data_directory
parameter is in use (attached patch)
c) we trigger archive recovery by the presence of either
recovery.conf or recovery.trigger in the data directory. At the end,
we rename to recovery.done just as we do now. This means that any
parameters put into recovery.conf will not be re-read when we SIGHUP
after end of recovery. Note that recovery.trigger will NOT be read for
parameters and is assumed to be zero-length.
(minor patch required)

I still prefer Greg Smith's proposal.

/messages/by-id/4EE91248.8010505@2ndQuadrant.com

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Josh Berkus
josh@agliodbs.com
In reply to: Robert Haas (#10)
Re: Changing recovery.conf parameters into GUCs

Robert, Simon, All,

On 04/01/2013 04:51 AM, Robert Haas wrote:> On Thu, Mar 28, 2013 at
11:48 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

a) recovery parameters are made into GUCs (for which we have a patch
from Fujii)
b) all processes automatically read recovery.conf as the last step in
reading configuration files, if it exists, even if data_directory
parameter is in use (attached patch)
c) we trigger archive recovery by the presence of either
recovery.conf or recovery.trigger in the data directory. At the end,
we rename to recovery.done just as we do now. This means that any
parameters put into recovery.conf will not be re-read when we SIGHUP
after end of recovery. Note that recovery.trigger will NOT be read for
parameters and is assumed to be zero-length.
(minor patch required)

I still prefer Greg Smith's proposal.

/messages/by-id/4EE91248.8010505@2ndQuadrant.com

So, this seems to still be stalled. Is there a way forward on this
which won't cause us to wait *another* version before we have working
replication GUCs?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#12Michael Paquier
michael@paquier.xyz
In reply to: Josh Berkus (#11)
Re: Changing recovery.conf parameters into GUCs

On Sat, Jul 6, 2013 at 3:49 AM, Josh Berkus <josh@agliodbs.com> wrote:

Robert, Simon, All,

On 04/01/2013 04:51 AM, Robert Haas wrote:> On Thu, Mar 28, 2013 at
11:48 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

a) recovery parameters are made into GUCs (for which we have a patch
from Fujii)
b) all processes automatically read recovery.conf as the last step in
reading configuration files, if it exists, even if data_directory
parameter is in use (attached patch)
c) we trigger archive recovery by the presence of either
recovery.conf or recovery.trigger in the data directory. At the end,
we rename to recovery.done just as we do now. This means that any
parameters put into recovery.conf will not be re-read when we SIGHUP
after end of recovery. Note that recovery.trigger will NOT be read for
parameters and is assumed to be zero-length.
(minor patch required)

I still prefer Greg Smith's proposal.

/messages/by-id/4EE91248.8010505@2ndQuadrant.com

So, this seems to still be stalled. Is there a way forward on this
which won't cause us to wait *another* version before we have working
replication GUCs?

Yeah, it would be good to revive this thread now, which is the
beginning of the development cycle. As of now, just to recall
everybody, an agreement on this patch still needs to be found... Simon
is concerned with backward compatibility. Greg presented a nice spec
some time ago (Robert and I liked it) which would break backward
compatibility but allow to begin with a fresh infrastructure.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13Josh Berkus
josh@agliodbs.com
In reply to: Simon Riggs (#1)
Re: Changing recovery.conf parameters into GUCs

On 07/05/2013 10:09 PM, Michael Paquier wrote:

Yeah, it would be good to revive this thread now, which is the
beginning of the development cycle. As of now, just to recall
everybody, an agreement on this patch still needs to be found... Simon
is concerned with backward compatibility. Greg presented a nice spec
some time ago (Robert and I liked it) which would break backward
compatibility but allow to begin with a fresh infrastructure.

As folks know, I favor Smith's approach. However, as far as I can find,
GSmith never posted a patch for his version.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Michael Paquier
michael@paquier.xyz
In reply to: Josh Berkus (#13)
Re: Changing recovery.conf parameters into GUCs

On 2013/07/09, at 4:09, Josh Berkus <josh@agliodbs.com> wrote:

On 07/05/2013 10:09 PM, Michael Paquier wrote:

Yeah, it would be good to revive this thread now, which is the
beginning of the development cycle. As of now, just to recall
everybody, an agreement on this patch still needs to be found... Simon
is concerned with backward compatibility. Greg presented a nice spec
some time ago (Robert and I liked it) which would break backward
compatibility but allow to begin with a fresh infrastructure.

As folks know, I favor Smith's approach. However, as far as I can find,
GSmith never posted a patch for his version.

Actually I did.
/messages/by-id/CAB7nPqR+fpopEDMoecK+AfZB5a8kUUvxpU=1a2JiX5d9s=0s6Q@mail.gmail.com
--
Michael
(Sent from my mobile phone)

#15Simon Riggs
simon@2ndQuadrant.com
In reply to: Josh Berkus (#11)
Re: Changing recovery.conf parameters into GUCs

On 5 July 2013 19:49, Josh Berkus <josh@agliodbs.com> wrote:

Robert, Simon, All,

On 04/01/2013 04:51 AM, Robert Haas wrote:> On Thu, Mar 28, 2013 at
11:48 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

a) recovery parameters are made into GUCs (for which we have a patch
from Fujii)
b) all processes automatically read recovery.conf as the last step in
reading configuration files, if it exists, even if data_directory
parameter is in use (attached patch)
c) we trigger archive recovery by the presence of either
recovery.conf or recovery.trigger in the data directory. At the end,
we rename to recovery.done just as we do now. This means that any
parameters put into recovery.conf will not be re-read when we SIGHUP
after end of recovery. Note that recovery.trigger will NOT be read for
parameters and is assumed to be zero-length.
(minor patch required)

I still prefer Greg Smith's proposal.

/messages/by-id/4EE91248.8010505@2ndQuadrant.com

So, this seems to still be stalled. Is there a way forward on this
which won't cause us to wait *another* version before we have working
replication GUCs?

This needs to be broken down rather than just say "I like Greg's
proposal", or I have written a patch. Writing the patch is not the/an
issue.

Greg's points were these (I have numbered them and named/characterised them)

1. MOVE SETTINGS
All settings move into the postgresql.conf.

Comment: AFAIK, all agree this.

2. RELOCATE RECOVERY PARAMETER FILE(s)
As of 9.2, relocating the postgresql.conf means there are no user
writable files needed in the data directory.

Comment: AFAIK, all except Heikki wanted this. He has very strong
objections to my commit that would have allowed relocating
recovery.conf outside of the data directory. By which he means both
the concepts of triggerring replication and of specifying parameters.
Changes in 9.3 specifically write files to the data directory that
expect this.

3. SEPARATE TRIGGER FILE
Creating a standby.enabled file in the directory that houses the
postgresql.conf (same logic as "include" uses to locate things) puts the
system into recovery mode. That feature needs to save some state, and
making those decisions based on existence of a file is already a thing
we do. Rather than emulating the rename to recovery.done that happens
now, the server can just delete it, to keep from incorrectly returning
to a state it's exited. A UI along the lines of the promote one,
allowing "pg_ctl standby", should fall out of here. I think this is
enough that people who just want to use replication features need never
hear about this file at all, at least during the important to simplify
first run through.

Comment: AFAIK, all except Heikki are OK with this.

4. DISALLOW PREVIOUS API
If startup finds a recovery.conf file where it used to live at,
abort--someone is expecting the old behavior. Hint to RTFM or include a
short migration guide right on the spot. That can have a nice section
about how you might use the various postgresql.conf include* features if
they want to continue managing those files separately. Example: rename
it as replication.conf and use include_if_exists if you want to be able
to rename it to recovery.done like before. Or drop it into a conf.d
directory where the rename will make it then skipped.

Comment: I am against this. Tool vendors are not the problem here.
There is no good reason to just break everybody's scripts with no
warning of future deprecataion and an alternate API, especially since
we now allow multiple parameter files.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16Josh Berkus
josh@agliodbs.com
In reply to: Simon Riggs (#1)
Re: Changing recovery.conf parameters into GUCs

On 07/08/2013 11:43 PM, Simon Riggs wrote:

This needs to be broken down rather than just say "I like Greg's
proposal", or I have written a patch. Writing the patch is not the/an
issue.

Greg's points were these (I have numbered them and named/characterised them)

Thanks for the nice summary! Makes it easy for the rest of us to address.

1. MOVE SETTINGS
All settings move into the postgresql.conf.

Comment: AFAIK, all agree this.

Good to go then.

2. RELOCATE RECOVERY PARAMETER FILE(s)
As of 9.2, relocating the postgresql.conf means there are no user
writable files needed in the data directory.

Comment: AFAIK, all except Heikki wanted this. He has very strong
objections to my commit that would have allowed relocating
recovery.conf outside of the data directory. By which he means both
the concepts of triggerring replication and of specifying parameters.
Changes in 9.3 specifically write files to the data directory that
expect this.

Yeah, I didn't understand why this was shot down either.

Heikki?

3. SEPARATE TRIGGER FILE
Creating a standby.enabled file in the directory that houses the
postgresql.conf (same logic as "include" uses to locate things) puts the
system into recovery mode. That feature needs to save some state, and
making those decisions based on existence of a file is already a thing
we do. Rather than emulating the rename to recovery.done that happens
now, the server can just delete it, to keep from incorrectly returning
to a state it's exited. A UI along the lines of the promote one,
allowing "pg_ctl standby", should fall out of here. I think this is
enough that people who just want to use replication features need never
hear about this file at all, at least during the important to simplify
first run through.

Comment: AFAIK, all except Heikki are OK with this.

One bit of complexity I'd like to see go away is that we have two
trigger files: one to put a server into replication, and one to promote
it. The promotion trigger file is a legacy of warm standby, I believe.
Maybe, now that we have pg_ctl promote available, we can eliminate the
promotion trigger?

Also, previously, deleting the recovery.conf file did not cause the
server to be promoted AFAIK. Is that something we should change if
we're going to keep a trigger file to start replication?

Also, I'm not keen on the idea that the start-replication trigger file
will still be *required*. I really want to be able to manage my setup
entirely through configuration/pg_ctl directives and not be forced to
use a trigger file.

4. DISALLOW PREVIOUS API
If startup finds a recovery.conf file where it used to live at,
abort--someone is expecting the old behavior. Hint to RTFM or include a
short migration guide right on the spot. That can have a nice section
about how you might use the various postgresql.conf include* features if
they want to continue managing those files separately. Example: rename
it as replication.conf and use include_if_exists if you want to be able
to rename it to recovery.done like before. Or drop it into a conf.d
directory where the rename will make it then skipped.

Comment: I am against this. Tool vendors are not the problem here.
There is no good reason to just break everybody's scripts with no
warning of future deprecataion and an alternate API, especially since
we now allow multiple parameter files.

Well, the issue is not so much the presence of a recovery.conf file full
of config variables ... which as you point out is now effectively
supported ... but the use of that as a trigger file. So I think the
two points you make here are flipped.

Personally, I don't care if we support the old recovery.conf-trigger
behavior as long as I'm not forced to use it. The main objection to
supporting both was code complexity, I believe.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#17Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Josh Berkus (#16)
Re: Changing recovery.conf parameters into GUCs

On 10.07.2013 02:54, Josh Berkus wrote:

On 07/08/2013 11:43 PM, Simon Riggs wrote:

1. MOVE SETTINGS
All settings move into the postgresql.conf.

Comment: AFAIK, all agree this.

Good to go then.

+1.

2. RELOCATE RECOVERY PARAMETER FILE(s)
As of 9.2, relocating the postgresql.conf means there are no user
writable files needed in the data directory.

Comment: AFAIK, all except Heikki wanted this. He has very strong
objections to my commit that would have allowed relocating
recovery.conf outside of the data directory. By which he means both
the concepts of triggerring replication and of specifying parameters.
Changes in 9.3 specifically write files to the data directory that
expect this.

Yeah, I didn't understand why this was shot down either.

Heikki?

Does this refer to this:

/messages/by-id/5152F778.2070205@vmware.com

? I listed some objections and suggestions there. Probably the biggest
issue back then, however, was that it was committed so late in the
release cycle. In any case, relocating the config/trigger file has
nothing to do with turning recovery.conf parameters into GUCs, so let's
not confuse this patch with that goal.

3. SEPARATE TRIGGER FILE
Creating a standby.enabled file in the directory that houses the
postgresql.conf (same logic as "include" uses to locate things) puts the
system into recovery mode. That feature needs to save some state, and
making those decisions based on existence of a file is already a thing
we do. Rather than emulating the rename to recovery.done that happens
now, the server can just delete it, to keep from incorrectly returning
to a state it's exited. A UI along the lines of the promote one,
allowing "pg_ctl standby", should fall out of here. I think this is
enough that people who just want to use replication features need never
hear about this file at all, at least during the important to simplify
first run through.

Comment: AFAIK, all except Heikki are OK with this.

Sorry, I don't quite understand what this is about. Can you point me to
the previous discussion on this?

"pg_ctl standby" sounds handy. It's not very useful without something
like pg_rewind or some other mechanism to do a clean failover, though.
Have to make sure that we have enough safeguards in place that you can't
shoot yourself in the foot with that, though; if you turn a master
server into a standby with that, must make sure that you can't corrupt
the database if you point that standby to another standby.

But I don't see how that's related to changing recovery.conf parameters
into gucs.

One bit of complexity I'd like to see go away is that we have two
trigger files: one to put a server into replication, and one to promote
it. The promotion trigger file is a legacy of warm standby, I believe.
Maybe, now that we have pg_ctl promote available, we can eliminate the
promotion trigger?

No, see /messages/by-id/5112A54B.8090500@vmware.com.

Also, previously, deleting the recovery.conf file did not cause the
server to be promoted AFAIK. Is that something we should change if
we're going to keep a trigger file to start replication?

Deleting recovery.conf file (and restarting) takes the server out of
standby mode, but in an unsafe way. Yeah, would be nice to do something
about that.

4. DISALLOW PREVIOUS API
If startup finds a recovery.conf file where it used to live at,
abort--someone is expecting the old behavior. Hint to RTFM or include a
short migration guide right on the spot. That can have a nice section
about how you might use the various postgresql.conf include* features if
they want to continue managing those files separately. Example: rename
it as replication.conf and use include_if_exists if you want to be able
to rename it to recovery.done like before. Or drop it into a conf.d
directory where the rename will make it then skipped.

Comment: I am against this. Tool vendors are not the problem here.
There is no good reason to just break everybody's scripts with no
warning of future deprecataion and an alternate API, especially since
we now allow multiple parameter files.

Well, the issue is not so much the presence of a recovery.conf file full
of config variables ... which as you point out is now effectively
supported ... but the use of that as a trigger file. So I think the
two points you make here are flipped.

Personally, I don't care if we support the old recovery.conf-trigger
behavior as long as I'm not forced to use it. The main objection to
supporting both was code complexity, I believe.

I'm also undecided on this one. If we can figure out a good way forward
that keeps backwards-compatibility, good. But it's not worth very much
to me - if we can get a better interface overall by dropping
backwards-compatibility, then let's drop it.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#18Greg Smith
gsmith@gregsmith.com
In reply to: Heikki Linnakangas (#17)
Re: Changing recovery.conf parameters into GUCs

On 7/10/13 9:39 AM, Heikki Linnakangas wrote:

On 10.07.2013 02:54, Josh Berkus wrote:

One bit of complexity I'd like to see go away is that we have two
trigger files: one to put a server into replication, and one to promote
it. The promotion trigger file is a legacy of warm standby, I believe.
Maybe, now that we have pg_ctl promote available, we can eliminate the
promotion trigger?

No, see /messages/by-id/5112A54B.8090500@vmware.com.

Right, you had some good points in there. STONITH is so hard already,
we need to be careful about eliminating options there.

All the summaries added here have actually managed to revive this one
usefully early in the release cycle! Well done. I just tried to apply
Michael's 20130325_recovery_guc_v3.patch and the bit rot isn't that bad
either. 5 rejection files, nothing big in them.

The only overlap between the recovery.conf GUC work and refactoring the
trigger file is that the trigger file is referenced in there, and we
really need to point it somewhere to be most useful.

Personally, I don't care if we support the old recovery.conf-trigger
behavior as long as I'm not forced to use it.

If you accept Heikki's argument for why the file can't go away
altogether, and it's pretty reasonable, I think two changes reach a
point where everyone can live with this:

-We need a useful default filename for trigger_file to point at.
-"pg_ctl failover" creates that file.

As for the location to default to, the pg_standby docs suggest
/tmp/pgsql.trigger.5432 while the "Binary Replication Tutorial" on the
wiki uses a RedHat directory layout $PGDATA/failover

The main reason I've preferred something in the data directory is that
triggering a standby is too catastrophic for me to be comfortable
putting it in /tmp. Any random hooligan with a shell account can
trigger a standby and break its replication. Putting the unix socket
into /tmp only works because the server creates the file as part of
startup. Here that's not possible, because creating the trigger is the
signalling mechanism.

I don't think there needs to be a CLI interface for putting the
alternate possible text into the trigger--that you can ask for 'fast'
startup. It's nice to have available as an expert, but it's fine for
that to be harder to do.

--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#19Michael Paquier
michael@paquier.xyz
In reply to: Greg Smith (#18)
Re: Changing recovery.conf parameters into GUCs

On Mon, Jul 15, 2013 at 9:09 AM, Greg Smith <greg@2ndquadrant.com> wrote:

All the summaries added here have actually managed to revive this one
usefully early in the release cycle! Well done. I just tried to apply
Michael's 20130325_recovery_guc_v3.patch and the bit rot isn't that bad
either. 5 rejection files, nothing big in them.

Not sure if it will help, but I have in one of my dev branches a
version of this patch in sync with master branch... Please see
attached... At least it will avoid to have to realign the code of v3.
--
Michael

Attachments:

20130714_recovery_guc_v4.patchapplication/octet-stream; name=20130714_recovery_guc_v4.patchDownload+906-961