On Fri, Feb 5, 2021 at 10:06 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Fri, Feb 5, 2021 at 6:22 AM Bharath Rupireddy
> <bharath.rupireddyforpostgres@gmail.com> wrote:
> >
> > On Thu, Feb 4, 2021 at 7:20 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > How can we do that this is not a 1 byte flag this is enum so I don't
> > > > think we can read any atomic state without a spin lock here.
> > >
> > > I have fixed the other comments and the updated patch is attached.
> >
> > Can we just do like below so that we could use the existing
> > SetRecoveryPause instead of setting the state outside?
> >
> > /* loop until recoveryPauseState is set to RECOVERY_NOT_PAUSED */
> > while (1)
> > {
> > RecoveryPauseState state;
> >
> > state = GetRecoveryPauseState();
> >
> > if (state == RECOVERY_NOT_PAUSED)
> > break;
> >
> > HandleStartupProcInterrupts();
> >
> > if (CheckForStandbyTrigger())
> > return;
> > pgstat_report_wait_start(WAIT_EVENT_RECOVERY_PAUSE);
> >
> > /*
> > * If recovery pause is requested then set it paused. While we are in
> > * the loop, user might resume and pause again so set this every time.
> > */
> > if (state == RECOVERY_PAUSE_REQUESTED)
> > SetRecoveryPause(RECOVERY_PAUSED)
>
> We can not do that, basically, under one lock we need to check the
> state and set it to pause. Because by the time you release the lock
> someone might set it to RECOVERY_NOT_PAUSED then you don't want to set
> it to RECOVERY_PAUSED.
Got it. Thanks.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
