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>


Reply via email to