Re: unite recovery.conf and postgresql.conf

On Tue, Nov 1, 2011 at 5:59 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Mon, Oct 31, 2011 at 5:23 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> On Mon, Oct 31, 2011 at 7:38 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>>>> In 9.2 the presence of recovery.conf can and therefore should continue
>>>> to act as it does in 9.1.
>>>
>>> This means that recovery.conf is renamed to recovery.done at the end of
>>> recovery. IOW, all settings in recovery.conf are reset when recovery ends.
>>> Then if you run "pg_ctl reload" after recovery, you'll get something like
>>> the following error message and the reload will always fail;
>>>
>>>   LOG:  parameter "standby_mode" cannot be changed without restarting
>>> the server
>>
>>> To resolve this issue,
>>
>> This issue exists whether or not we have recovery.conf etc., so yes,
>> we must solve the problem.
>
> No. If we don't have recovery.conf, all parameters exist in postgresql.conf.
> The above issue would not occur unless a user makes a mistake, e.g.,
> change the value of the parameter which cannot be changed without
> the server restart.

I don't understand what you are saying. You seem to be suggesting that
it would be OK for someone to set standby_mode = on and reload the
config and for that to go completely unremarked. If we are adding new
parameters to postgresql.conf then its clear that some people will
misconfigure them and we need to have error messages for that.

If you change a parameter that only has effect during recovery then
must get an error if it is changed during normal running.

That is the reason we need to mark the recovery parameters with a new
flag, so that we can ignore any errors caused by changing them.

That has nothing at all to do with recovery.conf - you need it even if
you put everything in postgresql.conf.

>>> I think that we need to introduce new GUC flag
>>> indicating parameters are used only during recovery, and need to define
>>> recovery parameters with that flag. Whenever "pg_ctl reload" is executed,
>>> all the processes check whether recovery is in progress, and ignore
>>> silently the parameters with that flag if not. I'm not sure how easy we
>>> can implement this. In addition to introducing that flag, we might need to
>>> change some processes which cannot access to the shared memory so that
>>> they can. Otherwise, they cannot know whether recovery is in progress.
>>> Or we might need to change them so that they always ignore recovery
>>> parameters.
>>
>> The postmaster knows whether its in recovery or not without checking
>> shared memory. Various postmaster states describe this. If not
>> postmaster, other backends can run recoveryinprogress() as normal.
>
> AFAIR archiver and syslogger cannot access to the shared memory, i.e.,
> they cannot run RecoveryInProgress(). They don't use any recovery
> parameters for now, so we can change them so that they always ignore
> those parameters. Though I'm not inclined to add the process-specific code
> like the following into the GUC mechanism as much as possible.
>
>    if (I am postmaster)
>    {
>        if (recovery is NOT in progress)
>            reset the recovery parameters;
>    }
>    else if (I am archiver or syslogger)
>        /* always ignore */
>    else
>    {
>        if (recovery is NOT in progress)
>            reset the recovery parameters;
>    }
>

I don't see the problem. The code above could easily be simplified and
only needs to exist in one place, not copied around for each GUC.

When we had recovery.conf and postgresql.conf you knew which
parameters took effects at specific times. That metadata needs to be
added back into the system so we can report errors properly.

Considering the amount of code we will be removing, a couple of extra
lines seems trivial.

AFAICS we need to reset all recovery parameters at the end of recovery
anyway. Having SHOW recovery_target_xid return a value other than 0 is
not appropriate during normal running, same for standby_mode and the
other parameters.

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