On Mon, Feb 8, 2021 at 9:49 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Mon, Feb 8, 2021 at 9:35 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > > > If a user call pg_wal_replay_pause while waiting in
> > > > > RecoveryRequiresIntParameter,
> > > > > the state become 'pause requested' and this never returns to 'paused'.
> > > > > Should we check recoveryPauseState in this loop as in
> > > >
> > > >
> > > > I think the right fix should be that the state should never go from
> > > > ‘paused’ to ‘pause requested’ so I think pg_wal_replay_pause should take
> > > > care of that.
> > >
> > > It makes sense to take care of this in pg_wal_replay_pause, but I wonder
> > > it can not handle the case that a user resume and pause again while a sleep.
> >
> > Right, we will have to check and set in the loop. But we should not
> > allow the state to go from paused to pause requested irrespective of
> > this.
>
> We can think of a state machine with the states "not paused", "pause
> requested", "paused". While we can go to "not paused" from any state,
> but cannot go to "pause requested" from "paused".
>
> So, will pg_wal_replay_pause throw an error or warning or silently
> return when it's called and the state is "paused" already?
It should just silently return because pg_wal_replay_pause just claim
it request to pause, but it not mean that it can not pause
immediately.
Maybe we
> should add better commenting in pg_wal_replay_pause why we don't set
> "pause requested" when the state is already "paused".
> And also, if we are adding below code in the
> RecoveryRequiresIntParameter loop, it's better to make it a function,
> like your earlier patch.
>
> /*
> * 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.
> */
> SpinLockAcquire(&XLogCtl->info_lck);
> if (XLogCtl->recoveryPauseState == RECOVERY_PAUSE_REQUESTED)
> XLogCtl->recoveryPauseState = RECOVERY_PAUSED;
> SpinLockRelease(&XLogCtl->info_lck);
Yes, it should go back to function now as in the older versions.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com