On 2020-01-28 06:01, Kyotaro Horiguchi wrote:
> Hello.
>
> At Mon, 27 Jan 2020 12:16:02 +0100, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in
>> On 2020-01-15 05:02, Kyotaro Horiguchi wrote:
>>> FWIW, I restate this (perhaps) more clearly.
>>> At Wed, 15 Jan 2020 11:02:24 +0900 (JST), Kyotaro Horiguchi
>>> <horikyota.ntt@gmail.com> wrote in
>>>> recvoery_target_* is not cleared after startup. If a server crashed
>>>> just after the last shutdown checkpoint, any recovery_target_* setting
>>>> prevents the server from starting regardless of its value.
>>> recvoery_target_* is not automatically cleared after a successful
>>> archive recovery. After that, if the server crashed just after the
>>> last shutdown checkpoint, any recovery_target_* setting prevents the
>>> server from starting regardless of its value.
>>
>> Thank you for this clarification. Here is a new patch that addresses
>> that and also the other comments raised about my previous patch.
>
> The code looks fine, renaming reachedStopPoint to
> reachedRecoveryTarget looks very nice. Doc part looks fine, too.
>
>
> PostgresNode.pm
> +Is has_restoring is used, standby mode is used by default. To use
>
> "Is has_restoring used,", or "If has_restoring is used," ?
Committed with that fix.
> The change seems aiming not to break compatibility with external test
> scripts, but it looks quite confusing to me. The problem here is both
> enable_streaming/restoring independently put trigger files, so don't
> we separte placing of trigger files out of the functions?
Yeah, this is all historically grown, but a major refactoring seems out
of scope for this thread. It seems hard to come up with a more elegant
way, since after all the underlying mechanisms are also all intertwined.
Your patch adds even more code, so I'm not sure it's an improvement.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
