Re: Proposal for changes to recovery.conf API

On 12 January 2017 at 05:49, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Wed, Jan 11, 2017 at 7:36 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> On 11 January 2017 at 09:51, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>>>> 5. recovery.conf parameters are all moved to postgresql.conf, with these changes
>>>
>>> In current design of the patch, when recovery parameters are misconfigured
>>> (e.g., set recovery_target_timeline to invalid timeline id) and
>>> the configuration file is reloaded, the startup process emits FATAL error and
>>> the server goes down. I don't think this is fine. Basically even
>>> misconfiguration of the parameters should not cause the server crash.
>>> If invalid settings are supplied, I think that we just should warn them
>>> and ignore those new settings, like current other GUC is. Thought?
>>
>> Thanks for your comments.
>>
>> The specification of the recovery target parameters should be different, IMHO.
>>
>> If the user is performing a recovery and the target of the recovery is
>> incorrectly specified then it is clear that the recovery cannot
>> continue with an imprecisely specified target.
>
> Could you tell me what case of "the target of the recovery is incorrectly
> specified" are you thinking of? For example, you're thinking the case where
> invalid format of timestamp value is specified in recovery_target_time?
> In this case, I think that we should issue a WARNING and ignore that invalid
> setting when the configuration file is reloaded. Of course, if it's the server
> startup time, such invalid setting should prevent the server from starting up.
>
> Regarding recovery_target_timeline, isn't it better to mark that parameter
> as PGC_POSTMASTER? I'm not sure if we really want to change the recovery
> target timeline without server restart and also not sure if that operation is
> safe.

It's one of the main objectives of the patch to allow user to reset
parameters online so I don't think we should set PGC_POSTMASTER.

Of course, if the user provides an incorrect parameter value for
search, we have no way to understand their intentions. But we do know
they wanted to reset the parameter, so we should not continue using
the old parameter.

I understand your wish to throw a WARNING and "stay up", but in the
context of a recovery the whole purpose of starting the server is to
recover. If we are unable to do that because the user has failed to
set the parameter correctly then we should throw ERROR.

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