Re: Standby accepts recovery_target_timeline setting?

On 9/28/19 10:54 AM, Fujii Masao wrote:
> On Sat, Sep 28, 2019 at 2:01 AM David Steele <david@pgmasters.net> wrote:
>> On 9/27/19 11:58 AM, Fujii Masao wrote:
>>>
>>> Yes, recovery target settings are used even when neither backup_label
>>> nor recovery.signal exist, i.e., just a crash recovery, in v12. This is
>>> completely different behavior from prior versions.
>>
>> I'm not able to reproduce this.  I only see recovery settings being used
>> if backup_label, recovery.signal, or standby.signal is present.
>>
>> Do you have an example?
> 
> Yes, here is the example:
> 
> initdb -D data
> pg_ctl -D data start
> psql -c "select pg_create_restore_point('hoge')"
> psql -c "alter system set recovery_target_name to 'hoge'"
> psql -c "create table test as select num from generate_series(1, 100) num"
> pg_ctl -D data -m i stop
> pg_ctl -D data start
> 
> After restarting the server at the above final step, you will see
> the following log messages indicating that recovery stops at
> recovery_target_name.
> 
> 2019-09-28 22:42:04.849 JST [16944] LOG:  recovery stopping at restore
> point "hoge", time 2019-09-28 22:42:03.86558+09
> 2019-09-28 22:42:04.849 JST [16944] FATAL:  requested recovery stop
> point is before consistent recovery point

That's definitely not good behavior.

>>> IMO, since v12 is RC1 now, it's not good idea to change the logic to new.
>>> So at least for v12, we basically should change the recovery logic so that
>>> it behaves in the same way as prior versions. That is,
>>>
>>> - Stop the recovery with an error if any recovery target is set in
>>>    crash recovery
>>
>> This seems reasonable.  I tried adding a recovery.signal and
>> restore_command for crash recovery and I just got an error that it
>> couldn't find 00000002.history in the archive.
> 
> You added recovery.signal, so it means that you started an archive recovery,
> not crash recovery. Right?

Correct.

> Anyway I'm thinking to apply something like attached patch, to emit an error
> if recovery target is set in crash recovery.

The patch looks reasonable.

>>> - Do not enter an archive recovery mode if recovery.signal is missing
>>
>> Agreed.  Perhaps it's OK to use restore_command if a backup_label is
>> present
> 
> Yeah, it's maybe OK, but differenet behavior from current version.
> So, at least for v12, I'm inclined to prevent crash recovery with backup_label
> from using restore_command, i.e., only WAL files in pg_wal will be replayed
> in this case.

Agreed.  Seems like that could be added to the patch above easily
enough.  More checks would be needed to prevent the behaviors I've been
seeing in the other thread, but it should be possible to more or less
mimic the old behavior with sufficient checks.

Regards,
-- 
-David
david@pgmasters.net