Horton
Menu

Re: Proposal for changes to recovery.conf API

Fujii Masao <masao.fujii@gmail.com>

View thread
From: Fujii Masao <masao.fujii@gmail.com>
To: Simon Riggs <simon@2ndquadrant.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Bruce Momjian <bruce@momjian.us>, Josh Berkus <josh@agliodbs.com>, Magnus Hagander <magnus@hagander.net>, Michael Paquier <michael.paquier@gmail.com>, Andres Freund <andres@anarazel.de>, Abhijit Menon-Sen <ams@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-01-11T09:51:30Z
Lists: pgsql-hackers 
On Mon, Jan 2, 2017 at 6:14 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 20 December 2016 at 15:11, Simon Riggs <simon@2ndquadrant.com> wrote:
>> On 20 December 2016 at 15:03, Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>>> API for crash recovery will never be changed. That is, crash recovery needs
>>> neither recovery.trigger nor standby.trigger. When the server starts a crash
>>> recovery without any trigger file, any recovery parameter settings in
>>> postgresql.conf are ignored. Right?
>>
>> Yes. There are no conceptual changes, just the API.
>>
>> The goals are: visibility and reloading of recovery parameters,
>> removal of special case code.
>
> OK, so here's the patch, plus doc cleanup patch.

Thanks for the patch!

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

-        if (PrimaryConnInfo == NULL && recoveryRestoreCommand == NULL)
+        if (SenderConnInfo == NULL && recoveryRestoreCommand == NULL)

Seems "SenderConnInfo == NULL" should be changed to "SenderConnInfo[0] == '\0'".
recoveryRestoreCommand, as well.

Regards,

-- 
Fujii Masao