Re: Is Recovery actually paused?

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