From 5d033d028c54142c872ca4ce8cef6ac05e9ec55a Mon Sep 17 00:00:00 2001 From: David Steele Date: Wed, 28 Sep 2016 19:45:33 -0400 Subject: [PATCH] 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 | 10 ++++++++ lib/pgBackRest/Common/Exception.pm | 2 ++ lib/pgBackRest/Db.pm | 24 ++++++++++++------- test/expect/backup-full-001.log | 8 +++++++ test/lib/pgBackRestTest/Backup/BackupTest.pm | 17 +++++++++---- .../Backup/Common/HostDbTest.pm | 7 +++++- 6 files changed, 55 insertions(+), 13 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index f4cada8d2..bb8923435 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -124,8 +124,18 @@

Fixed the check command to prevent an error message from being logged if the backup directory does not exist.

+ + +

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.

+ + +

version number included in command start INFO log output.

diff --git a/lib/pgBackRest/Common/Exception.pm b/lib/pgBackRest/Common/Exception.pm index e2bed5822..99e29ce26 100644 --- a/lib/pgBackRest/Common/Exception.pm +++ b/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); diff --git a/lib/pgBackRest/Db.pm b/lib/pgBackRest/Db.pm index 5f17494a7..d5636ddeb 100644 --- a/lib/pgBackRest/Db.pm +++ b/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); } - # 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 archive checking is enabled then perform various validations + if (optionGet(OPTION_BACKUP_ARCHIVE_CHECK)) { - confess &log(ERROR, "archive_mode=always not supported", ERROR_FEATURE_NOT_SUPPORTED); - } + # 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); + } - # Check if archive_command is set - if (!optionGet(OPTION_BACKUP_STANDBY) && optionGet(OPTION_BACKUP_ARCHIVE_CHECK)) - { + # Error if archive_mode = always (support has not been added yet) + 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 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); } } diff --git a/test/expect/backup-full-001.log b/test/expect/backup-full-001.log index 6d5b81199..6bddf5c66 100644 --- a/test/expect/backup-full-001.log +++ b/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 ------------------------------------------------------------------------------------------------------------------------------------ diff --git a/test/lib/pgBackRestTest/Backup/BackupTest.pm b/test/lib/pgBackRestTest/Backup/BackupTest.pm index 9a057168a..5135e7ee4 100755 --- a/test/lib/pgBackRestTest/Backup/BackupTest.pm +++ b/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 - $oHostDbMaster->clusterStop({bIgnoreLogError => true}); + # Check ERROR_ARCHIVE_DISABLED error + $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 diff --git a/test/lib/pgBackRestTest/Backup/Common/HostDbTest.pm b/test/lib/pgBackRestTest/Backup/Common/HostDbTest.pm index 1b2178b3c..e1e390af5 100644 --- a/test/lib/pgBackRestTest/Backup/Common/HostDbTest.pm +++ b/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) {