Re: [proposal] recovery_target "latest"

On 11/7/19 4:36 PM, Grigory Smolkin wrote:
>
> On 11/7/19 12:56 PM, Kyotaro Horiguchi wrote:
>> At Thu, 7 Nov 2019 12:22:28 +0300, Grigory Smolkin
>> <g(dot)smolkin(at)postgrespro(dot)ru> wrote in
>>> On 11/7/19 8:36 AM, Kyotaro Horiguchi wrote:
>>>> At Thu, 7 Nov 2019 02:28:39 +0300, Grigory Smolkin
>>>> <g(dot)smolkin(at)postgrespro(dot)ru> wrote in
>>>>> On 11/6/19 1:55 PM, Grigory Smolkin wrote:
>>>>>> On 11/6/19 12:56 PM, Fujii Masao wrote:
>>>>>>> What happens if this parameter is set to latest in the standby
>>>>>>> mode?
>>>>>>> Or the combination of those settings should be prohibited?
>>>>>> Currently it will behave just like regular standby, so I think, to
>>>>>> avoid confusion and keep things simple, the combination of them
>>>>>> should
>>>>>> be prohibited.
>>>>>> Thank you for pointing this out, I will work on it.
>>>>> Attached new patch revision, now it is impossible to use
>>>>> recovery_target 'latest' in standby mode.
>>>>> TAP test is updated to reflect this behavior.
>>>> In the first place, latest (or anything it could be named as) is
>>>> defined as the explit label for the default behavior. Thus the latest
>>>> should work as if nothing is set to recovery_target* following the
>>>> definition.  That might seems somewhat strange but I think at least it
>>>> is harmless.
>>>
>>> Well, it was more about getting default behavior by using some
>>> explicit recovery_target, not the other way around. Because it will
>>> break some 3rd party backup and replication applications, that may
>>> rely on old behavior of ignoring recovery_target_action when no
>>> recovery_target is provided.
>>> But you think that it is worth pursuing, I can do that.
>> Ah. Sorry for the misleading statement. What I had in my mind was
>> somewhat the mixture of them.  I thought that recovery_target =''
>> behaves the same way as now, r_t_action is ignored. And 'latest' just
>> makes recovery_target_action work as the current non-empty
>> recovery_target's does. But I'm not confident that it is a good
>> design.
>>
>>>> recovery_target=immediate + r_t_action=shutdown for a standby works as
>>>> commanded. Do we need to inhibit that, too?
>>> Why something, that work as expected, should be inhibited?
>> To make sure, I don't think we should do that. I meant by the above
>> that standby mode is already accepting recovery_target_action so
>> inhibiting that only for 'latest' is not orthogonal and could be more
>> confusing for users, and complicatig the code. So my opinion is we
>> shouldn't inhibit 'latest' unless r_t_action harms.
>
> I gave it some thought and now think that prohibiting recovery_target
> 'latest' and standby was a bad idea.
> All recovery_targets follow the same pattern of usage, so
> recovery_target "latest" also must be capable of working in standby mode.
> All recovery_targets have a clear deterministic 'target' where
> recovery should end.
> In case of recovery_target "latest" this target is the end of
> available WAL, therefore the end of available WAL must be more clearly
> defined.
> I will work on it.
>
> Thank you for a feedback.

Attached new patch revision, now end of available WAL is defined as the
fact of missing required WAL.
In case of standby, the end of WAL is defined as 2 consecutive switches
of WAL source, that didn`t provided requested record.
In case of streaming standby, each switch of WAL source is forced after
3 failed attempts to get new data from walreceiver.

All constants are taken off the top of my head and serves as proof of
concept.
TAP test is updated.

>
>
>>
>> regards.
>>
--
Grigory Smolkin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company