On 13 December 2013 13:22, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2013-12-13 13:09:13 +0000, Simon Riggs wrote:
>> On 13 December 2013 11:58, Andres Freund <andres@2ndquadrant.com> wrote:
>> > On 2013-12-13 11:56:47 +0000, Simon Riggs wrote:
>> >> On 12 December 2013 21:58, Fabrízio de Royes Mello
>> >> <fabriziomello@gmail.com> wrote:
>> >> > Reviewing the committed patch I noted that the "CheckForStandbyTrigger()"
>> >> > after the delay was removed.
>> >> >
>> >> > If we promote the standby during the delay and don't check the trigger
>> >> > immediately after the delay, then we will replay undesired WALs records.
>> >> >
>> >> > The attached patch add this check.
>> >>
>> >> I removed it because it was after the pause. I'll replace it, but
>> >> before the pause.
>> >
>> > Doesn't after the pause make more sense? If somebody promoted while we
>> > were waiting, we want to recognize that before rolling forward? The wait
>> > can take a long while after all?
>>
>> That would change the way pause currently works, which is OOS for that patch.
>
> But this feature isn't pause itself - it's imo something
> independent. Note that we currently
> a) check pause again after recoveryApplyDelay(),
> b) do check for promotion if the sleep in recoveryApplyDelay() is
> interrupted. So not checking after the final sleep seems confusing.
I'm proposing the attached patch.
This patch implements a consistent view of recovery pause, which is
that when paused, we don't check for promotion, during or immediately
after. That is user noticeable behaviour and shouldn't be changed
without thought and discussion on a separate thread with a clear
descriptive title. (I might argue in favour of it myself, I'm not yet
decided).
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services