From a10dd8ba98b2c0e16b560e447336c663e7b865de Mon Sep 17 00:00:00 2001 From: David Steele Date: Wed, 5 Aug 2015 22:05:45 -0400 Subject: [PATCH] Implemented issue #121: Check data from db against what's passed to backrest. --- README.md | 4 ++ doc/doc.xml | 6 +++ lib/BackRest/Backup.pm | 9 +++-- lib/BackRest/Db.pm | 82 +++++++++++++++++++++++++++++---------- lib/BackRest/Exception.pm | 3 +- 5 files changed, 78 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index d4b6c9f78..eba4e113c 100644 --- a/README.md +++ b/README.md @@ -790,6 +790,10 @@ Get information about backups in the `db` stanza. * Now using Perl DBI for connections to PostgreSQL rather than psql. The `cmd-psql` and `cmd-psql-option` settings have been removed and replaced with `db-port` and `db-socket-path`. +* Added checks to be sure the `db-path` setting is consistent with `db-port` by comparing the `data_directory` as reported by the cluster against the `db-path` setting and the version as reported by the cluster against the value read from pg_control. The `db-socket-path` setting is checked to be sure it is an absolute path. + +* Added vagrant test configurations for Ubuntu 14.04 and CentOS 7. + ### v0.78: Remove CPAN dependencies, stability improvements * Removed dependency on CPAN packages for multi-threaded operation. While it might not be a bad idea to update the threads and Thread::Queue packages, it is no longer necessary. diff --git a/doc/doc.xml b/doc/doc.xml index cac3fca7b..0611d0078 100644 --- a/doc/doc.xml +++ b/doc/doc.xml @@ -747,6 +747,12 @@ Run a full backup on the db stanza. --type can Now using Perl DBI for connections to rather than psql. The cmd-psql and cmd-psql-option settings have been removed and replaced with db-port and db-socket-path. + + Added checks to be sure the db-path setting is consistent with db-port by comparing the data_directory as reported by the cluster against the db-path setting and the version as reported by the cluster against the value read from pg_control. The db-socket-path setting is checked to be sure it is an absolute path. + + + Added vagrant test configurations for Ubuntu 14.04 and CentOS 7. + diff --git a/lib/BackRest/Backup.pm b/lib/BackRest/Backup.pm index 7c9bbe9f0..87cff8b95 100644 --- a/lib/BackRest/Backup.pm +++ b/lib/BackRest/Backup.pm @@ -560,9 +560,9 @@ sub backup $bNoStartStop || optionGet(OPTION_BACKUP_ARCHIVE_CHECK)); # Database info - my ($strDbVersion, $iControlVersion, $iCatalogVersion, $ullDbSysId) = $oDb->info($oFile, $strDbClusterPath); + my ($fDbVersion, $iControlVersion, $iCatalogVersion, $ullDbSysId) = $oDb->info($oFile, $strDbClusterPath); - $oBackupManifest->set(MANIFEST_SECTION_BACKUP_DB, MANIFEST_KEY_DB_VERSION, undef, $strDbVersion); + $oBackupManifest->set(MANIFEST_SECTION_BACKUP_DB, MANIFEST_KEY_DB_VERSION, undef, $fDbVersion); $oBackupManifest->setNumeric(MANIFEST_SECTION_BACKUP_DB, MANIFEST_KEY_CONTROL, undef, $iControlVersion); $oBackupManifest->setNumeric(MANIFEST_SECTION_BACKUP_DB, MANIFEST_KEY_CATALOG, undef, $iCatalogVersion); $oBackupManifest->setNumeric(MANIFEST_SECTION_BACKUP_DB, MANIFEST_KEY_SYSTEM_ID, undef, $ullDbSysId); @@ -595,7 +595,8 @@ sub backup my $strTimestampDbStart; ($strArchiveStart, $strTimestampDbStart) = - $oDb->backup_start(BACKREST_EXE . ' backup started ' . timestamp_string_get(undef, $lTimestampStart), $bStartFast); + $oDb->backup_start($oFile, $strDbClusterPath, BACKREST_EXE . ' backup started ' . + timestamp_string_get(undef, $lTimestampStart), $bStartFast); $oBackupManifest->set(MANIFEST_SECTION_BACKUP, MANIFEST_KEY_ARCHIVE_START, undef, $strArchiveStart); &log(INFO, "archive start: ${strArchiveStart}"); @@ -734,7 +735,7 @@ sub backup &log(DEBUG, "retrieving archive logs ${strArchiveStart}:${strArchiveStop}"); my $oArchive = new BackRest::Archive(); my $strArchiveId = $oArchive->getCheck($oFile); - my @stryArchive = $oArchive->range($strArchiveStart, $strArchiveStop, $oDb->versionGet() < 9.3); + my @stryArchive = $oArchive->range($strArchiveStart, $strArchiveStop, $fDbVersion < 9.3); foreach my $strArchive (@stryArchive) { diff --git a/lib/BackRest/Db.pm b/lib/BackRest/Db.pm index 13b309a63..1746e0404 100644 --- a/lib/BackRest/Db.pm +++ b/lib/BackRest/Db.pm @@ -111,6 +111,14 @@ sub executeSql # Connect to the db my $strDbName = 'postgres'; my $strDbUser = getpwuid($<); + my $strDbSocketPath = optionGet(OPTION_DB_SOCKET_PATH, false); + + if (defined($strDbSocketPath) && $strDbSocketPath !~ /^\//) + { + confess &log(ERROR, "'${strDbSocketPath}' is not valid for '" . OPTION_DB_SOCKET_PATH . "' option:" . + " path must be absolute", ERROR_OPTION_INVALID_VALUE); + } + my $strDbUri = "dbi:Pg:dbname=${strDbName};port=" . optionGet(OPTION_DB_PORT) . (optionTest(OPTION_DB_SOCKET_PATH) ? ';host=' . optionGet(OPTION_DB_SOCKET_PATH) : ''); @@ -179,11 +187,20 @@ sub info logDebug(OP_DB_INFO, DEBUG_CALL, undef, {isRemote => $oFile->is_remote(PATH_DB_ABSOLUTE), dbPath => $strDbPath}); + # Return data from the cache if it exists + if (defined($self->{info}{$strDbPath})) + { + return $self->{info}{$strDbPath}{fDbVersion}, + $self->{info}{$strDbPath}{iControlVersion}, + $self->{info}{$strDbPath}{iCatalogVersion}, + $self->{info}{$strDbPath}{ullDbSysId}; + } + # Database info my $iCatalogVersion; my $iControlVersion; my $ullDbSysId; - my $strDbVersion; + my $fDbVersion; if ($oFile->is_remote(PATH_DB_ABSOLUTE)) { @@ -201,7 +218,7 @@ sub info # Split the result into return values my @stryToken = split(/\t/, $strResult); - $strDbVersion = $stryToken[0]; + $fDbVersion = $stryToken[0]; $iControlVersion = $stryToken[1]; $iCatalogVersion = $stryToken[2]; $ullDbSysId = $stryToken[3]; @@ -240,48 +257,48 @@ sub info # Make sure the control version is supported if ($iControlVersion == 942 && $iCatalogVersion == 201409291) { - $strDbVersion = '9.4'; + $fDbVersion = '9.4'; } # Leave 9.5 catalog version out until it stabilizes (then move 9.5 to the top of the list) elsif ($iControlVersion == 942) # && $iCatalogVersion == 201505311) { - $strDbVersion = '9.5'; + $fDbVersion = '9.5'; } elsif ($iControlVersion == 937 && $iCatalogVersion == 201306121) { - $strDbVersion = '9.3'; + $fDbVersion = '9.3'; } elsif ($iControlVersion == 922 && $iCatalogVersion == 201204301) { - $strDbVersion = '9.2'; + $fDbVersion = '9.2'; } elsif ($iControlVersion == 903 && $iCatalogVersion == 201105231) { - $strDbVersion = '9.1'; + $fDbVersion = '9.1'; } elsif ($iControlVersion == 903 && $iCatalogVersion == 201008051) { - $strDbVersion = '9.0'; + $fDbVersion = '9.0'; } elsif ($iControlVersion == 843 && $iCatalogVersion == 200904091) { - $strDbVersion = '8.4'; + $fDbVersion = '8.4'; } elsif ($iControlVersion == 833 && $iCatalogVersion == 200711281) { - $strDbVersion = '8.3'; + $fDbVersion = '8.3'; } elsif ($iControlVersion == 822 && $iCatalogVersion == 200611241) { - $strDbVersion = '8.2'; + $fDbVersion = '8.2'; } elsif ($iControlVersion == 812 && $iCatalogVersion == 200510211) { - $strDbVersion = '8.1'; + $fDbVersion = '8.1'; } elsif ($iControlVersion == 74 && $iCatalogVersion == 200411041) { - $strDbVersion = '8.0'; + $fDbVersion = '8.0'; } else { @@ -291,10 +308,16 @@ sub info } } - &log(DEBUG, OP_DB_INFO . "=>: dbVersion = ${strDbVersion}, controlVersion = ${iControlVersion}" . + # Store data in the cache + $self->{info}{$strDbPath}{fDbVersion} = $fDbVersion; + $self->{info}{$strDbPath}{iControlVersion} = $iControlVersion; + $self->{info}{$strDbPath}{iCatalogVersion} = $iCatalogVersion; + $self->{info}{$strDbPath}{ullDbSysId} = $ullDbSysId; + + &log(DEBUG, OP_DB_INFO . "=>: dbVersion = ${fDbVersion}, controlVersion = ${iControlVersion}" . ", catalogVersion = ${iCatalogVersion}, dbSysId = ${ullDbSysId}"); - return $strDbVersion, $iControlVersion, $iCatalogVersion, $ullDbSysId; + return $fDbVersion, $iControlVersion, $iCatalogVersion, $ullDbSysId; } #################################################################################################################################### @@ -304,13 +327,16 @@ sub versionGet { my $self = shift; - if (defined($self->{fVersion})) + # Get data from the cache if possible + if (defined($self->{fVersion}) && defined($self->{strDbPath})) { - return $self->{fVersion}; + return $self->{fVersion}, $self->{strDbPath}; } - $self->{fVersion} = - trim($self->executeSql("select (regexp_matches(split_part(version(), ' ', 2), '^[0-9]+\.[0-9]+'))[1]")); + # Get version and db-path from + ($self->{fVersion}, $self->{strDbPath}) = split("\t", trim( + $self->executeSql("select (regexp_matches(split_part(version(), ' ', 2), '^[0-9]+\.[0-9]+'))[1], setting" . + " from pg_settings where name = 'data_directory'"))); my $strVersionSupport = versionSupport(); @@ -321,7 +347,7 @@ sub versionGet logDebug(OP_DB_VERSION_GET, DEBUG_RESULT, {dbVersion => $self->{fVersion}}); - return $self->{fVersion}; + return $self->{fVersion}, $self->{strDbPath}; } #################################################################################################################################### @@ -330,10 +356,24 @@ sub versionGet sub backup_start { my $self = shift; + my $oFile = shift; + my $strDbPath = shift; my $strLabel = shift; my $bStartFast = shift; - $self->versionGet(); + # Get the version from the control file + my ($fDbVersion) = $self->info($oFile, $strDbPath); + + # Get version and db path from the database + my ($fCompareDbVersion, $strCompareDbPath) = $self->versionGet(); + + # Error if they are not identical + if (!($fDbVersion == $fCompareDbVersion && $strDbPath eq $strCompareDbPath)) + { + confess &log(ERROR, "version '${fCompareDbVersion}' and db-path '${strCompareDbPath}' queried from cluster does not match" . + " version '${fDbVersion}' and db-path '${strDbPath}' read from '${strDbPath}/global/pg_control'\n" . + "HINT: the db-path and db-port settings likely reference different clusters", ERROR_DB_MISMATCH); + } # Only allow start-fast option for version >= 8.4 if ($self->{fVersion} < 8.4 && $bStartFast) diff --git a/lib/BackRest/Exception.pm b/lib/BackRest/Exception.pm index 8ed5d8882..7d95a8fcb 100644 --- a/lib/BackRest/Exception.pm +++ b/lib/BackRest/Exception.pm @@ -47,6 +47,7 @@ use constant ERROR_FILE_MISSING => 130, ERROR_DB_CONNECT => 131, ERROR_DB_QUERY => 132, + ERROR_DB_MISMATCH => 133, ERROR_UNKNOWN => 199 }; @@ -57,7 +58,7 @@ our @EXPORT = qw(ERROR_ASSERT ERROR_CHECKSUM ERROR_CONFIG ERROR_FILE_INVALID ERR ERROR_RESTORE_PATH_NOT_EMPTY ERROR_FILE_OPEN ERROR_FILE_READ ERROR_PARAM_REQUIRED ERROR_ARCHIVE_MISMATCH ERROR_ARCHIVE_DUPLICATE ERROR_VERSION_NOT_SUPPORTED ERROR_PATH_CREATE ERROR_COMMAND_INVALID ERROR_HOST_CONNECT ERROR_UNKNOWN ERROR_LOCK_ACQUIRE ERROR_BACKUP_MISMATCH ERROR_FILE_SYNC ERROR_PATH_OPEN ERROR_PATH_SYNC - ERROR_FILE_MISSING ERROR_DB_CONNECT ERROR_DB_QUERY); + ERROR_FILE_MISSING ERROR_DB_CONNECT ERROR_DB_QUERY ERROR_DB_MISMATCH); #################################################################################################################################### # CONSTRUCTOR