From 2310e423e98259838826ccc8093e43668fab061a Mon Sep 17 00:00:00 2001 From: David Steele Date: Tue, 27 Jun 2017 16:47:40 -0400 Subject: [PATCH] =?UTF-8?q?Fixed=20an=20issue=20that=20prevented=20tablesp?= =?UTF-8?q?aces=20from=20being=20backed=20up=20on=20PostgreSQL=20=E2=89=A4?= =?UTF-8?q?=208.4.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The integration tests that were supposed to prevent this regression did not work as intended. They verified the contents of a table in the (supposedly) restored tablespace, deleted the table, and then deleted the tablespace. All of this was deemed sufficient to prove that the tablespace had been restored correctly and was valid. However, PostgreSQL will happily recreate a tablespace on the basis of a single full-page write, at least in the affected versions. Since writes to the test table were replayed from WAL with each recovery, all the tests passed even though the tablespace was missing after the restore. The tests have been updated to include direct comparisons against the file system and a new table that is not replayed after a restore because it is created before the backup and never modified again. Versions ≥ 9.0 were not affected due to numerous synthetic integration tests that verify backups and restores file by file. --- doc/xml/release.xml | 6 +++ lib/pgBackRest/Manifest.pm | 7 ++- .../pgBackRestTest/Module/Real/RealAllTest.pm | 46 ++++++++++++++++++- 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 71adc3e83..bfc7c4de8 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -165,6 +165,8 @@ +

IMPORTANT NOTE: 8.3 and 8.4 installations utilizing tablespaces should upgrade immediately from any 1.XX release and run a full backup. A bug prevented tablespaces from being backed up on these versions only. ≥ 9.0 is not affected.

+ @@ -173,6 +175,10 @@

Fixed missing flag in C library build that resulted in a mismatched binary on 32-bit systems.

 + + +

Fixed an issue that prevented tablespaces from being backed up on ≤ 8.4.

+ diff --git a/lib/pgBackRest/Manifest.pm b/lib/pgBackRest/Manifest.pm index 2342fa4d3..d9e60e734 100644 --- a/lib/pgBackRest/Manifest.pm +++ b/lib/pgBackRest/Manifest.pm @@ -800,7 +800,12 @@ sub build if ($bTablespace) { - $strFilter = $self->tablespacePathGet(); + # Only versions >= 9.0 have the special top-level tablespace path. Below 9.0 the database files are stored + # directly in the path referenced by the symlink. + if ($strDbVersion >= PG_VERSION_90) + { + $strFilter = $self->tablespacePathGet(); + } $self->set(MANIFEST_SECTION_TARGET_PATH, MANIFEST_TARGET_PGTBLSPC, undef, $self->get(MANIFEST_SECTION_TARGET_PATH, MANIFEST_TARGET_PGDATA)); diff --git a/test/lib/pgBackRestTest/Module/Real/RealAllTest.pm b/test/lib/pgBackRestTest/Module/Real/RealAllTest.pm index c6fc73b64..b5fb0d0cc 100644 --- a/test/lib/pgBackRestTest/Module/Real/RealAllTest.pm +++ b/test/lib/pgBackRestTest/Module/Real/RealAllTest.pm @@ -529,7 +529,10 @@ sub run $oHostDbMaster->sqlExecute( "create tablespace ts1 location '" . $oHostDbMaster->tablespacePath(1) . "'", {bAutoCommit => true}); - $oHostDbMaster->sqlExecute("alter table test set tablespace ts1", {bCheckPoint => true}); + $oHostDbMaster->sqlExecute("alter table test set tablespace ts1"); + + # Create a table in the tablespace that will not be modified again to be sure it does get full page writes in the WAL later + $oHostDbMaster->sqlExecute("create table test_exists (id int) tablespace ts1", {bCommit => true, bCheckPoint => true}); # Create a table in the tablespace $oHostDbMaster->sqlExecute("create table test_remove (id int)"); @@ -681,6 +684,47 @@ sub run $oHostDbMaster->clusterStart(); $oHostDbMaster->sqlSelectOneTest('select message from test', $bTestLocal ? $strNameMessage : $strIncrMessage); + # The tablespace path should exist and have files in it + my $strTablespacePath = $oHostDbMaster->tablespacePath(1); + + # Version <= 8.4 always places a PG_VERSION file in the tablespace + if ($oHostDbMaster->pgVersion() <= PG_VERSION_84) + { + if (!storageDb()->exists("${strTablespacePath}/" . DB_FILE_PGVERSION)) + { + confess &log(ASSERT, "unable to find '" . DB_FILE_PGVERSION . "' in tablespace path '${strTablespacePath}'"); + } + } + # Version >= 9.0 creates a special path using the version and catalog number + else + { + # Backup info will have the catalog number + my $oBackupInfo = new pgBackRest::Common::Ini( + storageRepo()->pathGet(STORAGE_REPO_BACKUP . qw{/} . FILE_BACKUP_INFO), + {bLoad => false, strContent => ${storageRepo()->get(STORAGE_REPO_BACKUP . qw{/} . FILE_BACKUP_INFO)}}); + + # Construct the special path + $strTablespacePath .= + '/PG_' . $oHostDbMaster->pgVersion() . qw{_} . $oBackupInfo->get(INFO_BACKUP_SECTION_DB, INFO_BACKUP_KEY_CATALOG); + + # Check that path exists + if (!storageDb()->pathExists($strTablespacePath)) + { + confess &log(ASSERT, "unable to find tablespace path '${strTablespacePath}'"); + } + } + + # Make sure there are some files in the tablespace path (exclude PG_VERSION if <= 8.4 since that was tested above) + if (grep(!/^PG\_VERSION$/i, storageDb()->list($strTablespacePath)) == 0) + { + confess &log(ASSERT, "no files found in tablespace path '${strTablespacePath}'"); + } + + # This table should exist to prove that the tablespace was restored. It has not been updated since it was created so it + # should not be created by any full page writes. Once it is verified to exist it can be dropped. + $oHostDbMaster->sqlSelectOneTest("select count(*) from test_exists", 0); + $oHostDbMaster->sqlExecute('drop table test_exists'); + # Now it should be OK to drop database test2 if ($bTestLocal) {