1
0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-12 10:04:14 +02:00

Improve manifest file updates.

The prior manifestFileUpdate() function was pretty difficult to use since all the parameters had to specified. Instead, pass a ManifestFile struct that has all members set as needed.

When new struct members are added the manifestFileUpdate() call sites will still need to be reviewed, but this should make the process of adding members a bit simpler.
This commit is contained in:
David Steele 2022-10-04 14:19:12 -10:00 committed by GitHub
parent f981fb45d9
commit 1ea6a4142e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 83 additions and 86 deletions

View File

@ -47,6 +47,19 @@
</release-improvement-list>
<release-development-list>
<release-item>
<commit subject="Improve manifest file updates.">
<github-pull-request id="1888"/>
</commit>
<release-item-contributor-list>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="stefan.fercot"/>
</release-item-contributor-list>
<p>Block incremental backup.</p>
</release-item>
<release-item>
<github-pull-request id="1879"/>

View File

@ -571,7 +571,7 @@ backupResumeClean(
removeReason = "missing in manifest";
else
{
const ManifestFile file = manifestFileFind(manifest, manifestName);
ManifestFile file = manifestFileFind(manifest, manifestName);
if (file.reference != NULL)
removeReason = "reference in manifest";
@ -594,9 +594,13 @@ backupResumeClean(
removeReason = "zero size";
else
{
manifestFileUpdate(
manifest, manifestName, file.size, fileResume.sizeRepo, fileResume.checksumSha1, NULL,
fileResume.checksumPage, fileResume.checksumPageError, fileResume.checksumPageErrorList, 0, 0);
file.sizeRepo = fileResume.sizeRepo;
memcpy(file.checksumSha1, fileResume.checksumSha1, HASH_TYPE_SHA1_SIZE_HEX + 1);
file.checksumPage = fileResume.checksumPage;
file.checksumPageError = fileResume.checksumPageError;
file.checksumPageErrorList = fileResume.checksumPageErrorList;
manifestFileUpdate(manifest, &file);
}
}
}
@ -1170,7 +1174,7 @@ backupJobResult(
while (!pckReadNullP(jobResult))
{
const ManifestFile file = manifestFileFind(manifest, pckReadStrP(jobResult));
ManifestFile file = manifestFileFind(manifest, pckReadStrP(jobResult));
const BackupCopyResult copyResult = (BackupCopyResult)pckReadU32P(jobResult);
const uint64_t copySize = pckReadU64P(jobResult);
const uint64_t bundleOffset = pckReadU64P(jobResult);
@ -1312,10 +1316,17 @@ backupJobResult(
}
// Update file info and remove any reference to the file's existence in a prior backup
manifestFileUpdate(
manifest, file.name, copySize, repoSize, strZ(copyChecksum), VARSTR(NULL), file.checksumPage,
checksumPageError, checksumPageErrorList != NULL ? jsonFromVar(varNewVarLst(checksumPageErrorList)) : NULL,
bundleId, bundleOffset);
file.size = copySize;
file.sizeRepo = repoSize;
memcpy(file.checksumSha1, strZ(copyChecksum), HASH_TYPE_SHA1_SIZE_HEX + 1);
file.reference = NULL;
file.checksumPageError = checksumPageError;
file.checksumPageErrorList = checksumPageErrorList != NULL ?
jsonFromVar(varNewVarLst(checksumPageErrorList)) : NULL;
file.bundleId = bundleId;
file.bundleOffset = bundleOffset;
manifestFileUpdate(manifest, &file);
}
}
@ -1533,15 +1544,18 @@ backupProcessQueue(const BackupData *const backupData, Manifest *const manifest,
for (unsigned int fileIdx = 0; fileIdx < manifestFileTotal(manifest); fileIdx++)
{
const ManifestFilePack *const filePack = manifestFilePackGet(manifest, fileIdx);
const ManifestFile file = manifestFileUnpack(manifest, filePack);
ManifestFile file = manifestFileUnpack(manifest, filePack);
// If bundling store zero-length files immediately in the manifest without copying them
if (jobData->bundle && file.size == 0)
{
LOG_DETAIL_FMT(
"store zero-length file %s", strZ(storagePathP(backupData->storagePrimary, manifestPathPg(file.name))));
manifestFileUpdate(
manifest, file.name, 0, 0, strZ(HASH_TYPE_SHA1_ZERO_STR), VARSTR(NULL), file.checksumPage, false, NULL, 0, 0);
memcpy(file.checksumSha1, HASH_TYPE_SHA1_ZERO, HASH_TYPE_SHA1_SIZE_HEX + 1);
file.reference = NULL;
manifestFileUpdate(manifest, &file);
continue;
}

View File

@ -172,7 +172,10 @@ manifestFilePack(const Manifest *const manifest, const ManifestFile *const file)
// Reference
if (file->reference != NULL)
cvtUInt64ToVarInt128((uintptr_t)file->reference, buffer, &bufferPos, sizeof(buffer));
{
cvtUInt64ToVarInt128(
(uintptr_t)strLstAddIfMissing(manifest->referenceList, file->reference), buffer, &bufferPos, sizeof(buffer));
}
// Mode
if (flag & (1 << manifestFilePackFlagMode))
@ -316,9 +319,6 @@ manifestFileAdd(Manifest *const this, ManifestFile *const file)
file->user = manifestOwnerCache(this, file->user);
file->group = manifestOwnerCache(this, file->group);
if (file->reference != NULL)
file->reference = strLstAddIfMissing(this->referenceList, file->reference);
MEM_CONTEXT_BEGIN(lstMemContext(this->pub.fileList))
{
const ManifestFilePack *const filePack = manifestFilePack(this, file);
@ -1477,7 +1477,7 @@ manifestBuildIncr(Manifest *this, const Manifest *manifestPrior, BackupType type
for (unsigned int fileIdx = 0; fileIdx < lstSize(this->pub.fileList); fileIdx++)
{
const ManifestFile file = manifestFile(this, fileIdx);
ManifestFile file = manifestFile(this, fileIdx);
// Check if prior file can be used
if (manifestFileExists(manifestPrior, file.name))
@ -1486,11 +1486,16 @@ manifestBuildIncr(Manifest *this, const Manifest *manifestPrior, BackupType type
if (file.size == filePrior.size && (delta || file.size == 0 || file.timestamp == filePrior.timestamp))
{
manifestFileUpdate(
this, file.name, file.size, filePrior.sizeRepo, filePrior.checksumSha1,
VARSTR(filePrior.reference != NULL ? filePrior.reference : manifestPrior->pub.data.backupLabel),
filePrior.checksumPage, filePrior.checksumPageError, filePrior.checksumPageErrorList,
filePrior.bundleId, filePrior.bundleOffset);
file.sizeRepo = filePrior.sizeRepo;
memcpy(file.checksumSha1, filePrior.checksumSha1, HASH_TYPE_SHA1_SIZE_HEX + 1);
file.reference = filePrior.reference != NULL ? filePrior.reference : manifestPrior->pub.data.backupLabel;
file.checksumPage = filePrior.checksumPage;
file.checksumPageError = filePrior.checksumPageError;
file.checksumPageErrorList = filePrior.checksumPageErrorList;
file.bundleId = filePrior.bundleId;
file.bundleOffset = filePrior.bundleOffset;
manifestFileUpdate(this, &file);
}
}
}
@ -2753,64 +2758,23 @@ manifestFileRemove(const Manifest *this, const String *name)
}
void
manifestFileUpdate(
Manifest *const this, const String *const name, const uint64_t size, const uint64_t sizeRepo, const char *const checksumSha1,
const Variant *const reference, const bool checksumPage, const bool checksumPageError,
const String *const checksumPageErrorList, const uint64_t bundleId, const uint64_t bundleOffset)
manifestFileUpdate(Manifest *const this, const ManifestFile *const file)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(MANIFEST, this);
FUNCTION_TEST_PARAM(STRING, name);
FUNCTION_TEST_PARAM(UINT64, size);
FUNCTION_TEST_PARAM(UINT64, sizeRepo);
FUNCTION_TEST_PARAM(STRINGZ, checksumSha1);
FUNCTION_TEST_PARAM(VARIANT, reference);
FUNCTION_TEST_PARAM(BOOL, checksumPage);
FUNCTION_TEST_PARAM(BOOL, checksumPageError);
FUNCTION_TEST_PARAM(STRING, checksumPageErrorList);
FUNCTION_TEST_PARAM(UINT64, bundleId);
FUNCTION_TEST_PARAM(UINT64, bundleOffset);
FUNCTION_TEST_PARAM(MANIFEST_FILE, file);
FUNCTION_TEST_END();
ASSERT(this != NULL);
ASSERT(name != NULL);
ASSERT(file != NULL);
ASSERT(
(!checksumPage && !checksumPageError && checksumPageErrorList == NULL) ||
(checksumPage && !checksumPageError && checksumPageErrorList == NULL) || (checksumPage && checksumPageError));
(!file->checksumPage && !file->checksumPageError && file->checksumPageErrorList == NULL) ||
(file->checksumPage && !file->checksumPageError && file->checksumPageErrorList == NULL) ||
(file->checksumPage && file->checksumPageError));
ASSERT(file->size != 0 || (file->bundleId == 0 && file->bundleOffset == 0));
ManifestFilePack **const filePack = manifestFilePackFindInternal(this, name);
ManifestFile file = manifestFileUnpack(this, *filePack);
// Update reference if set
if (reference != NULL)
{
if (varStr(reference) == NULL)
file.reference = NULL;
else
file.reference = strLstAddIfMissing(this->referenceList, varStr(reference));
}
// Update checksum if set
if (checksumSha1 != NULL)
{
ASSERT(strlen(checksumSha1) == HASH_TYPE_SHA1_SIZE_HEX);
memcpy(file.checksumSha1, checksumSha1, HASH_TYPE_SHA1_SIZE_HEX + 1);
}
// Update repo size
file.size = size;
file.sizeRepo = sizeRepo;
// Update checksum page info
file.checksumPage = checksumPage;
file.checksumPageError = checksumPageError;
file.checksumPageErrorList = checksumPageErrorList;
// Update bundle info
file.bundleId = bundleId;
file.bundleOffset = bundleOffset;
manifestFilePackUpdate(this, filePack, &file);
ManifestFilePack **const filePack = manifestFilePackFindInternal(this, file->name);
manifestFilePackUpdate(this, filePack, file);
FUNCTION_TEST_RETURN_VOID();
}

View File

@ -325,9 +325,7 @@ manifestFileTotal(const Manifest *const this)
}
// Update a file with new data
void manifestFileUpdate(
Manifest *this, const String *name, uint64_t size, uint64_t sizeRepo, const char *checksumSha1, const Variant *reference,
bool checksumPage, bool checksumPageError, const String *checksumPageErrorList, uint64_t bundleId, uint64_t bundleOffset);
void manifestFileUpdate(Manifest *const this, const ManifestFile *file);
/***********************************************************************************************************************************
Link functions and getters/setters

View File

@ -1606,8 +1606,14 @@ testRun(void)
TEST_TITLE("manifest validation");
// Munge files to produce errors
manifestFileUpdate(manifest, STRDEF("pg_data/postgresql.conf"), 4457, 0, NULL, NULL, false, false, NULL, 0, 0);
manifestFileUpdate(manifest, STRDEF("pg_data/base/32768/33000.32767"), 0, 0, NULL, NULL, true, false, NULL, 0, 0);
ManifestFile file = manifestFileFind(manifest, STRDEF("pg_data/postgresql.conf"));
file.checksumSha1[0] = '\0';
file.sizeRepo = 0;
manifestFileUpdate(manifest, &file);
file = manifestFileFind(manifest, STRDEF("pg_data/base/32768/33000.32767"));
file.size = 0;
manifestFileUpdate(manifest, &file);
TEST_ERROR(
manifestValidate(manifest, false), FormatError,
@ -1622,10 +1628,14 @@ testRun(void)
"repo size must be > 0 for file 'pg_data/postgresql.conf'");
// Undo changes made to files
manifestFileUpdate(manifest, STRDEF("pg_data/base/32768/33000.32767"), 32768, 32768, NULL, NULL, true, false, NULL, 0, 0);
manifestFileUpdate(
manifest, STRDEF("pg_data/postgresql.conf"), 4457, 4457, "184473f470864e067ee3a22e64b47b0a1c356f29", NULL, false,
false, NULL, 0, 0);
file = manifestFileFind(manifest, STRDEF("pg_data/postgresql.conf"));
memcpy(file.checksumSha1, "184473f470864e067ee3a22e64b47b0a1c356f29", HASH_TYPE_SHA1_SIZE_HEX + 1);
file.sizeRepo = 4457;
manifestFileUpdate(manifest, &file);
file = manifestFileFind(manifest, STRDEF("pg_data/base/32768/33000.32767"));
file.size = 32768;
manifestFileUpdate(manifest, &file);
TEST_RESULT_VOID(manifestValidate(manifest, true), "successful validate");
@ -1776,7 +1786,6 @@ testRun(void)
TEST_TITLE("manifest getters");
// ManifestFile getters
ManifestFile file = {0};
TEST_ERROR(manifestFileFind(manifest, STRDEF("bogus")), AssertError, "unable to find 'bogus' in manifest file list");
TEST_ASSIGN(file, manifestFileFind(manifest, STRDEF("pg_data/PG_VERSION")), "manifestFileFind()");
TEST_RESULT_STR_Z(file.name, "pg_data/PG_VERSION", "find file");
@ -1791,10 +1800,9 @@ testRun(void)
fileMunge.checksumSha1[0] = '\0';
manifestFilePackUpdate(manifest, fileMungePack, &fileMunge);
TEST_RESULT_VOID(
manifestFileUpdate(
manifest, STRDEF("pg_data/postgresql.conf"), 4457, 4457, NULL, varNewStr(NULL), false, false, NULL, 0, 0),
"update file");
file = manifestFileFind(manifest, STRDEF("pg_data/postgresql.conf"));
file.checksumSha1[0] = '\0';
manifestFileUpdate(manifest, &file);
// ManifestDb getters
const ManifestDb *db = NULL;