> On Sun, Feb 7, 2021 at 6:44 PM Bharath Rupireddy > <bharath.rupireddyforpostgres@gmail.com> wrote: > > > > On Fri, Feb 5, 2021 at 10:14 AM Bharath Rupireddy > > <bharath.rupireddyforpostgres@gmail.com> wrote: > > > > 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. > > > > Hi Dilip, I have one more question: > > > > + /* test for recovery pause, if user has requested the pause */ > > + if (((volatile XLogCtlData *) XLogCtl)->recoveryPauseState == > > + RECOVERY_PAUSE_REQUESTED) > > + recoveryPausesHere(false); > > + > > + now = GetCurrentTimestamp(); > > + > > > > Do we need now = GetCurrentTimestamp(); here? Because, I see that > > whenever the variable now is used within the for loop in > > WaitForWALToBecomeAvailable, it's re-calculated anyways. It's being > > used within case XLOG_FROM_STREAM: > > > > Am I missing something? > > Yeah, I don't see any reason for doing this, maybe it got copy pasted > by mistake. Thanks for observing this.
I also have a question:
@@ -6270,14 +6291,14 @@ RecoveryRequiresIntParameter(const char *param_name, int currValue, int minValue currValue, minValue)));
ereport(LOG, (errmsg("recovery has paused"), errdetail("If recovery is unpaused, the server will shut down."), errhint("You can then restart the server after making the necessary configuration changes.")));
- while (RecoveryIsPaused()) + while (GetRecoveryPauseState() != RECOVERY_NOT_PAUSED) { HandleStartupProcInterrupts();
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.
