You've already forked pgbackrest
mirror of
https://github.com/pgbackrest/pgbackrest.git
synced 2025-07-05 00:28:52 +02:00
Pack manifest file structs to save memory.
Manifests with a very large number of files can use a considerable amount of memory. There are a lot of zeroes in the data so it can be stored more efficiently by using base-128 varint encoding for the integers and storing the strings in the same allocation. The downside is that the data needs to be unpacked in order to be used, but in most cases this seems fast enough (about 10% slower than before) except for saving the manifest, which is 10% slower up to 10 million files and then gets about 5x slower by 100 million (two minutes on my M1 Mac). Profiling does not show this slowdown so I wonder if this is related to the change in memory layout. Curiously, the function that increased most was jsonFromStrInternal(), which was not modified. That gives more weight to the idea that there is some kind of memory issue going on here and one hopes that servers would be less affected. Either way, they largest use cases we have seen are for about 6 million files so if we can improve that case I believe we will be better off. Further analysis showed that most of the time was taken up writing the size and timestamp fields, which makes almost no sense. The same amount of time was used if they were hard-coded to 0, which points to some odd memory issue on the M1 architecture. This change has been planned for a while, but the particular impetus at this time is that small file support requires additional fields that would increase manifest memory usage by about 20%, even if the feature is not used. Note that the Pack code has been updated to use the new varint encoder, but the decoder remains separate because it needs to fetch one byte at a time.
This commit is contained in:
@ -23,7 +23,7 @@ typedef struct TestBackupValidateCallbackData
|
||||
{
|
||||
const Storage *storage; // Storage object when needed (e.g. fileCompressed = true)
|
||||
const String *path; // Subpath when storage is specified
|
||||
const Manifest *manifest; // Manifest to check for files/links/paths
|
||||
Manifest *manifest; // Manifest to check for files/links/paths
|
||||
const ManifestData *manifestData; // Manifest data
|
||||
String *content; // String where content should be added
|
||||
} TestBackupValidateCallbackData;
|
||||
@ -76,30 +76,31 @@ testBackupValidateCallback(void *callbackData, const StorageInfo *info)
|
||||
|
||||
// Check against the manifest
|
||||
// ---------------------------------------------------------------------------------------------------------------------
|
||||
const ManifestFile *file = manifestFileFind(data->manifest, manifestName);
|
||||
ManifestFilePack **const filePack = manifestFilePackFindInternal(data->manifest, manifestName);
|
||||
ManifestFile file = manifestFileUnpack(*filePack);
|
||||
|
||||
// Test size and repo-size. If compressed then set the repo-size to size so it will not be in test output. Even the same
|
||||
// compression algorithm can give slightly different results based on the version so repo-size is not deterministic for
|
||||
// compression.
|
||||
if (size != file->size)
|
||||
if (size != file.size)
|
||||
THROW_FMT(AssertError, "'%s' size does match manifest", strZ(manifestName));
|
||||
|
||||
if (info->size != file->sizeRepo)
|
||||
if (info->size != file.sizeRepo)
|
||||
THROW_FMT(AssertError, "'%s' repo size does match manifest", strZ(manifestName));
|
||||
|
||||
if (data->manifestData->backupOptionCompressType != compressTypeNone)
|
||||
((ManifestFile *)file)->sizeRepo = file->size;
|
||||
file.sizeRepo = file.size;
|
||||
|
||||
// Test the checksum. pg_control and WAL headers have different checksums depending on cpu architecture so remove
|
||||
// the checksum from the test output.
|
||||
if (!strEqZ(checksum, file->checksumSha1))
|
||||
if (!strEqZ(checksum, file.checksumSha1))
|
||||
THROW_FMT(AssertError, "'%s' checksum does match manifest", strZ(manifestName));
|
||||
|
||||
if (strEqZ(manifestName, MANIFEST_TARGET_PGDATA "/" PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL) ||
|
||||
strBeginsWith(
|
||||
manifestName, strNewFmt(MANIFEST_TARGET_PGDATA "/%s/", strZ(pgWalPath(data->manifestData->pgVersion)))))
|
||||
{
|
||||
((ManifestFile *)file)->checksumSha1[0] = '\0';
|
||||
file.checksumSha1[0] = '\0';
|
||||
}
|
||||
|
||||
// Test mode, user, group. These values are not in the manifest but we know what they should be based on the default
|
||||
@ -113,6 +114,9 @@ testBackupValidateCallback(void *callbackData, const StorageInfo *info)
|
||||
if (!strEq(info->group, TEST_GROUP_STR))
|
||||
THROW_FMT(AssertError, "'%s' group should be '" TEST_GROUP "'", strZ(manifestName));
|
||||
|
||||
// Update changes to manifest file
|
||||
manifestFilePackUpdate(data->manifest, filePack, &file);
|
||||
|
||||
break;
|
||||
}
|
||||
|
||||
@ -1310,7 +1314,7 @@ testRun(void)
|
||||
|
||||
manifestTargetAdd(manifestResume, &(ManifestTarget){.name = MANIFEST_TARGET_PGDATA_STR, .path = STRDEF("/pg")});
|
||||
manifestPathAdd(manifestResume, &(ManifestPath){.name = MANIFEST_TARGET_PGDATA_STR});
|
||||
manifestFileAdd(manifestResume, &(ManifestFile){.name = STRDEF("pg_data/" PG_FILE_PGVERSION)});
|
||||
manifestFileAdd(manifestResume, (ManifestFile){.name = STRDEF("pg_data/" PG_FILE_PGVERSION)});
|
||||
|
||||
manifestSave(
|
||||
manifestResume,
|
||||
@ -1430,7 +1434,7 @@ testRun(void)
|
||||
OBJ_NEW_BEGIN(Manifest)
|
||||
{
|
||||
manifest = manifestNewInternal();
|
||||
manifestFileAdd(manifest, &(ManifestFile){.name = STRDEF("pg_data/test")});
|
||||
manifestFileAdd(manifest, (ManifestFile){.name = STRDEF("pg_data/test")});
|
||||
}
|
||||
OBJ_NEW_END();
|
||||
|
||||
@ -1748,9 +1752,12 @@ testRun(void)
|
||||
storagePg(), PG_FILE_PGVERSION, storageRepoWrite(),
|
||||
strZ(strNewFmt(STORAGE_REPO_BACKUP "/%s/pg_data/PG_VERSION", strZ(resumeLabel))));
|
||||
|
||||
strcpy(
|
||||
((ManifestFile *)manifestFileFind(manifestResume, STRDEF("pg_data/PG_VERSION")))->checksumSha1,
|
||||
"06d06bb31b570b94d7b4325f511f853dbe771c21");
|
||||
ManifestFilePack **const filePack = manifestFilePackFindInternal(manifestResume, STRDEF("pg_data/PG_VERSION"));
|
||||
ManifestFile file = manifestFileUnpack(*filePack);
|
||||
|
||||
strcpy(file.checksumSha1, "06d06bb31b570b94d7b4325f511f853dbe771c21");
|
||||
|
||||
manifestFilePackUpdate(manifestResume, filePack, &file);
|
||||
|
||||
// Save the resume manifest
|
||||
manifestSave(
|
||||
@ -1841,14 +1848,19 @@ testRun(void)
|
||||
HRN_STORAGE_PUT_EMPTY(
|
||||
storageRepoWrite(), strZ(strNewFmt(STORAGE_REPO_BACKUP "/%s/pg_data/global/pg_control.gz", strZ(resumeLabel))));
|
||||
|
||||
((ManifestFile *)manifestFileFind(manifestResume, STRDEF("pg_data/global/pg_control")))->checksumSha1[0] = 0;
|
||||
ManifestFilePack **const filePack = manifestFilePackFindInternal(manifestResume, STRDEF("pg_data/global/pg_control"));
|
||||
ManifestFile file = manifestFileUnpack(*filePack);
|
||||
|
||||
file.checksumSha1[0] = 0;
|
||||
|
||||
manifestFilePackUpdate(manifestResume, filePack, &file);
|
||||
|
||||
// Size does not match between cluster and resume manifest
|
||||
HRN_STORAGE_PUT_Z(storagePgWrite(), "size-mismatch", "TEST", .timeModified = backupTimeStart);
|
||||
HRN_STORAGE_PUT_EMPTY(
|
||||
storageRepoWrite(), strZ(strNewFmt(STORAGE_REPO_BACKUP "/%s/pg_data/size-mismatch.gz", strZ(resumeLabel))));
|
||||
manifestFileAdd(
|
||||
manifestResume, &(ManifestFile){
|
||||
manifestResume, (ManifestFile){
|
||||
.name = STRDEF("pg_data/size-mismatch"), .checksumSha1 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
|
||||
.size = 33});
|
||||
|
||||
@ -1857,7 +1869,7 @@ testRun(void)
|
||||
HRN_STORAGE_PUT_EMPTY(
|
||||
storageRepoWrite(), strZ(strNewFmt(STORAGE_REPO_BACKUP "/%s/pg_data/time-mismatch.gz", strZ(resumeLabel))));
|
||||
manifestFileAdd(
|
||||
manifestResume, &(ManifestFile){
|
||||
manifestResume, (ManifestFile){
|
||||
.name = STRDEF("pg_data/time-mismatch"), .checksumSha1 = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", .size = 4,
|
||||
.timestamp = backupTimeStart - 1});
|
||||
|
||||
@ -1867,7 +1879,7 @@ testRun(void)
|
||||
storageRepoWrite(), strZ(strNewFmt(STORAGE_REPO_BACKUP "/%s/pg_data/zero-size.gz", strZ(resumeLabel))),
|
||||
"ZERO-SIZE");
|
||||
manifestFileAdd(
|
||||
manifestResume, &(ManifestFile){.name = STRDEF("pg_data/zero-size"), .size = 0, .timestamp = backupTimeStart});
|
||||
manifestResume, (ManifestFile){.name = STRDEF("pg_data/zero-size"), .size = 0, .timestamp = backupTimeStart});
|
||||
|
||||
// Path is not in manifest
|
||||
HRN_STORAGE_PATH_CREATE(
|
||||
@ -2028,7 +2040,7 @@ testRun(void)
|
||||
HRN_STORAGE_PUT_EMPTY(
|
||||
storageRepoWrite(), strZ(strNewFmt(STORAGE_REPO_BACKUP "/%s/pg_data/resume-ref.gz", strZ(resumeLabel))));
|
||||
manifestFileAdd(
|
||||
manifestResume, &(ManifestFile){.name = STRDEF("pg_data/resume-ref"), .size = 0, .reference = STRDEF("BOGUS")});
|
||||
manifestResume, (ManifestFile){.name = STRDEF("pg_data/resume-ref"), .size = 0, .reference = STRDEF("BOGUS")});
|
||||
|
||||
// Time does not match between cluster and resume manifest (but resume because time is in future so delta enabled). Note
|
||||
// also that the repo file is intenionally corrupt to generate a warning about corruption in the repository.
|
||||
@ -2036,7 +2048,7 @@ testRun(void)
|
||||
HRN_STORAGE_PUT_EMPTY(
|
||||
storageRepoWrite(), strZ(strNewFmt(STORAGE_REPO_BACKUP "/%s/pg_data/time-mismatch2.gz", strZ(resumeLabel))));
|
||||
manifestFileAdd(
|
||||
manifestResume, &(ManifestFile){
|
||||
manifestResume, (ManifestFile){
|
||||
.name = STRDEF("pg_data/time-mismatch2"), .checksumSha1 = "984816fd329622876e14907634264e6f332e9fb3", .size = 4,
|
||||
.timestamp = backupTimeStart});
|
||||
|
||||
|
Reference in New Issue
Block a user