Обсуждение: replay pause vs. standby promotion

Hi,

Currently if pg_wal_replay_pause() is called after the standby
promotion is triggerred before the promotion has successfully
finished, WAL replay is paused. That is, the replay pause is
preferred than the promotion. Is this desiderable behavior?

ISTM that most users including me want the recovery to complete
as soon as possible and the server to become the master when
they requeste the promotion. So I'm thinking to change
the recovery so that it ignore the pause request after the promotion
is triggerred. Thought?

I want to start this discussion because this is related to the patch
(propoesd at the thread [1]) that I'm reviewing. It does that partially,
i.e., prefers the promotion only when the pause is requested by
recovery_target_action=pause. But I think that it's reasonable and
more consistent to do that whether whichever the pause is requested
by pg_wal_replay_pause() or recovery_target_action.

BTW, regarding "replay pause vs. delayed standby", any wait by
recovery_min_apply_delay doesn't happen after the promotion
is triggerred. IMO "pause" should be treated as the similar.

[1]
https://www.postgresql.org/message-id/flat/19168211580382043@myt5-b646bde4b8f3.qloud-c.yandex.net

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

Hello

> I want to start this discussion because this is related to the patch
> (propoesd at the thread [1]) that I'm reviewing. It does that partially,
> i.e., prefers the promotion only when the pause is requested by
> recovery_target_action=pause. But I think that it's reasonable and
> more consistent to do that whether whichever the pause is requested
> by pg_wal_replay_pause() or recovery_target_action.

+1.
I'm just not sure if this is safe for replay logic, so I did not touch this behavior in my proposal. (hmm, I wanted to
mentionthis, but apparently forgot)
 

regards, Sergei

On Wed, 04 Mar 2020 15:00:54 +0300
Sergei Kornilov <sk@zsrv.org> wrote:

> Hello
> 
> > I want to start this discussion because this is related to the patch
> > (propoesd at the thread [1]) that I'm reviewing. It does that partially,
> > i.e., prefers the promotion only when the pause is requested by
> > recovery_target_action=pause. But I think that it's reasonable and
> > more consistent to do that whether whichever the pause is requested
> > by pg_wal_replay_pause() or recovery_target_action.  
> 
> +1.

+1

And pg_wal_replay_pause () should probably raise an error explaining the
standby ignores the pause because of ongoing promotion.

On 2020/03/04 23:40, Jehan-Guillaume de Rorthais wrote:
> On Wed, 04 Mar 2020 15:00:54 +0300
> Sergei Kornilov <sk@zsrv.org> wrote:
> 
>> Hello
>>
>>> I want to start this discussion because this is related to the patch
>>> (propoesd at the thread [1]) that I'm reviewing. It does that partially,
>>> i.e., prefers the promotion only when the pause is requested by
>>> recovery_target_action=pause. But I think that it's reasonable and
>>> more consistent to do that whether whichever the pause is requested
>>> by pg_wal_replay_pause() or recovery_target_action.
>>
>> +1.
> 
> +1
> 
> And pg_wal_replay_pause () should probably raise an error explaining the
> standby ignores the pause because of ongoing promotion.

OK, so patch attached.

This patch causes, if a promotion is triggered while recovery is paused,
the paused state to end and a promotion to continue. OTOH, this patch
makes pg_wal_replay_pause() and _resume() throw an error if it's executed
while a promotion is ongoing.

Regarding recovery_target_action, if the recovery target is reached
while a promotion is ongoing, "pause" setting will act the same as "promote",
i.e., recovery will finish and the server will start to accept connections.

To implement the above, I added new shared varible indicating whether
a promotion is triggered or not. Only startup process can update this shared
variable. Other processes like read-only backends can check whether
promotion is ongoing, via this variable.

I added new function PromoteIsTriggered() that returns true if a promotion
is triggered. Since the name of this function and the existing function
IsPromoteTriggered() are confusingly similar, I changed the name of
IsPromoteTriggered() to IsPromoteSignaled, as more appropriate name.

I'd like to apply the change of log message that Sergei proposed at [1]
after commiting this patch if it's ok.

[1]
https://www.postgresql.org/message-id/flat/19168211580382043@myt5-b646bde4b8f3.qloud-c.yandex.net

Regards,


-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

On 2020/03/20 15:22, Atsushi Torikoshi wrote:
> 
> On Fri, Mar 6, 2020 at 10:18 PM Fujii Masao <masao.fujii@oss.nttdata.com <mailto:masao.fujii@oss.nttdata.com>>
wrote:
> 
> 
>     OK, so patch attached.
> 
>     This patch causes, if a promotion is triggered while recovery is paused,
>     the paused state to end and a promotion to continue. OTOH, this patch
>     makes pg_wal_replay_pause() and _resume() throw an error if it's executed
>     while a promotion is ongoing. 
> 
>     Regarding recovery_target_action, if the recovery target is reached
>     while a promotion is ongoing, "pause" setting will act the same as "promote",
>     i.e., recovery will finish and the server will start to accept connections.
> 
>     To implement the above, I added new shared varible indicating whether
>     a promotion is triggered or not. Only startup process can update this shared
>     variable. Other processes like read-only backends can check whether
>     promotion is ongoing, via this variable.
> 
>     I added new function PromoteIsTriggered() that returns true if a promotion
>     is triggered. Since the name of this function and the existing function
>     IsPromoteTriggered() are confusingly similar, I changed the name of
>     IsPromoteTriggered() to IsPromoteSignaled, as more appropriate name.
> 
> 
> I've confirmed the patch works as you described above.
> And I also poked around it a little bit but found no problems.

Thanks for the review!
Barrying any objection, I will commit the patch.

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

Hello

(I am trying to find an opportunity to review this patch...)

Consider test case with streaming replication:

on primary: create table foo (i int);
on standby:

postgres=# select pg_wal_replay_pause();
 pg_wal_replay_pause 
---------------------
 
(1 row)

postgres=# select pg_is_wal_replay_paused();
 pg_is_wal_replay_paused 
-------------------------
 t
(1 row)

postgres=# table foo;
 i 
---
(0 rows)

Execute "insert into foo values (1);" on primary

postgres=# select pg_promote ();
 pg_promote 
------------
 t
(1 row)

postgres=# table foo;
 i 
---
 1

And we did replay one additional change during promote. I think this is wrong behavior. Possible can be fixed by

+    if (PromoteIsTriggered()) break;
    /* Setup error traceback support for ereport() */
    errcallback.callback = rm_redo_error_callback;

regards, Sergei

On 2020/03/23 22:46, Sergei Kornilov wrote:
> Hello
> 
> (I am trying to find an opportunity to review this patch...)

Thanks for the review! It's really helpful!


> Consider test case with streaming replication:
> 
> on primary: create table foo (i int);
> on standby:
> 
> postgres=# select pg_wal_replay_pause();
>   pg_wal_replay_pause
> ---------------------
>   
> (1 row)
> 
> postgres=# select pg_is_wal_replay_paused();
>   pg_is_wal_replay_paused
> -------------------------
>   t
> (1 row)
> 
> postgres=# table foo;
>   i
> ---
> (0 rows)
> 
> Execute "insert into foo values (1);" on primary
> 
> postgres=# select pg_promote ();
>   pg_promote
> ------------
>   t
> (1 row)
> 
> postgres=# table foo;
>   i
> ---
>   1
> 
> And we did replay one additional change during promote. I think this is wrong behavior. Possible can be fixed by
> 
> +    if (PromoteIsTriggered()) break;
>      /* Setup error traceback support for ereport() */
>      errcallback.callback = rm_redo_error_callback;

You meant that the promotion request should cause the recovery
to finish immediately even if there are still outstanding WAL records,
and cause the standby to become the master? I don't think that
it's the expected (also existing) behavior of the promotion. That is,
the promotion request should cause the recovery to replay as much
WAL records as possible, to the end, in order to avoid data loss. No?

If we would like to have the promotion method to finish recovery
immediately, IMO we should implement something like
"pg_ctl promote -m fast". That is, we need to add new method into
the promotion.

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

On Mon, Mar 23, 2020 at 10:36 AM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:
> If we would like to have the promotion method to finish recovery
> immediately, IMO we should implement something like
> "pg_ctl promote -m fast". That is, we need to add new method into
> the promotion.

I think 'immediate' would be a better choice. One reason is that we've
used the term 'fast promotion' in the past for a different feature.
Another is that 'immediate' might sound slightly scary to people who
are familiar with what 'pg_ctl stop -mimmediate' does. And you want
people doing this to be just a little bit scared: not too scared, but
a little scared.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hello

> You meant that the promotion request should cause the recovery
> to finish immediately even if there are still outstanding WAL records,
> and cause the standby to become the master?

Oh, I get your point. But yes, I expect that in case of promotion request during a pause, the user (me too) will want
tohave exactly the current state, not latest available in WALs.
 

Real usercase from my experience:
The user wants to update a third-party application. In case of problems, he wants to return to the old version of the
applicationand the unchanged replica. Thus, it sets a pause on standby and performs an update. If all is ok - he will
resumereplay. In case of some problems he plans to promote standby.
 
But oops, standby will ignore promote signals during pause and we need get currect LSN from standby and restart it with
recovery_target_lsn= ? and recovery_target_action = promote to achieve this state.
 

regards, Sergei

On 2020/03/23 23:55, Robert Haas wrote:
> On Mon, Mar 23, 2020 at 10:36 AM Fujii Masao
> <masao.fujii@oss.nttdata.com> wrote:
>> If we would like to have the promotion method to finish recovery
>> immediately, IMO we should implement something like
>> "pg_ctl promote -m fast". That is, we need to add new method into
>> the promotion.
> 
> I think 'immediate' would be a better choice. One reason is that we've
> used the term 'fast promotion' in the past for a different feature.
> Another is that 'immediate' might sound slightly scary to people who
> are familiar with what 'pg_ctl stop -mimmediate' does. And you want
> people doing this to be just a little bit scared: not too scared, but
> a little scared.

+1

When I proposed the feature five years before, I used "immediate"
as the option value.
https://postgr.es/m/CAHGQGwHtvyDqKZaYWYA9zyyLEcAKiF5P0KpcpuNE_tsrGTFtQw@mail.gmail.com

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

On 2020/03/24 0:17, Sergei Kornilov wrote:
> Hello
> 
>> You meant that the promotion request should cause the recovery
>> to finish immediately even if there are still outstanding WAL records,
>> and cause the standby to become the master?
> 
> Oh, I get your point. But yes, I expect that in case of promotion request during a pause, the user (me too) will want
tohave exactly the current state, not latest available in WALs.
 

Basically I'd like the promotion to make the standby replay all the WAL
even if it's requested during pause state. OTOH I understand there
are use cases where immediate promotion is useful, as you explained.
So, +1 to add something like "pg_ctl promote -m immediate".

But I'm afraid that now it's too late to add such feature into v13.
Probably it's an item for v14....

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

On 2020/03/24 0:57, Fujii Masao wrote:
> 
> 
> On 2020/03/24 0:17, Sergei Kornilov wrote:
>> Hello
>>
>>> You meant that the promotion request should cause the recovery
>>> to finish immediately even if there are still outstanding WAL records,
>>> and cause the standby to become the master?
>>
>> Oh, I get your point. But yes, I expect that in case of promotion request during a pause, the user (me too) will
wantto have exactly the current state, not latest available in WALs.
 
> 
> Basically I'd like the promotion to make the standby replay all the WAL
> even if it's requested during pause state. OTOH I understand there
> are use cases where immediate promotion is useful, as you explained.
> So, +1 to add something like "pg_ctl promote -m immediate".
> 
> But I'm afraid that now it's too late to add such feature into v13.
> Probably it's an item for v14....

I pushed the latest version of the patch. If you have further opinion
about immediate promotion, let's keep discussing that!

Also we need to go back to the original patch posted at [1].

[1]
https://www.postgresql.org/message-id/flat/19168211580382043@myt5-b646bde4b8f3.qloud-c.yandex.net

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

Hello

> I pushed the latest version of the patch. If you have further opinion
> about immediate promotion, let's keep discussing that!

Thank you!

Honestly, I forgot that the promotion is documented in high-availability.sgml as:

> Before failover, any WAL immediately available in the archive or in pg_wal will be
> restored, but no attempt is made to connect to the master.

I mistakenly thought that promote should be "immediately"...

> If a promotion is triggered while recovery is paused, the paused state ends and a promotion continues.

Could we add a few words in func.sgml to clarify the behavior? Especially for users from my example above. Something
like:

> If a promotion is triggered while recovery is paused, the paused state ends, replay of any WAL immediately available
inthe archive or in pg_wal will be continued and then a promotion will be completed.
 

regards, Sergei

On 2020/03/25 0:17, Sergei Kornilov wrote:
> Hello
> 
>> I pushed the latest version of the patch. If you have further opinion
>> about immediate promotion, let's keep discussing that!
> 
> Thank you!
> 
> Honestly, I forgot that the promotion is documented in high-availability.sgml as:
> 
>> Before failover, any WAL immediately available in the archive or in pg_wal will be
>> restored, but no attempt is made to connect to the master.
> 
> I mistakenly thought that promote should be "immediately"...
> 
>> If a promotion is triggered while recovery is paused, the paused state ends and a promotion continues.
> 
> Could we add a few words in func.sgml to clarify the behavior? Especially for users from my example above. Something
like:
> 
>> If a promotion is triggered while recovery is paused, the paused state ends, replay of any WAL immediately available
inthe archive or in pg_wal will be continued and then a promotion will be completed.
 

This description is true if pause is requested by pg_wal_replay_pause(),
but not if recovery target is reached and pause is requested by
recovery_target_action=pause. In the latter case, even if there are WAL data
avaiable in pg_wal or archive, they are not replayed, i.e., the promotion
completes immediately. Probably we should document those two cases
explicitly to avoid the confusion about a promotion and recovery pause?

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

Hi

>>  Could we add a few words in func.sgml to clarify the behavior? Especially for users from my example above.
Somethinglike:
 
>>
>>>  If a promotion is triggered while recovery is paused, the paused state ends, replay of any WAL immediately
availablein the archive or in pg_wal will be continued and then a promotion will be completed.
 
>
> This description is true if pause is requested by pg_wal_replay_pause(),
> but not if recovery target is reached and pause is requested by
> recovery_target_action=pause. In the latter case, even if there are WAL data
> avaiable in pg_wal or archive, they are not replayed, i.e., the promotion
> completes immediately. Probably we should document those two cases
> explicitly to avoid the confusion about a promotion and recovery pause?

This is description for pg_wal_replay_pause, but actually we suggest to call pg_wal_replay_resume in
recovery_target_action=pause...So, I agree, we need to document both cases.
 

PS: I think we have inconsistent behavior here... Read wal during promotion from local pg_wal AND call restore_command,
butignore walreceiver also seems strange for my DBA hat...
 

regards, Sergei

On 2020/03/25 19:42, Sergei Kornilov wrote:
> Hi
> 
>>>   Could we add a few words in func.sgml to clarify the behavior? Especially for users from my example above.
Somethinglike:
 
>>>
>>>>   If a promotion is triggered while recovery is paused, the paused state ends, replay of any WAL immediately
availablein the archive or in pg_wal will be continued and then a promotion will be completed.
 
>>
>> This description is true if pause is requested by pg_wal_replay_pause(),
>> but not if recovery target is reached and pause is requested by
>> recovery_target_action=pause. In the latter case, even if there are WAL data
>> avaiable in pg_wal or archive, they are not replayed, i.e., the promotion
>> completes immediately. Probably we should document those two cases
>> explicitly to avoid the confusion about a promotion and recovery pause?
> 
> This is description for pg_wal_replay_pause, but actually we suggest to call pg_wal_replay_resume in
recovery_target_action=pause...So, I agree, we need to document both cases.
 
> 
> PS: I think we have inconsistent behavior here... Read wal during promotion from local pg_wal AND call
restore_command,but ignore walreceiver also seems strange for my DBA hat...
 

If we don't ignore walreceiver and does try to connect to the master,
a promotion and recovery cannot end forever since new WAL data can
be streamed. You think this behavior is more consistent?

IMO it's valid to replay all the WAL data available to avoid data loss
before a promotion completes.

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters

Hello

> If we don't ignore walreceiver and does try to connect to the master,
> a promotion and recovery cannot end forever since new WAL data can
> be streamed. You think this behavior is more consistent?

We have no simple point to stop replay.
Well, except for "immediately" - just one easy stop. But I agree that this is not the best option. Simple and clear,
butnot best one for data when we want to replay as much as possible from archive.
 

> IMO it's valid to replay all the WAL data available to avoid data loss
> before a promotion completes.

But in case of still working primary (with archive_command) we choose quite random time to promote. A random time when
theprimary did not save the new wal segment.
 
or even when a temporary error of restore_command occurs? We mention just cp command in docs. I know users uses cp
(e.g.from NFS) without further error handling.
 

regards, Sergei