Re: Is Recovery actually paused?

On Mon, 25 Jan 2021 14:53:18 +0530
Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:

> I have changed as per other functions for consistency.

Thank you for updating the patch. Here are a few comments:

(1)

- SetRecoveryPause(true);
+ SetRecoveryPause(RECOVERY_PAUSE_REQUESTED);

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();

This fix would be required for code added by the following commit.
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=15251c0a60be76eedee74ac0e94b433f9acca5af

Due to this, the recovery could get paused after the configuration
change in the primary. However, after applying this patch,
pg_is_wal_replay_paused returns "pause requested" although it should
return "paused".

To fix this, we must pass RECOVERY_PAUSED to SetRecoveryPause() instead
of RECOVERY_PAUSE_REQUESTED. Or, we can call CheckAndSetRecoveryPause()
in the loop like recoveryPausesHere(), but this seems redundant.

(2)
- while (RecoveryIsPaused())
+ while (GetRecoveryPauseState() != RECOVERY_NOT_PAUSED)
{
+
HandleStartupProcInterrupts();

Though it is trivial, I think the new line after the brace is unnecessary.

Regards,
Yugo Nagata

--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>