Re: quoting and recovery.conf

Is there a reason we require single quotes around boolean values in
recovery.conf?

standby_mode = 'off'

This does not work:

standby_mode = off

I knew there were inconsistencies between quoting in postgresql.conf and
recovery.conf, but I didn't realize it extended to boolean quoting. I
see this at the top of recovery.conf now:

# This file consists of lines of the form:
#
# name = 'value'
#
# (The quotes around the value are NOT optional, but the "=" is.)
#

and this issue existed in 8.4 as well. Seems I just never noticed it,
and it not specifically mentioned in the TODO item we already have.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

On Thu, May 13, 2010 at 1:00 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Is there a reason we require single quotes around boolean values in
> recovery.conf?
>
>        standby_mode = 'off'
>
> This does not work:
>
>        standby_mode = off
>
> I knew there were inconsistencies between quoting in postgresql.conf and
> recovery.conf, but I didn't realize it extended to boolean quoting.  I
> see this at the top of recovery.conf now:
>
>        # This file consists of lines of the form:
>        #
>        #   name = 'value'
>        #
>        # (The quotes around the value are NOT optional, but the "=" is.)
>        #
>
> and this issue existed in 8.4 as well.  Seems I just never noticed it,
> and it not specifically mentioned in the TODO item we already have.

I think we should add a TODO to parse recovery.conf with the same code
we use to parse postgresql.conf, or possibly merge the two files.
This issue was previously alluded to here:

http://archives.postgresql.org/pgsql-hackers/2010-04/msg00211.php

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

Robert Haas wrote:
> On Thu, May 13, 2010 at 1:00 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > Is there a reason we require single quotes around boolean values in
> > recovery.conf?
> >
> > ? ? ? ?standby_mode = 'off'
> >
> > This does not work:
> >
> > ? ? ? ?standby_mode = off
> >
> > I knew there were inconsistencies between quoting in postgresql.conf and
> > recovery.conf, but I didn't realize it extended to boolean quoting. ?I
> > see this at the top of recovery.conf now:
> >
> > ? ? ? ?# This file consists of lines of the form:
> > ? ? ? ?#
> > ? ? ? ?# ? name = 'value'
> > ? ? ? ?#
> > ? ? ? ?# (The quotes around the value are NOT optional, but the "=" is.)
> > ? ? ? ?#
> >
> > and this issue existed in 8.4 as well. ?Seems I just never noticed it,
> > and it not specifically mentioned in the TODO item we already have.
>
> I think we should add a TODO to parse recovery.conf with the same code
> we use to parse postgresql.conf, or possibly merge the two files.
> This issue was previously alluded to here:
>
> http://archives.postgresql.org/pgsql-hackers/2010-04/msg00211.php

We have a TODO already for this:

Allow recovery.conf to support the same syntax as postgresql.conf,
including quoting

* recovery.conf parsing problems

I thought the problem was just quotes inside strings, not the
requirement of quotes for everything.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

On Thu, May 13, 2010 at 11:33 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> I think we should add a TODO to parse recovery.conf with the same code
> we use to parse postgresql.conf, or possibly merge the two files.
> This issue was previously alluded to here:
>
> http://archives.postgresql.org/pgsql-hackers/2010-04/msg00211.php

And more than alluded to here:

http://archives.postgresql.org/message-id/407d949e1002131017u657e4aefo2647c2cbf24fe7b4@mail.gmail.com

--
greg

Greg Stark <gsstark(at)mit(dot)edu> writes:
> On Thu, May 13, 2010 at 11:33 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> I think we should add a TODO to parse recovery.conf with the same code
>> we use to parse postgresql.conf, or possibly merge the two files.
>> This issue was previously alluded to here:
>>
>> http://archives.postgresql.org/pgsql-hackers/2010-04/msg00211.php

> And more than alluded to here:

> http://archives.postgresql.org/message-id/407d949e1002131017u657e4aefo2647c2cbf24fe7b4@mail.gmail.com

The main reason for having a separate recovery.conf file is that its
existence is what drives the setting of InArchiveRecovery. If we were
to devise some other trigger for that condition, it'd be possible to
fold all those settings in as GUC variables.

regards, tom lane

On Fri, May 14, 2010 at 9:37 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> I thought the problem was just quotes inside strings, not the
> requirement of quotes for everything.

You can embed a single quote in a parameter value by writing two quotes.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Thu, 2010-05-13 at 21:15 -0400, Tom Lane wrote:
> Greg Stark <gsstark(at)mit(dot)edu> writes:
> > On Thu, May 13, 2010 at 11:33 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> >> I think we should add a TODO to parse recovery.conf with the same code
> >> we use to parse postgresql.conf, or possibly merge the two files.
> >> This issue was previously alluded to here:
> >>
> >> http://archives.postgresql.org/pgsql-hackers/2010-04/msg00211.php
>
> > And more than alluded to here:
>
> > http://archives.postgresql.org/message-id/407d949e1002131017u657e4aefo2647c2cbf24fe7b4@mail.gmail.com
>
> The main reason for having a separate recovery.conf file is that its
> existence is what drives the setting of InArchiveRecovery. If we were
> to devise some other trigger for that condition, it'd be possible to
> fold all those settings in as GUC variables.

And the removal of recovery.conf at end of recovery prevents the
re-entry into archive recovery if we crash.

--
Simon Riggs www.2ndQuadrant.com

On Fri, May 14, 2010 at 6:41 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> The main reason for having a separate recovery.conf file is that its
>> existence is what drives the setting of InArchiveRecovery.  If we were
>> to devise some other trigger for that condition, it'd be possible to
>> fold all those settings in as GUC variables.
>
> And the removal of recovery.conf at end of recovery prevents the
> re-entry into archive recovery if we crash.

These things made sense when we were only dealing with PITR recovery
from a backup intending to bring up the database when the PITR reached
the target. That's now only one specific use case. For hot standbys we
don't want them to come up ever, even if we crash. And for other
standby databases we probably want to control this manually, not
automatically.

In these cases it's really confusing for users that some parameters
have to be in postgresql.conf and some in recovery.conf. They don't
see any distinction between these two files since they don't intend to
ever see the case where the target is reached and the file moved out
of the way automatically.

My suggestion is we should fold all the parameters into
postgresql.conf and treat recovery.conf as an additional
postgresql.conf to read. It would allow any GUC. The only difference
is that it would be moved out of the way automatically when the target
is reached.

Ideally I would have all the configuration be in postgresql.conf and
be identical between all servers, masters and slaves. The only
difference would be a single line which controlled whether to push or
pull wal data. Switching roles would be a regular GUC change in
postgresql.conf and pg_ctl reload.

--
greg

On Thu, 2010-05-27 at 16:20 +0100, Greg Stark wrote:

> My suggestion is we should fold all the parameters into
> postgresql.conf and treat recovery.conf as an additional
> postgresql.conf to read. It would allow any GUC. The only difference
> is that it would be moved out of the way automatically when the target
> is reached.

Good idea, needs a little fleshing out.

What do we do if recovery.conf and postgresql.conf have different
settings in them? Trigger reload at end of recovery? Presumably we would
also ignore server startup parameters in recovery.conf?

--
Simon Riggs www.2ndQuadrant.com

On Fri, May 28, 2010 at 12:20 AM, Greg Stark <gsstark(at)mit(dot)edu> wrote:
> My suggestion is we should fold all the parameters into
> postgresql.conf and treat recovery.conf as an additional
> postgresql.conf to read. It would allow any GUC. The only difference
> is that it would be moved out of the way automatically when the target
> is reached.

I agree to move all the parameter to postgresql.conf. If we do so,
ISTM that it's not useful to leave recovery.conf as a configuration
file. Instead, how about treating recovery.conf as just a trigger
file for recovery? That is, only when recovery.conf is found, the
recovery parameters in postgresql.conf are used and an archive
recovery or standby server start.

If we should avoid a hard-code of a trigger file name, we can
introduce new GUC paramter specifying the path of that. Then a user
can focus on postgresql.conf about configuration. Thought?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center