Skip to content

Commit

Permalink
Include recreated system databases during selective restore.
Browse files Browse the repository at this point in the history 
Some standard system databases (e.g. postgres) may be recreated by the user and have an OID that makes them look like user databases.

Identify the standard three system databases (template0, template1, postgres) and restore them non-zeroed no matter what OID they have.
  • Loading branch information
@pgstef
pgstef committed Mar 15, 2021
1 parent c0283ee commit 6942ff5
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 16 deletions.
9 changes: 9 additions & 0 deletions doc/xml/release.xml
View file
Expand Up @@ -81,6 +81,15 @@
</release-feature-list>

<release-improvement-list>
<release-item>
<release-item-contributor-list>
<release-item-contributor id="stefan.fercot"/>
<release-item-reviewer id="cynthia.shang"/>
</release-item-contributor-list>

<p>Include recreated system databases during selective restore.</p>
</release-item>

<release-item>
<release-item-contributor-list>
<release-item-reviewer id="cynthia.shang"/>
Expand Down
47 changes: 39 additions & 8 deletions src/command/restore/restore.c
View file
Expand Up @@ -1300,15 +1300,40 @@ restoreSelectiveExpression(Manifest *manifest)
strNewFmt("^" MANIFEST_TARGET_PGTBLSPC "/[0-9]+/%s/[0-9]+/" PG_FILE_PGVERSION, strZ(tablespaceId)));
}

// Generate a list of databases in base or in a tablespace
// Generate a list of databases in base or in a tablespace and get all standard system databases, even in cases where
// users have recreated them
StringList *systemDbIdList = strLstNew();
StringList *dbList = strLstNew();

for (unsigned int systemDbIdx = 0; systemDbIdx < manifestDbTotal(manifest); systemDbIdx++)
{
const ManifestDb *systemDb = manifestDb(manifest, systemDbIdx);

if (strEqZ(systemDb->name, "template0") || strEqZ(systemDb->name, "template1") ||
strEqZ(systemDb->name, "postgres") || systemDb->id < PG_USER_OBJECT_MIN_ID)
{
// Build the system id list and add to the dbList for logging and checking
const String *systemDbId = varStrForce(VARUINT(systemDb->id));
strLstAdd(systemDbIdList, systemDbId);
strLstAdd(dbList, systemDbId);
}
}

for (unsigned int fileIdx = 0; fileIdx < manifestFileTotal(manifest); fileIdx++)
{
const ManifestFile *file = manifestFile(manifest, fileIdx);

if (regExpMatch(baseRegExp, file->name) || regExpMatch(tablespaceRegExp, file->name))
strLstAddIfMissing(dbList, strBase(strPath(file->name)));
{
String *dbId = strBase(strPath(file->name));

// In the highly unlikely event that a system database was somehow added after the backup began, it will only be
// found in the file list and not the manifest db section, so add it to the system database list
if (cvtZToUInt64(strZ(dbId)) < PG_USER_OBJECT_MIN_ID)
strLstAddIfMissing(systemDbIdList, dbId);

strLstAddIfMissing(dbList, dbId);
}
}

strLstSort(dbList, sortOrderAsc);
Expand Down Expand Up @@ -1340,24 +1365,30 @@ restoreSelectiveExpression(Manifest *manifest)
}

// Error if the db is a system db
if (cvtZToUInt64(strZ(includeDb)) < PG_USER_OBJECT_MIN_ID)
if (strLstExists(systemDbIdList, includeDb))
THROW(DbInvalidError, "system databases (template0, postgres, etc.) are included by default");

// Remove from list of DBs to zero
strLstRemove(dbList, includeDb);
}

// Exclude the system databases from the list
strLstSort(systemDbIdList, sortOrderAsc);
dbList = strLstMergeAnti(dbList, systemDbIdList);

// Build regular expression to identify files that will be zeroed
strLstSort(dbList, sortOrderAsc);
String *expression = NULL;

for (unsigned int dbIdx = 0; dbIdx < strLstSize(dbList); dbIdx++)
if (!strLstEmpty(dbList))
{
const String *db = strLstGet(dbList, dbIdx);
LOG_DETAIL_FMT("databases excluded (zeroed) from selective restore (%s)", strZ(strLstJoin(dbList, ", ")));

// Only user created databases can be zeroed, never system databases
if (cvtZToUInt64(strZ(db)) >= PG_USER_OBJECT_MIN_ID)
// Generate the expression from the list of databases to be zeroed. Only user created databases can be zeroed, never
// system databases.
for (unsigned int dbIdx = 0; dbIdx < strLstSize(dbList); dbIdx++)
{
const String *db = strLstGet(dbList, dbIdx);

// Create expression string or append |
if (expression == NULL)
expression = strNew("");
Expand Down
2 changes: 2 additions & 0 deletions test/expect/mock-all-001.log
View file
Expand Up @@ -2818,6 +2818,7 @@ P00 INFO: restore command begin [BACKREST-VERSION]: --buffer-size=[BUFFER-SIZE
P00 INFO: repo1: restore backup set [BACKUP-DIFF-4]
P00 INFO: map tablespace 'pg_tblspc/2' to '[TEST_PATH]/db-primary/db/tablespace/ts2-2'
P00 DETAIL: databases found for selective restore (1, 16384, 32768)
P00 DETAIL: databases excluded (zeroed) from selective restore (32768)
P00 DETAIL: check '[TEST_PATH]/db-primary/db/base-2' exists
P00 DETAIL: check '[TEST_PATH]/db-primary/db/tablespace/ts2-2/[TS_PATH-1]' exists
P00 DETAIL: remove 'global/pg_control' so cluster will not start if restore does not complete
Expand Down Expand Up @@ -2882,6 +2883,7 @@ P00 INFO: restore command begin [BACKREST-VERSION]: --buffer-size=[BUFFER-SIZE
P00 INFO: repo1: restore backup set [BACKUP-DIFF-4]
P00 INFO: map tablespace 'pg_tblspc/2' to '[TEST_PATH]/db-primary/db/tablespace/ts2-2'
P00 DETAIL: databases found for selective restore (1, 16384, 32768)
P00 DETAIL: databases excluded (zeroed) from selective restore (16384)
P00 DETAIL: check '[TEST_PATH]/db-primary/db/base-2' exists
P00 DETAIL: check '[TEST_PATH]/db-primary/db/tablespace/ts2-2/[TS_PATH-1]' exists
P00 DETAIL: remove 'global/pg_control' so cluster will not start if restore does not complete
Expand Down
59 changes: 51 additions & 8 deletions test/src/module/command/restoreTest.c
View file
Expand Up @@ -1289,18 +1289,24 @@ testRun(void)

MEM_CONTEXT_BEGIN(manifest->memContext)
{
manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("postgres"), .id = 12173, .lastSystemId = 12168});
// Give non-systemId to postgres db
manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("postgres"), .id = 16385, .lastSystemId = 12168});
manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("template0"), .id = 12168, .lastSystemId = 12168});
manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("template1"), .id = 1, .lastSystemId = 12168});
manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("user-made-system-db"), .id = 16380, .lastSystemId = 12168});
manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF(UTF8_DB_NAME), .id = 16384, .lastSystemId = 12168});
manifestFileAdd(
manifest, &(ManifestFile){.name = STRDEF(MANIFEST_TARGET_PGDATA "/" PG_PATH_BASE "/1/" PG_FILE_PGVERSION)});
manifestFileAdd(
manifest, &(ManifestFile){.name = STRDEF(MANIFEST_TARGET_PGDATA "/" PG_PATH_BASE "/16381/" PG_FILE_PGVERSION)});
manifestFileAdd(
manifest, &(ManifestFile){.name = STRDEF(MANIFEST_TARGET_PGDATA "/" PG_PATH_BASE "/16385/" PG_FILE_PGVERSION)});
}
MEM_CONTEXT_END();

TEST_ERROR(restoreSelectiveExpression(manifest), DbMissingError, "database to include '" UTF8_DB_NAME "' does not exist");

TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1)");
TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1, 12168, 16380, 16381, 16385)");

// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("all databases selected");
Expand All @@ -1315,7 +1321,7 @@ testRun(void)
TEST_RESULT_STR(restoreSelectiveExpression(manifest), NULL, "all databases selected");

TEST_RESULT_LOG(
"P00 DETAIL: databases found for selective restore (1, 16384)\n"
"P00 DETAIL: databases found for selective restore (1, 12168, 16380, 16381, 16384, 16385)\n"
"P00 INFO: nothing to filter - all user databases have been selected");

// -------------------------------------------------------------------------------------------------------------------------
Expand All @@ -1329,7 +1335,33 @@ testRun(void)
restoreSelectiveExpression(manifest), DbInvalidError,
"system databases (template0, postgres, etc.) are included by default");

TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1, 16384)");
TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1, 12168, 16380, 16381, 16384, 16385)");

// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("error on system database with non-systemId selected");

argList = strLstDup(argListClean);
strLstAddZ(argList, "--db-include=16385");
harnessCfgLoad(cfgCmdRestore, argList);

TEST_ERROR(
restoreSelectiveExpression(manifest), DbInvalidError,
"system databases (template0, postgres, etc.) are included by default");

TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1, 12168, 16380, 16381, 16384, 16385)");

// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("error on system database with non-systemId selected, by name");

argList = strLstDup(argListClean);
strLstAddZ(argList, "--db-include=postgres");
harnessCfgLoad(cfgCmdRestore, argList);

TEST_ERROR(
restoreSelectiveExpression(manifest), DbInvalidError,
"system databases (template0, postgres, etc.) are included by default");

TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1, 12168, 16380, 16381, 16384, 16385)");

// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("error on missing database selected");
Expand All @@ -1340,7 +1372,7 @@ testRun(void)

TEST_ERROR(restoreSelectiveExpression(manifest), DbMissingError, "database to include '7777777' does not exist");

TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1, 16384)");
TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1, 12168, 16380, 16381, 16384, 16385)");

// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("select database by id");
Expand All @@ -1360,7 +1392,8 @@ testRun(void)
TEST_RESULT_STR_Z(restoreSelectiveExpression(manifest), "(^pg_data/base/32768/)", "check expression");

TEST_RESULT_LOG(
"P00 DETAIL: databases found for selective restore (1, 16384, 32768)");
"P00 DETAIL: databases found for selective restore (1, 12168, 16380, 16381, 16384, 16385, 32768)\n"
"P00 DETAIL: databases excluded (zeroed) from selective restore (32768)");

// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("one database selected without tablespace id");
Expand All @@ -1379,7 +1412,9 @@ testRun(void)
TEST_RESULT_STR_Z(
restoreSelectiveExpression(manifest), "(^pg_data/base/32768/)|(^pg_tblspc/16387/32768/)", "check expression");

TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1, 16384, 32768)");
TEST_RESULT_LOG(
"P00 DETAIL: databases found for selective restore (1, 12168, 16380, 16381, 16384, 16385, 32768)\n"
"P00 DETAIL: databases excluded (zeroed) from selective restore (32768)");

// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("one database selected with tablespace id");
Expand All @@ -1389,6 +1424,7 @@ testRun(void)

MEM_CONTEXT_BEGIN(manifest->memContext)
{
manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("test3"), .id = 65536, .lastSystemId = 12168});
manifestFileAdd(
manifest, &(ManifestFile){
.name = STRDEF(MANIFEST_TARGET_PGTBLSPC "/16387/PG_9.4_201409291/65536/" PG_FILE_PGVERSION)});
Expand All @@ -1401,7 +1437,9 @@ testRun(void)
"|(^pg_tblspc/16387/PG_9.4_201409291/65536/)",
"check expression");

TEST_RESULT_LOG("P00 DETAIL: databases found for selective restore (1, 16384, 32768, 65536)");
TEST_RESULT_LOG(
"P00 DETAIL: databases found for selective restore (1, 12168, 16380, 16381, 16384, 16385, 32768, 65536)\n"
"P00 DETAIL: databases excluded (zeroed) from selective restore (32768, 65536)");
}

// *****************************************************************************************************************************
Expand Down Expand Up @@ -2252,6 +2290,9 @@ testRun(void)
.checksumSha1 = "4d7b2a36c5387decf799352a3751883b7ceb96aa"});
storagePutP(storageNewWriteP(storageRepoWrite(), STRDEF(TEST_REPO_PATH "base/1/2")), fileBuffer);

// system db name
manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("template1"), .id = 1, .lastSystemId = 12168});

// base/16384 directory
manifestPathAdd(
manifest,
Expand Down Expand Up @@ -2283,6 +2324,7 @@ testRun(void)
storagePutP(storageNewWriteP(storageRepoWrite(), STRDEF(TEST_REPO_PATH "base/16384/16385")), fileBuffer);

// base/32768 directory
manifestDbAdd(manifest, &(ManifestDb){.name = STRDEF("test2"), .id = 32768, .lastSystemId = 12168});
manifestPathAdd(
manifest,
&(ManifestPath){
Expand Down Expand Up @@ -2532,6 +2574,7 @@ testRun(void)
"P00 INFO: map link 'pg_wal' to '../wal'\n"
"P00 INFO: map link 'postgresql.conf' to '../config/postgresql.conf'\n"
"P00 DETAIL: databases found for selective restore (1, 16384, 32768)\n"
"P00 DETAIL: databases excluded (zeroed) from selective restore (32768)\n"
"P00 DETAIL: check '{[path]}/pg' exists\n"
"P00 DETAIL: check '{[path]}/config' exists\n"
"P00 DETAIL: check '{[path]}/wal' exists\n"
Expand Down

0 comments on commit 6942ff5

Please sign in to comment.