Fix bogus log message when starting from a cleanly shut down state.

In commit 70e81861fa to split xlog.c, I moved the startup code that
updates the state in the control file and prints out the "database
system was not properly shut down" message to the log, but I
accidentally removed the "if (InRecovery)" check around it. As a
result, that message was printed even if the system was cleanly shut
down, also during 'initdb'.

Discussion: https://www.postgresql.org/message-id/3357075.1645031062@sss.pgh.pa.us

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index d5269ede80f579f02178a9a87f77e0e5a76c263a..f9f212680b065b657e0b005b2b067ae97c6f34ed 100644 (file)
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -840,69 +840,75 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
        }
 
        /*
-        * Update pg_control to show that we are recovering and to show the
-        * selected checkpoint as the place we are starting from. We also mark
-        * pg_control with any minimum recovery stop point obtained from a backup
-        * history file.
+        * If recovery is needed, update our in-memory copy of pg_control to show
+        * that we are recovering and to show the selected checkpoint as the place
+        * we are starting from. We also mark pg_control with any minimum recovery
+        * stop point obtained from a backup history file.
+        *
+        * We don't write the changes to disk yet, though. Only do that after
+        * initializing various subsystems.
         */
-       if (InArchiveRecovery)
-       {
-               ControlFile->state = DB_IN_ARCHIVE_RECOVERY;
-       }
-       else
+       if (InRecovery)
        {
-               ereport(LOG,
-                               (errmsg("database system was not properly shut down; "
-                                               "automatic recovery in progress")));
-               if (recoveryTargetTLI > ControlFile->checkPointCopy.ThisTimeLineID)
+               if (InArchiveRecovery)
+               {
+                       ControlFile->state = DB_IN_ARCHIVE_RECOVERY;
+               }
+               else
+               {
                        ereport(LOG,
-                                       (errmsg("crash recovery starts in timeline %u "
-                                                       "and has target timeline %u",
-                                                       ControlFile->checkPointCopy.ThisTimeLineID,
-                                                       recoveryTargetTLI)));
-               ControlFile->state = DB_IN_CRASH_RECOVERY;
-       }
-       ControlFile->checkPoint = CheckPointLoc;
-       ControlFile->checkPointCopy = checkPoint;
-       if (InArchiveRecovery)
-       {
-               /* initialize minRecoveryPoint if not set yet */
-               if (ControlFile->minRecoveryPoint < checkPoint.redo)
+                                       (errmsg("database system was not properly shut down; "
+                                                       "automatic recovery in progress")));
+                       if (recoveryTargetTLI > ControlFile->checkPointCopy.ThisTimeLineID)
+                               ereport(LOG,
+                                               (errmsg("crash recovery starts in timeline %u "
+                                                               "and has target timeline %u",
+                                                               ControlFile->checkPointCopy.ThisTimeLineID,
+                                                               recoveryTargetTLI)));
+                       ControlFile->state = DB_IN_CRASH_RECOVERY;
+               }
+               ControlFile->checkPoint = CheckPointLoc;
+               ControlFile->checkPointCopy = checkPoint;
+               if (InArchiveRecovery)
                {
-                       ControlFile->minRecoveryPoint = checkPoint.redo;
-                       ControlFile->minRecoveryPointTLI = checkPoint.ThisTimeLineID;
+                       /* initialize minRecoveryPoint if not set yet */
+                       if (ControlFile->minRecoveryPoint < checkPoint.redo)
+                       {
+                               ControlFile->minRecoveryPoint = checkPoint.redo;
+                               ControlFile->minRecoveryPointTLI = checkPoint.ThisTimeLineID;
+                       }
                }
-       }
-
-       /*
-        * Set backupStartPoint if we're starting recovery from a base backup.
-        *
-        * Also set backupEndPoint and use minRecoveryPoint as the backup end
-        * location if we're starting recovery from a base backup which was taken
-        * from a standby. In this case, the database system status in pg_control
-        * must indicate that the database was already in recovery. Usually that
-        * will be DB_IN_ARCHIVE_RECOVERY but also can be
-        * DB_SHUTDOWNED_IN_RECOVERY if recovery previously was interrupted before
-        * reaching this point; e.g. because restore_command or primary_conninfo
-        * were faulty.
-        *
-        * Any other state indicates that the backup somehow became corrupted and
-        * we can't sensibly continue with recovery.
-        */
-       if (haveBackupLabel)
-       {
-               ControlFile->backupStartPoint = checkPoint.redo;
-               ControlFile->backupEndRequired = backupEndRequired;
 
-               if (backupFromStandby)
+               /*
+                * Set backupStartPoint if we're starting recovery from a base backup.
+                *
+                * Also set backupEndPoint and use minRecoveryPoint as the backup end
+                * location if we're starting recovery from a base backup which was
+                * taken from a standby. In this case, the database system status in
+                * pg_control must indicate that the database was already in recovery.
+                * Usually that will be DB_IN_ARCHIVE_RECOVERY but also can be
+                * DB_SHUTDOWNED_IN_RECOVERY if recovery previously was interrupted
+                * before reaching this point; e.g. because restore_command or
+                * primary_conninfo were faulty.
+                *
+                * Any other state indicates that the backup somehow became corrupted
+                * and we can't sensibly continue with recovery.
+                */
+               if (haveBackupLabel)
                {
-                       if (dbstate_at_startup != DB_IN_ARCHIVE_RECOVERY &&
-                               dbstate_at_startup != DB_SHUTDOWNED_IN_RECOVERY)
-                               ereport(FATAL,
-                                               (errmsg("backup_label contains data inconsistent with control file"),
-                                                errhint("This means that the backup is corrupted and you will "
-                                                                "have to use another backup for recovery.")));
-                       ControlFile->backupEndPoint = ControlFile->minRecoveryPoint;
+                       ControlFile->backupStartPoint = checkPoint.redo;
+                       ControlFile->backupEndRequired = backupEndRequired;
+
+                       if (backupFromStandby)
+                       {
+                               if (dbstate_at_startup != DB_IN_ARCHIVE_RECOVERY &&
+                                       dbstate_at_startup != DB_SHUTDOWNED_IN_RECOVERY)
+                                       ereport(FATAL,
+                                                       (errmsg("backup_label contains data inconsistent with control file"),
+                                                        errhint("This means that the backup is corrupted and you will "
+                                                                        "have to use another backup for recovery.")));
+                               ControlFile->backupEndPoint = ControlFile->minRecoveryPoint;
+                       }
                }
        }