Re: Is Recovery actually paused?

Yugo Nagata <nagata@sraoss.co.jp>

From: Yugo NAGATA <nagata@sraoss.co.jp>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Simon Riggs <simon@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-02-08T01:06:46Z
Lists: pgsql-hackers
Hi,

On Sun, 7 Feb 2021 19:27:02 +0530
Dilip Kumar <dilipbalaut@gmail.com> wrote:

> 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)));
 
-			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 recoveryPausesHere?


Regards,
Yugo Nagata

-- 
Yugo NAGATA <nagata@sraoss.co.jp>



Commits

  1. Be clear about whether a recovery pause has taken effect.

  2. Pause recovery for insufficient parameter settings

  3. Remove most volatile qualifiers from xlog.c