Re: Is Recovery actually paused?

On Mon, 8 Feb 2021 at 6:38 AM, Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp> wrote:

> Hi,
>
> On Sun, 7 Feb 2021 19:27:02 +0530
> Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> > On Sun, Feb 7, 2021 at 6:44 PM Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > >
> > > On Fri, Feb 5, 2021 at 10:14 AM Bharath Rupireddy
> > > <bharath(dot)rupireddyforpostgres(at)gmail(dot)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