Request for vote to move forward with recovery.conf overhaul

On 21 December 2012 19:46, Bruce Momjian <bruce@momjian.us> wrote:

There has been discussion in the past of removing or significantly
changing the way streaming replication/point-in-time-recovery (PITR) is
setup in Postgres. Currently the file recovery.conf is used, but that
was designed for PITR and does not serve streaming replication well.

This all should have been overhauled when streaming replication was
added in 2010 in Postgres 9.0. However, time constraints and concern
about backward compatibility has hampered this overhaul.

At this point, backward compatibility seems to be hampering our ability
to move forward. I would like a vote that supports creation of a new
method for setting up streaming replication/point-in-time-recovery,
where backward compatibility is considered only where it is minimally
invasive.

Given that I've said all along that I want change, I'll vote for that,
especially since it is so reasonably worded.

I also want backwards compatibility, so I would like whoever does this
change to also spend some time on that, since it seems that the
balance of time/cost is good enough to make it sensible to do so. I
hope we can spend the time on that investigation, rather than further
debate around what we mean by the word minimally.

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

On Fri, Dec 21, 2012 at 2:46 PM, Bruce Momjian <bruce@momjian.us> wrote:

There has been discussion in the past of removing or significantly
changing the way streaming replication/point-in-time-recovery (PITR) is
setup in Postgres. Currently the file recovery.conf is used, but that
was designed for PITR and does not serve streaming replication well.

This all should have been overhauled when streaming replication was
added in 2010 in Postgres 9.0. However, time constraints and concern
about backward compatibility has hampered this overhaul.

At this point, backward compatibility seems to be hampering our ability
to move forward. I would like a vote that supports creation of a new
method for setting up streaming replication/point-in-time-recovery,
where backward compatibility is considered only where it is minimally
invasive.

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

+1

I seemed to have lost track of the thread that this spawned out of. Is
there a coherent plan for a way forward at this point? Last I recall
it was Simon's plan vs Bruce's plan. I also seem to recall there was a
patch out there too. I think it's even in the commitfest waiting on
author.

/me searches

Here:
https://commitfest.postgresql.org/action/patch_view?id=1026

Perhaps we can get the two plans enumerated, vote, and then get a patch out?

I'd really like to see this in 9.3, but not sure if that ship has
sailed for this feature or not.

Thanks.

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

On Tue, Jan 22, 2013 at 1:11 AM, Phil Sorber <phil@omniti.com> wrote:

On Fri, Dec 21, 2012 at 2:46 PM, Bruce Momjian <bruce@momjian.us> wrote:

There has been discussion in the past of removing or significantly
changing the way streaming replication/point-in-time-recovery (PITR) is
setup in Postgres. Currently the file recovery.conf is used, but that
was designed for PITR and does not serve streaming replication well.

This all should have been overhauled when streaming replication was
added in 2010 in Postgres 9.0. However, time constraints and concern
about backward compatibility has hampered this overhaul.

At this point, backward compatibility seems to be hampering our ability
to move forward. I would like a vote that supports creation of a new
method for setting up streaming replication/point-in-time-recovery,
where backward compatibility is considered only where it is minimally
invasive.

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

+1

I seemed to have lost track of the thread that this spawned out of. Is
there a coherent plan for a way forward at this point? Last I recall
it was Simon's plan vs Bruce's plan. I also seem to recall there was a
patch out there too. I think it's even in the commitfest waiting on
author.

/me searches

Here:
https://commitfest.postgresql.org/action/patch_view?id=1026

Perhaps we can get the two plans enumerated, vote, and then get a patch
out?

I'd really like to see this in 9.3, but not sure if that ship has
sailed for this feature or not.

Yes, that is one of the most important patches in the list, and I could put
some effort in it for either review or coding.
It is an 17-month-old-patch, so of course it does not apply on master.
However before taking any actions, I would like to know the following:
- Simon, are you planning to update this patch?
- As we are rushing to finish wrapping up 9.3, do you consider it is too
late to begin that?

Thanks,
--
Michael Paquier
http://michael.otacoo.com

On Mon, Jan 21, 2013 at 6:23 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Yes, that is one of the most important patches in the list, and I could put
some effort in it for either review or coding.

I think it would be great if you could elaborate on your reasons for
feeling that this patch is particularly important.

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

On Tue, Jan 22, 2013 at 9:27 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Jan 21, 2013 at 6:23 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Yes, that is one of the most important patches in the list, and I could

put

some effort in it for either review or coding.

I think it would be great if you could elaborate on your reasons for
feeling that this patch is particularly important.

Sure. recovery.conf has been initially created for PITR management, but
since 9.0 and the introduction of streaming replication it is being used
for too many things that it was first targeting for, like now it can be
used to define where a slave can connect to a root node, fetch the
archives, etc. I am seeing for a long time on hackers (2010?) that postgres
should make the move on giving up recovery.conf and merge it with
postgresql.conf.

I didn't know about the existence of a patch aimed to merge the parameters
of postgresql.conf and recovery.conf, and, just by looking at the patch,
half of the work looks to be already done. I thought it might be worth to
at least update the patch or provide some feedback.

I agree that this might break backward-compatibility and that it would be
more suited for a 10.0(?), but as 9.3 development is already close to its
end, progressing on this discussion and decide whether this could be
shipped for 9.3 or later release is important. If it is decided to give up
on this feature, well let's do that later. If it is worth the shot, let's
put some effort for it.
--
Michael Paquier
http://michael.otacoo.com

Le mardi 22 janvier 2013 01:54:50, Michael Paquier a écrit :

On Tue, Jan 22, 2013 at 9:27 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Jan 21, 2013 at 6:23 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Yes, that is one of the most important patches in the list, and I could

put

some effort in it for either review or coding.

I think it would be great if you could elaborate on your reasons for
feeling that this patch is particularly important.

Sure. recovery.conf has been initially created for PITR management, but
since 9.0 and the introduction of streaming replication it is being used
for too many things that it was first targeting for, like now it can be
used to define where a slave can connect to a root node, fetch the
archives, etc. I am seeing for a long time on hackers (2010?) that postgres
should make the move on giving up recovery.conf and merge it with
postgresql.conf.

I didn't know about the existence of a patch aimed to merge the parameters
of postgresql.conf and recovery.conf, and, just by looking at the patch,
half of the work looks to be already done. I thought it might be worth to
at least update the patch or provide some feedback.

I agree that this might break backward-compatibility and that it would be
more suited for a 10.0(?), but as 9.3 development is already close to its
end, progressing on this discussion and decide whether this could be
shipped for 9.3 or later release is important. If it is decided to give up
on this feature, well let's do that later. If it is worth the shot, let's
put some effort for it.

I though the idea is that for 9.3 we can have new feature, so everything can
go in postgreql.conf, and also allows using recovery.conf so it does not break
backward-compatibility.

--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

On 21 January 2013 23:23, Michael Paquier <michael.paquier@gmail.com> wrote:

It is an 17-month-old-patch, so of course it does not apply on master.
However before taking any actions, I would like to know the following:
- Simon, are you planning to update this patch?

It's on my list, but not at the front to the queue.

If you want to know what my priority queue looks like... (preliminary opinion)
1. Skip checkpoint on promoting from streaming replication (almost ready)
2. Further review of WAL decoding (opinion pending, but very solid)
3. Performance Improvement by reducing WAL for Update Operation (likely commit)
4. Row Level Security (possible commit)
5. Checksums (maybe commit)
6. Make recovery.conf parameters into GUCs (commit something of use,
but very basic)

Which is at least 2 weeks work

- As we are rushing to finish wrapping up 9.3, do you consider it is too
late to begin that?

It's late. And it doesn't get to jump my queue. Whether its too late
is not for me to say.

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

On Wed, Jan 23, 2013 at 8:51 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 21 January 2013 23:23, Michael Paquier <michael.paquier@gmail.com>
wrote:

It is an 17-month-old-patch, so of course it does not apply on master.
However before taking any actions, I would like to know the following:
- Simon, are you planning to update this patch?

It's on my list, but not at the front to the queue.

If you want to know what my priority queue looks like... (preliminary
opinion)
1. Skip checkpoint on promoting from streaming replication (almost ready)
2. Further review of WAL decoding (opinion pending, but very solid)
3. Performance Improvement by reducing WAL for Update Operation (likely
commit)
4. Row Level Security (possible commit)
5. Checksums (maybe commit)
6. Make recovery.conf parameters into GUCs (commit something of use,
but very basic)

Thanks. It is good to know.
As you are not planning to touch the patch, I took myself the last version
of Masao and realigned it with current head.
Here is what the patch does in details:
- Move all the recovery.conf parameters in postgresql.conf
- recovery.conf is removed (no backward compatibility in this version of
the patch)
- if you want to trigger a recovery at promotion or have the recovery
parameters read on slave at startup, you need to create a file called
recovery.trigger in PGDATA. (something like "touch recovery.conf" is
enough). Once recovery is done, recovery.trigger is changed to
recovery.done.

I found in the original patch a couple of bugs, which might have been there
because things have changed a bit since 9.2 dev, especially around the
trigger file. Those bugs are fixed. I also tested this new configuration
set with several slaves, cascading slaves and even played with timeline
switches... And it worked correctly.
I found that support for pg_basebackup -R was in the old patch, and I
haven't done anything for that yet.

Hope it helps. Thanks,
--
Michael Paquier
http://michael.otacoo.com

On Wed, Jan 23, 2013 at 1:49 PM, Michael Paquier
<michael.paquier@gmail.com>wrote:

I found that support for pg_basebackup -R was in the old patch, and I
haven't done anything for that yet.

Sorry, I meant that pg_basebackup -R support was NOT in the old patch, and
I haven't done anything about that.
Typed too quickly...
--
Michael Paquier
http://michael.otacoo.com

- if you want to trigger a recovery at promotion or have the recovery
parameters read on slave at startup, you need to create a file called
recovery.trigger in PGDATA. (something like "touch recovery.conf" is
enough). Once recovery is done, recovery.trigger is changed to
recovery.done.

So, per our compromise, we'd want to change the name of that file back
to recovery.conf. There's no point in renaming the file if we're not
going to get rid of it.

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

On 23 January 2013 04:49, Michael Paquier <michael.paquier@gmail.com> wrote:

- recovery.conf is removed (no backward compatibility in this version of the
patch)

If you want to pursue that, you know where it leads. No, rebasing a
rejected patch doesn't help, its just relighting a fire that shouldn't
ever have been lit.

Pushing to do that out of order is just going to drain essential time
out of this CF from all of us.

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

On 2013/01/23, at 18:12, Simon Riggs <simon@2ndQuadrant.com> wrote:

On 23 January 2013 04:49, Michael Paquier <michael.paquier@gmail.com> wrote:

- recovery.conf is removed (no backward compatibility in this version of the
patch)

If you want to pursue that, you know where it leads. No, rebasing a
rejected patch doesn't help, its just relighting a fire that shouldn't
ever have been lit.

Pushing to do that out of order is just going to drain essential time
out of this CF from all of us.

No problem to support both. The only problem I see is if the same parameter is defined in recovery.conf and postgresql.conf, is the priority given to recovery.conf?
--
Michael Paquier
http://michael.otacoo.com
(Sent from my mobile phone)

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

On Wed, Jan 23, 2013 at 6:36 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On 2013/01/23, at 18:12, Simon Riggs <simon@2ndQuadrant.com> wrote:

On 23 January 2013 04:49, Michael Paquier <michael.paquier@gmail.com> wrote:

- recovery.conf is removed (no backward compatibility in this version of the
patch)

If you want to pursue that, you know where it leads. No, rebasing a
rejected patch doesn't help, its just relighting a fire that shouldn't
ever have been lit.

Pushing to do that out of order is just going to drain essential time
out of this CF from all of us.

No problem to support both. The only problem I see is if the same parameter is defined in recovery.conf and postgresql.conf, is the priority given to recovery.conf?

I would think that if someone created a recovery.conf file they would
expect that to be given priority. Otherwise they would know that was a
deprecated method and would set it in postgresql.conf only.

--
Michael Paquier
http://michael.otacoo.com
(Sent from my mobile phone)

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

On Sun, Jan 27, 2013 at 7:14 AM, Phil Sorber <phil@omniti.com> wrote:

On Wed, Jan 23, 2013 at 6:36 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On 2013/01/23, at 18:12, Simon Riggs <simon@2ndQuadrant.com> wrote:

On 23 January 2013 04:49, Michael Paquier <michael.paquier@gmail.com>

wrote:

- recovery.conf is removed (no backward compatibility in this version

of the

patch)

If you want to pursue that, you know where it leads. No, rebasing a
rejected patch doesn't help, its just relighting a fire that shouldn't
ever have been lit.

Pushing to do that out of order is just going to drain essential time
out of this CF from all of us.

No problem to support both. The only problem I see is if the same

parameter is defined in recovery.conf and postgresql.conf, is the priority
given to recovery.conf?

I would think that if someone created a recovery.conf file they would
expect that to be given priority. Otherwise they would know that was a
deprecated method and would set it in postgresql.conf only.

Please find attached an half-cooked patch supporting both postgresql.conf
and recovery.conf. Priority is given to recovery.conf if the same parameter
is specified in both files. I have updated the docs in consequence but I
think they can be improved.
The main modification here is in xlog.c:readRecoveryCommandFile where the
deparsed output values of recovery.conf is transferred to the new GUCs
using SetConfigOption($OPTION, $VALUE, PGC_POSTMASTER, PGC_S_OVERRIDE) as
bridge. This does not work yet, SetConfigOption is not able to detect the
new values. Comments?
--
Michael Paquier
http://michael.otacoo.com

On Sat, Jan 26, 2013 at 9:49 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Sun, Jan 27, 2013 at 7:14 AM, Phil Sorber <phil@omniti.com> wrote:

On Wed, Jan 23, 2013 at 6:36 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On 2013/01/23, at 18:12, Simon Riggs <simon@2ndQuadrant.com> wrote:

On 23 January 2013 04:49, Michael Paquier <michael.paquier@gmail.com>
wrote:

- recovery.conf is removed (no backward compatibility in this version
of the
patch)

If you want to pursue that, you know where it leads. No, rebasing a
rejected patch doesn't help, its just relighting a fire that shouldn't
ever have been lit.

Pushing to do that out of order is just going to drain essential time
out of this CF from all of us.

No problem to support both. The only problem I see is if the same
parameter is defined in recovery.conf and postgresql.conf, is the priority
given to recovery.conf?

I would think that if someone created a recovery.conf file they would
expect that to be given priority. Otherwise they would know that was a
deprecated method and would set it in postgresql.conf only.

Please find attached an half-cooked patch supporting both postgresql.conf
and recovery.conf. Priority is given to recovery.conf if the same parameter
is specified in both files. I have updated the docs in consequence but I
think they can be improved.
The main modification here is in xlog.c:readRecoveryCommandFile where the
deparsed output values of recovery.conf is transferred to the new GUCs using
SetConfigOption($OPTION, $VALUE, PGC_POSTMASTER, PGC_S_OVERRIDE) as bridge.
This does not work yet, SetConfigOption is not able to detect the new
values. Comments?

So... what happens when recovery ends? Do the settings loaded from
recovery.conf get reverted, or what?

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

On Sun, Jan 27, 2013 at 1:41 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Sat, Jan 26, 2013 at 9:49 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Sun, Jan 27, 2013 at 7:14 AM, Phil Sorber <phil@omniti.com> wrote:

On Wed, Jan 23, 2013 at 6:36 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On 2013/01/23, at 18:12, Simon Riggs <simon@2ndQuadrant.com> wrote:

On 23 January 2013 04:49, Michael Paquier <michael.paquier@gmail.com

wrote:

- recovery.conf is removed (no backward compatibility in this

version

of the
patch)

If you want to pursue that, you know where it leads. No, rebasing a
rejected patch doesn't help, its just relighting a fire that

shouldn't

ever have been lit.

Pushing to do that out of order is just going to drain essential time
out of this CF from all of us.

No problem to support both. The only problem I see is if the same
parameter is defined in recovery.conf and postgresql.conf, is the

priority

given to recovery.conf?

I would think that if someone created a recovery.conf file they would
expect that to be given priority. Otherwise they would know that was a
deprecated method and would set it in postgresql.conf only.

Please find attached an half-cooked patch supporting both postgresql.conf
and recovery.conf. Priority is given to recovery.conf if the same

parameter

is specified in both files. I have updated the docs in consequence but I
think they can be improved.
The main modification here is in xlog.c:readRecoveryCommandFile where the
deparsed output values of recovery.conf is transferred to the new GUCs

using

SetConfigOption($OPTION, $VALUE, PGC_POSTMASTER, PGC_S_OVERRIDE) as

bridge.

This does not work yet, SetConfigOption is not able to detect the new
values. Comments?

So... what happens when recovery ends? Do the settings loaded from
recovery.conf get reverted, or what?

With current patch the settings are kept if set in postgresql.conf and
discarded if they are loaded as GUC after a server restart or reload.
--
Michael Paquier
http://michael.otacoo.com

On Sun, Jan 27, 2013 at 1:01 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

So... what happens when recovery ends? Do the settings loaded from
recovery.conf get reverted, or what?

With current patch the settings are kept if set in postgresql.conf and
discarded if they are loaded as GUC after a server restart or reload.

I can't understand that answer. Is there a word missing or something?

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

On 1/23/13 6:36 AM, Michael Paquier wrote:

The only problem I see is if the same parameter is defined in recovery.conf and postgresql.conf, is the priority given to recovery.conf?

This sort of thing is what dragged down the past work on this. I don't
really agree with the idea this thread is based on, that it was backward
compatibility that kept this from being finished last year. I put a
good bit of time into a proposal that a few people seemed happy with;
all its ideas seem to have washed away. That balanced a couple of goals:

-Allow a "pg_ctl standby" and "pg_ctl recover" command that work
similarly to "promote". This should slim down the work needed for the
first replication setup people do.
-Make it obvious when people try to use recovery.conf that it's not
supported anymore.
-Provide a migration path for tool authors strictly in the form of some
documentation and error message hints. That was it as far as
concessions to backward compatibility.

The wrap-up I did started at
/messages/by-id/4EE91248.8010505@2ndQuadrant.com
and only had a few responses/controversy from there. Robert wrote a
good summary:

1. Get rid of recovery.conf - error out if it is seen
2. For each parameter that was previously a recovery.conf parameter,
make it a GUC
3. For the parameter that was "does recovery.conf exist?", replace it
with "does standby.enabled exist?".

I thought this stopped from there because no one went back to clean up
Fujii's submission, which Simon and Michael have now put more time into.
There is not much distance between it and the last update Michael
sent. Here's the detailed notes from my original proposal, with updates
to incorporate the main feedback I got then; note that much of this is
documentation rather than code:

-Creating a standby.enabled file 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.

This file can be relocated to the config directory, similarly to how the
include directory looks for things. There was a concern that this would
require write permissions that don't exist on systems that relocate
configs, like Debian/Ubuntu. That doesn't look to be a real issue
though. Here's a random Debian server showing the postgres user can
write to all of those:

$ ls -ld /etc/postgresql
drwxr-xr-x 4 root root 4096 Jun 29 2012 /etc/postgresql
$ ls -ld /etc/postgresql/9.1
drwxr-xr-x 3 postgres postgres 4096 Jul 1 2012 /etc/postgresql/9.1
$ ls -ld /etc/postgresql/9.1/main
drwxr-xr-x 2 postgres postgres 4096 Mar 3 11:00 /etc/postgresql/9.1/main

-A similar recovery.enabled file turns on PITR recovery.

-It should be possible to copy a postgresql.conf file from master to
standby and just use it. For example:
--"standby_mode = on": Ignored unless you've started the server with
standby.enabled, won't bother the master if you include it.
--"primary_conninfo": This will look funny on the master showing it
connecting to itself, but it will get ignored there too.

-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
what you used to make recovery.conf 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 config/ directory (similarly to the
proposal for SET PERSISTENT) where the rename to recovery.done will make
it then skipped. (Only files ending with .conf are processed by
include_dir)

-Tools such as pgpool that want to write a simple configuration file,
only touching the things that used to go into recovery.conf, can tell
people to do the same trick. End their postgresql.conf with a call to
\include_if_exists replication.conf as part of setup. While I don't
like pushing problems toward tool vendors, as one I think validating if
this has been done doesn't require the sort of fully GUC compatible
parser people (rightly) want to avoid. A simple scan of the
postgresql.conf looking for the recommended text at the front of a line
could confirm whether that bit is there. And adding a single
"include_if_exists" line to the end of the postgresql.conf is not a
terrible edit job to consider pushing toward tools. None of this is any
more complicated than the little search and replace job that initdb does
right now.

-If you want to do something special yourself to clean up when recovery
finishes, perhaps to better emulate the old "those settings go away"
implementation, there's already recovery_end_command available for that.
Let's say you wanted to force the old name and did "include_if_exists
config/recovery.conf". Now you could do:

recovery_end_command = 'rm -f /tmp/pgsql.trigger.5432 && mv
conf.d/recovery.conf conf.d/recovery.done'

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

On Sun, Mar 3, 2013 at 9:25 PM, Greg Smith <greg@2ndquadrant.com> wrote:

-Allow a "pg_ctl standby" and "pg_ctl recover" command that work similarly
to "promote". This should slim down the work needed for the first
replication setup people do.
-Make it obvious when people try to use recovery.conf that it's not
supported anymore.
-Provide a migration path for tool authors strictly in the form of some
documentation and error message hints. That was it as far as concessions to
backward compatibility.

The wrap-up I did started at
/messages/by-id/4EE91248.8010505@2ndQuadrant.com and
only had a few responses/controversy from there. Robert wrote a good
summary:

1. Get rid of recovery.conf - error out if it is seen
2. For each parameter that was previously a recovery.conf parameter, make it
a GUC
3. For the parameter that was "does recovery.conf exist?", replace it with
"does standby.enabled exist?".

All that works for me.

I thought this stopped from there because no one went back to clean up
Fujii's submission, which Simon and Michael have now put more time into.
There is not much distance between it and the last update Michael sent.
Here's the detailed notes from my original proposal, with updates to
incorporate the main feedback I got then; note that much of this is
documentation rather than code:

-Creating a standby.enabled file 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.

This file can be relocated to the config directory, similarly to how the
include directory looks for things. There was a concern that this would
require write permissions that don't exist on systems that relocate configs,
like Debian/Ubuntu. That doesn't look to be a real issue though. Here's a
random Debian server showing the postgres user can write to all of those:

$ ls -ld /etc/postgresql
drwxr-xr-x 4 root root 4096 Jun 29 2012 /etc/postgresql
$ ls -ld /etc/postgresql/9.1
drwxr-xr-x 3 postgres postgres 4096 Jul 1 2012 /etc/postgresql/9.1
$ ls -ld /etc/postgresql/9.1/main
drwxr-xr-x 2 postgres postgres 4096 Mar 3 11:00 /etc/postgresql/9.1/main

-A similar recovery.enabled file turns on PITR recovery.

-It should be possible to copy a postgresql.conf file from master to standby
and just use it. For example:
--"standby_mode = on": Ignored unless you've started the server with
standby.enabled, won't bother the master if you include it.
--"primary_conninfo": This will look funny on the master showing it
connecting to itself, but it will get ignored there too.

-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 what you used
to make recovery.conf 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 config/ directory (similarly to the proposal for SET PERSISTENT) where the
rename to recovery.done will make it then skipped. (Only files ending with
.conf are processed by include_dir)

-Tools such as pgpool that want to write a simple configuration file,
only touching the things that used to go into recovery.conf, can tell
people to do the same trick. End their postgresql.conf with a call to
\include_if_exists replication.conf as part of setup. While I don't
like pushing problems toward tool vendors, as one I think validating if
this has been done doesn't require the sort of fully GUC compatible
parser people (rightly) want to avoid. A simple scan of the
postgresql.conf looking for the recommended text at the front of a line
could confirm whether that bit is there. And adding a single
"include_if_exists" line to the end of the postgresql.conf is not a
terrible edit job to consider pushing toward tools. None of this is any
more complicated than the little search and replace job that initdb does
right now.

-If you want to do something special yourself to clean up when recovery
finishes, perhaps to better emulate the old "those settings go away"
implementation, there's already recovery_end_command available for that.
Let's say you wanted to force the old name and did "include_if_exists
config/recovery.conf". Now you could do:

recovery_end_command = 'rm -f /tmp/pgsql.trigger.5432 && mv
conf.d/recovery.conf conf.d/recovery.done'

No argument with any of that, either.

If that's what we're implementing, I'm on board.

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