On Mon, 25 Jan 2021 14:53:18 +0530 Dilip Kumar <dilipbal...@gmail.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 <nag...@sraoss.co.jp>