On Fri, Feb 5, 2021 at 6:22 AM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
>
> On Thu, Feb 4, 2021 at 7:20 PM Dilip Kumar <dilipbal...@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.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
