On Fri, Dec 13, 2013 at 11:44 AM, Simon Riggs <simon@2ndquadrant.com> wrote: > > 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). >
In my previous message [1] I attach a patch equal to your ;-)
