Archive validation improvements:

* Fixed error message to properly display the archive command when an invalid archive command is detected.
* Check that archive_mode is enabled when archive-check option enabled.

doc/xml/release.xml

@@ -124,8 +124,18 @@
 
                         <p>Fixed the <cmd>check</cmd> command to prevent an error message from being logged if the backup directory does not exist.</p>
                     </release-item>
+
+                    <release-item>
+                        <p>Fixed error message to properly display the archive command when an invalid archive command is detected.</p>
+                    </release-item>
                 </release-bug-list>
 
+                <release-feature-list>
+                    <release-item>
+                        <p>Check that archive_mode is enabled when <br-option>archive-check</br-option> option enabled.</p>
+                    </release-item>
+                </release-feature-list>
+
                 <release-refactor-list>
                     <release-item>
                         <p><backrest/> version number included in command start INFO log output.</p>

lib/pgBackRest/Common/Exception.pm

@@ -142,6 +142,8 @@ use constant ERROR_PROTOCOL_OUTPUT_REQUIRED                         => ERROR_MIN
     push @EXPORT, qw(ERROR_PROTOCOL_OUTPUT_REQUIRED);
 use constant ERROR_LINK_OPEN                                        => ERROR_MINIMUM + 61;
     push @EXPORT, qw(ERROR_LINK_OPEN);
+use constant ERROR_ARCHIVE_DISABLED                                 => ERROR_MINIMUM + 62;
+    push @EXPORT, qw(ERROR_ARCHIVE_DISABLED);
 
 use constant ERROR_INVALID_VALUE                                    => ERROR_MAXIMUM - 2;
     push @EXPORT, qw(ERROR_INVALID_VALUE);

lib/pgBackRest/Db.pm

@@ -764,21 +764,29 @@ sub configValidate
             "HINT: the db-path and db-port settings likely reference different clusters", ERROR_DB_MISMATCH);
     }
 
+    # If archive checking is enabled then perform various validations
+    if (optionGet(OPTION_BACKUP_ARCHIVE_CHECK))
+    {
+        # Error if archive_mode = off since pg_start_backup () will fail
+        if ($self->executeSql('show archive_mode') eq 'off')
+        {
+            confess &log(ERROR, 'archive_mode must be enabled', ERROR_ARCHIVE_DISABLED);
+        }
+
         # Error if archive_mode = always (support has not been added yet)
-    if (optionGet(OPTION_BACKUP_ARCHIVE_CHECK) && $self->executeSql('show archive_mode') eq 'always')
+        if ($self->executeSql('show archive_mode') eq 'always')
         {
             confess &log(ERROR, "archive_mode=always not supported", ERROR_FEATURE_NOT_SUPPORTED);
         }
 
         # Check if archive_command is set
-    if (!optionGet(OPTION_BACKUP_STANDBY) && optionGet(OPTION_BACKUP_ARCHIVE_CHECK))
-    {
         my $strArchiveCommand = $self->executeSql('show archive_command');
 
         if (index($strArchiveCommand, BACKREST_EXE) == -1)
         {
             confess &log(ERROR,
-                'archive_command \'${strArchiveCommand}\' must contain \'' . BACKREST_EXE . '\'', ERROR_ARCHIVE_COMMAND_INVALID);
+                'archive_command ' . (defined($strArchiveCommand) ? "'${strArchiveCommand}'" : '[null]') . ' must contain \'' .
+                BACKREST_EXE . '\'', ERROR_ARCHIVE_COMMAND_INVALID);
         }
     }
 

test/expect/backup-full-001.log

@@ -5,6 +5,14 @@ check db - fail on missing archive.info file (db-master host)
 > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=detail --archive-timeout=0.1 --stanza=db check
 ------------------------------------------------------------------------------------------------------------------------------------
 
+full backup - fail on archive_mode=off (db-master host)
+> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --type=full --stanza=db backup
+------------------------------------------------------------------------------------------------------------------------------------
+
+check db - fail on archive_mode=off (db-master host)
+> [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --log-level-console=detail --archive-timeout=0.1 --stanza=db check
+------------------------------------------------------------------------------------------------------------------------------------
+
 full backup - fail on invalid archive_command (db-master host)
 > [CONTAINER-EXEC] db-master [BACKREST-BIN] --config=[TEST_PATH]/db-master/pgbackrest.conf --type=full --stanza=db backup
 ------------------------------------------------------------------------------------------------------------------------------------

test/lib/pgBackRestTest/Backup/BackupTest.pm

@@ -1794,15 +1794,24 @@ sub backupTestRun
                     'fail on missing archive.info file',
                     {iTimeout => 0.1, iExpectedExitStatus => ERROR_FILE_MISSING});
 
-                # Stop the cluster ignoring any errors in the postgresql log
+                # Check ERROR_ARCHIVE_DISABLED error
-                $oHostDbMaster->clusterStop({bIgnoreLogError => true});
+                $strComment = 'fail on archive_mode=off';
+                $oHostDbMaster->clusterRestart({bIgnoreLogError => true, bArchiveEnabled => false});
+
+                $oHostBackup->backup($strType, $strComment, {iExpectedExitStatus => ERROR_ARCHIVE_DISABLED});
+                $oHostDbMaster->check($strComment, {iTimeout => 0.1, iExpectedExitStatus => ERROR_ARCHIVE_DISABLED});
+
+                # If running the remote tests then also need to run check locally
+                if ($bHostBackup)
+                {
+                    $oHostBackup->check($strComment, {iTimeout => 0.1, iExpectedExitStatus => ERROR_ARCHIVE_DISABLED});
+                }
 
                 # Check ERROR_ARCHIVE_COMMAND_INVALID error
                 $strComment = 'fail on invalid archive_command';
-                $oHostDbMaster->clusterStart({bArchive => false});
+                $oHostDbMaster->clusterRestart({bIgnoreLogError => true, bArchive => false});
 
                 $oHostBackup->backup($strType, $strComment, {iExpectedExitStatus => ERROR_ARCHIVE_COMMAND_INVALID});
-
                 $oHostDbMaster->check($strComment, {iTimeout => 0.1, iExpectedExitStatus => ERROR_ARCHIVE_COMMAND_INVALID});
 
                 # If running the remote tests then also need to run check locally

test/lib/pgBackRestTest/Backup/Common/HostDbTest.pm

@@ -393,6 +393,7 @@ sub clusterStart
     my $bArchive = defined($$hParam{bArchive}) ? $$hParam{bArchive} : true;
     my $bArchiveAlways = defined($$hParam{bArchiveAlways}) ? $$hParam{bArchiveAlways} : false;
     my $bArchiveInvalid = defined($$hParam{bArchiveInvalid}) ? $$hParam{bArchiveInvalid} : false;
+    my $bArchiveEnabled = defined($$hParam{bArchiveEnabled}) ? $$hParam{bArchiveEnabled} : true;
 
     # Make sure postgres is not running
     if (-e $self->dbBasePath() . '/postmaster.pid')
@@ -410,7 +411,7 @@ sub clusterStart
         $self->dbBinPath() . '/pg_ctl start -o "-c port=' . $self->dbPort() .
         ($self->dbVersion() < PG_VERSION_95 ? ' -c checkpoint_segments=1' : '');
 
-    if ($self->dbVersion() >= PG_VERSION_83)
+    if ($bArchiveEnabled)
     {
         if ($self->dbVersion() >= PG_VERSION_95 && $bArchiveAlways)
         {
@@ -421,6 +422,10 @@ sub clusterStart
             $strCommand .= " -c archive_mode=on";
         }
     }
+    else
+    {
+        $strCommand .= " -c archive_mode=off";
+    }
 
     if ($bArchive)
     {