Horton

Thread

  1. Request for vote to move forward with recovery.conf overhaul

    Bruce Momjian <bruce@momjian.us> — 2012-12-21T19:46:10Z

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

  2. Re: Request for vote to move forward with recovery.conf overhaul

    Simon Riggs <simon@2ndquadrant.com> — 2012-12-22T00:49:42Z

    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

  3. Re: Request for vote to move forward with recovery.conf overhaul

    Phil Sorber <phil@omniti.com> — 2013-01-21T16:11:32Z

    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.

  4. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-01-21T23:23:45Z

    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

  5. Re: Request for vote to move forward with recovery.conf overhaul

    Robert Haas <robertmhaas@gmail.com> — 2013-01-22T00:27:47Z

    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

  6. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-01-22T00:54:50Z

    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

  7. Re: Request for vote to move forward with recovery.conf overhaul

    Cédric Villemain <cedric@2ndquadrant.com> — 2013-01-22T22:47:25Z

    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

  8. Re: Request for vote to move forward with recovery.conf overhaul

    Simon Riggs <simon@2ndquadrant.com> — 2013-01-22T23:51:47Z

    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

  9. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-01-23T04:49:16Z

    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

  10. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-01-23T04:54:07Z

    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

  11. Re: Request for vote to move forward with recovery.conf overhaul

    Josh Berkus <josh@agliodbs.com> — 2013-01-23T05:00:23Z

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

  12. Re: Request for vote to move forward with recovery.conf overhaul

    Simon Riggs <simon@2ndquadrant.com> — 2013-01-23T09:12:49Z

    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

  13. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-01-23T11:36:16Z

    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)

  14. Re: Request for vote to move forward with recovery.conf overhaul

    Phil Sorber <phil@omniti.com> — 2013-01-26T22:14:36Z

    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)

  15. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-01-27T02:49:49Z

    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

  16. Re: Request for vote to move forward with recovery.conf overhaul

    Robert Haas <robertmhaas@gmail.com> — 2013-01-27T04:41:15Z

    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

  17. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-01-27T06:01:51Z

    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

  18. Re: Request for vote to move forward with recovery.conf overhaul

    Robert Haas <robertmhaas@gmail.com> — 2013-01-27T17:01:21Z

    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

  19. Re: Request for vote to move forward with recovery.conf overhaul

    Greg Smith <greg@2ndquadrant.com> — 2013-03-04T02:25:57Z

    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 
http://www.postgresql.org/message-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

  20. Re: Request for vote to move forward with recovery.conf overhaul

    Robert Haas <robertmhaas@gmail.com> — 2013-03-06T04:23:31Z

    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
> http://www.postgresql.org/message-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

  21. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-03-06T05:08:06Z

    Thanks for taking time in typing a complete summary of the situation. That
really helps.

On Mon, Mar 4, 2013 at 11:25 AM, Greg Smith <greg@2ndquadrant.com> wrote:

> 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 http://www.postgresql.org/**
> message-id/4EE91248.8010505@**2ndQuadrant.com<http://www.postgresql.org/message-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?".
>
Agreed on that.


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

Just to be picky. I recommend using the version dated of 23rd of January as
a work base if we definitely get rid of recovery.conf, the patch file is
called 20130123_recovery_unite.patch.
The second patch I sent on the 27th tried to support both recovery.conf and
postgresql.conf, but this finishes with a lot of duplicated code as two
sets of functions are necessary to deparse options for both postgresql.conf
and recovery.conf...


> 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'
>
Looks good to me too.
Based on the patch I already sent before, there are a couple of things
missing:
- There are no pg_ctl standby/recover commands implemented yet (no that
difficult to add)
- a certain amount of work is needed for documentation

Btw, I have a concern about that: is it really the time to finish that for
this release knowing that the 9.3 feature freeze is getting close? I still
don't know when it will be but it is... close.
-- 
Michael

  22. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-03-09T03:34:54Z

    On Wed, Mar 6, 2013 at 2:08 PM, Michael Paquier
<michael.paquier@gmail.com>wrote:

> Thanks for taking time in typing a complete summary of the situation. That
> really helps.
>
> On Mon, Mar 4, 2013 at 11:25 AM, Greg Smith <greg@2ndquadrant.com> wrote:
>
>> 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 http://www.postgresql.org/**
>> message-id/4EE91248.8010505@**2ndQuadrant.com<http://www.postgresql.org/message-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?".
>>
> Agreed on that.
>
>
>> 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.
>
> Just to be picky. I recommend using the version dated of 23rd of January
> as a work base if we definitely get rid of recovery.conf, the patch file is
> called 20130123_recovery_unite.patch.
> The second patch I sent on the 27th tried to support both recovery.conf
> and postgresql.conf, but this finishes with a lot of duplicated code as two
> sets of functions are necessary to deparse options for both postgresql.conf
> and recovery.conf...
>
>
>> 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'
>>
> Looks good to me too.
> Based on the patch I already sent before, there are a couple of things
> missing:
> - There are no pg_ctl standby/recover commands implemented yet (no that
> difficult to add)
> - a certain amount of work is needed for documentation
>
> Btw, I have a concern about that: is it really the time to finish that for
> this release knowing that the 9.3 feature freeze is getting close? I still
> don't know when it will be but it is... close.
>
This patch is still in the current commit fest. Any objections in marking
it as returned with feedback and put it in the next commit fest? We could
work on that again at the next release as it looks that there is not much
time to work on it for 9.3.
-- 
Michael

  23. Re: Request for vote to move forward with recovery.conf overhaul

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2013-03-09T04:02:51Z

    Michael Paquier escribió:
> On Wed, Mar 6, 2013 at 2:08 PM, Michael Paquier
> <michael.paquier@gmail.com>wrote:

> > Looks good to me too.
> > Based on the patch I already sent before, there are a couple of things
> > missing:
> > - There are no pg_ctl standby/recover commands implemented yet (no that
> > difficult to add)
> > - a certain amount of work is needed for documentation
> >
> > Btw, I have a concern about that: is it really the time to finish that for
> > this release knowing that the 9.3 feature freeze is getting close? I still
> > don't know when it will be but it is... close.
> >
> This patch is still in the current commit fest. Any objections in marking
> it as returned with feedback and put it in the next commit fest? We could
> work on that again at the next release as it looks that there is not much
> time to work on it for 9.3.

If the patch is almost ready, IMHO you should complete it and post a
version that implements the consensus design.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

  24. Re: Request for vote to move forward with recovery.conf overhaul

    Greg Smith <greg@2ndquadrant.com> — 2013-03-09T04:20:54Z

    On 3/8/13 10:34 PM, Michael Paquier wrote:
> This patch is still in the current commit fest. Any objections in
> marking it as returned with feedback and put it in the next commit fest?

There are currently 20 "Needs Review" and 14 "Waiting on Author" things 
left in the queue, so it's not quite that there's no time left.  There 
really isn't very much left to do on this.  The rough consensus idea 
from before takes a while to describe, but there was not a complicated 
implementation in that.  The overlap with the still possible to commit 
SET PERSISTENT is probably the worst potential issue this is facing now, 
but that's not even a real issue yet.

If you're out of time to work on it and want to back out of here having 
made good progress, that's fine.  I'd be tempted to work on this thing 
myself for a bit just to try and finally get it done.  If it gets punted 
forward, we'll be right back to facing bit rot and remembering what was 
going on again, which is what killed the momentum toward committing this 
the last time.

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

  25. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-03-09T04:35:28Z

    On Sat, Mar 9, 2013 at 1:20 PM, Greg Smith <greg@2ndquadrant.com> wrote:

> There are currently 20 "Needs Review" and 14 "Waiting on Author" things
> left in the queue, so it's not quite that there's no time left.  There
> really isn't very much left to do on this.  The rough consensus idea from
> before takes a while to describe, but there was not a complicated
> implementation in that.  The overlap with the still possible to commit SET
> PERSISTENT is probably the worst potential issue this is facing now, but
> that's not even a real issue yet.
>
OK thanks for your feedback.


> If you're out of time to work on it and want to back out of here having
> made good progress, that's fine.  I'd be tempted to work on this thing
> myself for a bit just to try and finally get it done.  If it gets punted
> forward, we'll be right back to facing bit rot and remembering what was
> going on again, which is what killed the momentum toward committing this
> the last time.

I think I will be able to work on that but not before Monday. This depends
also on how REINDEX CONCURRENTLY goes...
-- 
Michael

  26. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-03-13T11:50:54Z

    Hi all,

Please find attached an updated patch doing what is written below.

On Mon, Mar 4, 2013 at 11:25 AM, Greg Smith <greg@2ndquadrant.com> wrote:

> 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?".
>

There are still a couple of things missing:
- pg_basebackup supports an option --write-recovery-conf, I haven't
modified anything yet, but I think that we should replace that by an option
that write standby.enabled in base backup and adds the relevant parameters
in postgresql.conf. Any input on that is welcome.
- no migration guide is written yet. Where to write it? I think I will need
some help here...
- The current error message if recovery.conf is found in data folder is
that:
+       if (AllocateFile(RECOVERY_COMMAND_FILE, "r") != NULL)
+               ereport(FATAL,
+                               (errmsg("\"%s\" is not supported anymore as
a recovery method",
+                                               RECOVERY_COMMAND_FILE),
+                                errdetail("Refer to appropriate
documentation about migration methods")));
Any better ideas?

I found some inconsistent behavior when a slave had no standby.enabled
files and recovery settings: the slave with "hot_standby = on" tried to
recover WAL files from archives instead of failing with errors of the type
"could not locate required checkpoint record" and then stop. This is fixed.

Regards,
-- 
Michael

  27. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-03-25T04:08:29Z

    Hi all,

pg_basebackup contains an option called --write-recovery-conf allowing the
user to generate automatically some values for recovery.conf.
As in this patch all the recovery parameters are moved to postgresql.conf
as GUCs, I replaced the existing option by a new option called
--write-enable-standby that simply creates an empty standby.enabled file in
the base backup if specified. By looking at the code of pg_basebackup, I
noticed that adding some automatically-generated input to postgresql.conf
would not impact so much the code when using plain format. However adding
special handling in the code for tar format would make the code less
readable and less intuitive. Also do we really need to generate recovery
parameters with pg_basebackup? In the spec discussed we want the user to
define once recovery parameters in the master's postgresql.conf such as the
same values could be reused directly in the slave's base backup.

I have also reworked the documentation a bit:
- Addition of a section called "Recovery" in the configuration section to
describe all the parameters that are activated only if standby.enabled is
found at server startup.
- Addition of a note at the end of the new "Recovery" section indicating
how to migrate an existing configuration using recovery.conf to the new
system using include_dir or include_if_exists.

Feedback is warmly welcome.
Regards,
-- 
Michael

  28. Re: Request for vote to move forward with recovery.conf overhaul

    Simon Riggs <simon@2ndquadrant.com> — 2013-03-25T04:14:22Z

    On 25 March 2013 04:08, Michael Paquier <michael.paquier@gmail.com> wrote:

> Feedback is warmly welcome.

I'll look at this in the coming week.

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

  29. Re: Request for vote to move forward with recovery.conf overhaul

    Michael Paquier <michael.paquier@gmail.com> — 2013-03-25T23:26:42Z

    On Mon, Mar 25, 2013 at 1:14 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

> On 25 March 2013 04:08, Michael Paquier <michael.paquier@gmail.com> wrote:
>
> > Feedback is warmly welcome.
>
> I'll look at this in the coming week.
>
Thanks.
-- 
Michael