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? 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);
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
