On Mon, 8 Feb 2021 at 6:38 AM, Yugo NAGATA <nag...@sraoss.co.jp> wrote:
> Hi, > > On Sun, 7 Feb 2021 19:27:02 +0530 > Dilip Kumar <dilipbal...@gmail.com> wrote: > > > On Sun, Feb 7, 2021 at 6:44 PM Bharath Rupireddy > > <bharath.rupireddyforpostg...@gmail.com> wrote: > > > > > > On Fri, Feb 5, 2021 at 10:14 AM Bharath Rupireddy > > > <bharath.rupireddyforpostg...@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))); > > - SetRecoveryPause(true); > + SetRecoveryPause(RECOVERY_PAUSED); > > 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. > -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com