On Mon, Feb 8, 2021 at 9:49 AM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Mon, Feb 8, 2021 at 9:35 AM Dilip Kumar <dilipbal...@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
