Re: pause recovery if pitr target not reached

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