db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-12 10:04:14 +02:00
Code Issues Releases Activity

Fix spurious automatic delta backup on backup from standby.

Browse Source 
When performing backup from standby the file sizes on the standby may not be equal to file sizes on the primary. This is because replication continues during the backup and by the time the file is copied from the standby it may have changed. Since we cap the size of all files copied from the standby this practically applies to truncation and in particular truncation of free space maps (at least, every case we have seen so far is an fsm). Free space maps are especially vulnerable since they are only partially replicated, which amplifies the difference between the primary and standby.

On an incremental backup it may look like the size has changed on the primary (because of the final size recorded by the standby in the prior backup) but the timestamp may not have changed on the primary and this will trigger a checksum delta for safety. While this has no impact on backup integrity, checksum delta incrementals can run much longer than regular incrementals and backup schedules may be impacted.

The solution is to preserve the original size in the manifest and use it to do the time/size check. In the case of backup from standby the original size will always be the size on the primary, which makes comparisons against subsequent file sizes on the primary consistent. Original size is only stored in the manifest when it differs from final size, so there should not be any noticeable manifest bloat.
This commit is contained in:
David Steele 2023-07-18 07:35:12 +02:00 committed by GitHub
parent 4c27d74bbd
commit 5ed6f8df14
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 66 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
doc/xml/release.xml
View File

@ -46,6 +46,20 @@
<p>Fix block incremental file names in <cmd>verify</cmd> command.</p>
</release-item>
<release-item>
<github-issue id="2020"/>
<github-pull-request id="2124"/>
<release-item-contributor-list>
<release-item-ideator id="krmozejko"/>
<release-item-ideator id="don.seiler"/>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="stephen.frost"/>
</release-item-contributor-list>
<p>Fix spurious automatic delta backup on backup from standby.</p>
</release-item>
<release-item>
<github-issue id="2062"/>
<github-pull-request id="2070"/>
@ -12775,6 +12789,11 @@
<contributor-id type="github">kaceymiri</contributor-id>
</contributor>
<contributor id="krmozejko">
<contributor-name-display>krmozejko</contributor-name-display>
<contributor-id type="github">krmozejko</contributor-id>
</contributor>
<contributor id="kyle.nevins">
<contributor-name-display>Kyle Nevins</contributor-name-display>
<contributor-id type="github">kyle-nevins</contributor-id>

2
src/command/backup/backup.c
View File

@ -1209,6 +1209,7 @@ backupFilePut(
.user = basePath->user,
.group = basePath->group,
.size = strSize(content),
.sizeOriginal = strSize(content),
.sizeRepo = pckReadU64P(ioFilterGroupResultP(filterGroup, SIZE_FILTER_TYPE)),
.timestamp = timestamp,
.checksumSha1 = bufPtr(pckReadBinP(ioFilterGroupResultP(filterGroup, CRYPTO_HASH_FILTER_TYPE, .idx = 0))),
@ -2403,6 +2404,7 @@ backupArchiveCheckCopy(const BackupData *const backupData, Manifest *const manif
.user = basePath->user,
.group = basePath->group,
.size = backupData->walSegmentSize,
.sizeOriginal = backupData->walSegmentSize,
.sizeRepo = pckReadU64P(ioFilterGroupResultP(filterGroup, SIZE_FILTER_TYPE)),
.timestamp = manifestData(manifest)->backupTimestampStop,
.checksumSha1 = bufPtr(bufNewDecode(encodingHex, strSubN(archiveFile, 25, 40))),

31
src/info/manifest.c
View File

@ -108,6 +108,7 @@ typedef enum
manifestFilePackFlagChecksumPage,
manifestFilePackFlagChecksumPageError,
manifestFilePackFlagChecksumPageErrorList,
manifestFilePackFlagSizeOriginal,
manifestFilePackFlagMode,
manifestFilePackFlagUser,
manifestFilePackFlagUserNull,
@ -166,6 +167,9 @@ manifestFilePack(const Manifest *const manifest, const ManifestFile *const file)
if (file->blockIncrSize != 0)
flag |= 1 << manifestFilePackFlagBlockIncr;
if (file->sizeOriginal != file->size)
flag |= 1 << manifestFilePackFlagSizeOriginal;
if (file->mode != manifest->fileModeDefault)
flag |= 1 << manifestFilePackFlagMode;
@ -184,6 +188,10 @@ manifestFilePack(const Manifest *const manifest, const ManifestFile *const file)
// Size
cvtUInt64ToVarInt128(file->size, buffer, &bufferPos, sizeof(buffer));
// Original size
if (flag & (1 << manifestFilePackFlagSizeOriginal))
cvtUInt64ToVarInt128(file->sizeOriginal, buffer, &bufferPos, sizeof(buffer));
// Use the first timestamp that appears as the base for all other timestamps. Ideally we would like a timestamp as close to the
// middle as possible but it doesn't seem worth doing the calculation.
if (manifestPackBaseTime == -1)
@ -308,6 +316,12 @@ manifestFileUnpack(const Manifest *const manifest, const ManifestFilePack *const
// Size
result.size = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
// Original size
if (flag & (1 << manifestFilePackFlagSizeOriginal))
result.sizeOriginal = cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX);
else
result.sizeOriginal = result.size;
// Timestamp
result.timestamp =
manifestPackBaseTime - (time_t)cvtInt64FromZigZag(cvtUInt64FromVarInt128((const uint8_t *)filePack, &bufferPos, UINT_MAX));
@ -1052,6 +1066,7 @@ manifestBuildInfo(
.user = info->user,
.group = info->group,
.size = info->size,
.sizeOriginal = info->size,
.sizeRepo = info->size,
.timestamp = info->timeModified,
};
@ -1634,12 +1649,12 @@ manifestBuildIncr(Manifest *this, const Manifest *manifestPrior, BackupType type
}
// Check for size change with no timestamp change
if (file.size != filePrior.size && file.timestamp == filePrior.timestamp)
if (file.sizeOriginal != filePrior.sizeOriginal && file.timestamp == filePrior.timestamp)
{
LOG_WARN_FMT(
"file '%s' has same timestamp (%" PRId64 ") as prior but different size (prior %" PRIu64 ", current"
" %" PRIu64 "), enabling delta checksum",
strZ(manifestPathPg(file.name)), (int64_t)file.timestamp, filePrior.size, file.size);
strZ(manifestPathPg(file.name)), (int64_t)file.timestamp, filePrior.sizeOriginal, file.sizeOriginal);
this->pub.data.backupOptionDelta = BOOL_TRUE_VAR;
break;
@ -1850,6 +1865,7 @@ manifestBuildComplete(
#define MANIFEST_KEY_PATH STRID5("path", 0x450300)
#define MANIFEST_KEY_REFERENCE STRID5("reference", 0x51b8b2298b20)
#define MANIFEST_KEY_SIZE STRID5("size", 0x2e9330)
#define MANIFEST_KEY_SIZE_ORIGINAL STRID5("szo", 0x3f530)
#define MANIFEST_KEY_SIZE_REPO STRID5("repo-size", 0x5d267b7c0b20)
#define MANIFEST_KEY_TABLESPACE_ID "tablespace-id"
#define MANIFEST_KEY_TABLESPACE_NAME "tablespace-name"
@ -2053,6 +2069,13 @@ manifestLoadCallback(void *callbackData, const String *const section, const Stri
if (file.size == 0)
file.checksumSha1 = bufPtrConst(HASH_TYPE_SHA1_ZERO_BUF);
// If original is not present in the manifest file then it is the same as size (i.e. the file did not change size during
// copy) -- to save space the original size is only stored in the manifest file if it is different than size.
if (jsonReadKeyExpectStrId(json, MANIFEST_KEY_SIZE_ORIGINAL))
file.sizeOriginal = jsonReadUInt64(json);
else
file.sizeOriginal = file.size;
// Timestamp is required so error if it is not present
if (jsonReadKeyExpectStrId(json, MANIFEST_KEY_TIMESTAMP))
file.timestamp = (time_t)jsonReadInt64(json);
@ -2766,6 +2789,10 @@ manifestSaveCallback(void *const callbackData, const String *const sectionNext,
jsonWriteUInt64(jsonWriteKeyStrId(json, MANIFEST_KEY_SIZE_REPO), file.sizeRepo);
jsonWriteUInt64(jsonWriteKeyStrId(json, MANIFEST_KEY_SIZE), file.size);
if (file.sizeOriginal != file.size)
jsonWriteUInt64(jsonWriteKeyStrId(json, MANIFEST_KEY_SIZE_ORIGINAL), file.sizeOriginal);
jsonWriteUInt64(jsonWriteKeyStrId(json, MANIFEST_KEY_TIMESTAMP), (uint64_t)file.timestamp);
if (!varEq(manifestOwnerVar(file.user), saveData->userDefault))

3
src/info/manifest.h
View File

@ -157,7 +157,8 @@ typedef struct ManifestFile
size_t blockIncrSize; // Size of incremental blocks
size_t blockIncrChecksumSize; // Size of incremental block checksum
uint64_t blockIncrMapSize; // Block incremental map size
uint64_t size; // Original size
uint64_t size; // Final size (after copy)
uint64_t sizeOriginal; // Original size (from manifest build)
uint64_t sizeRepo; // Size in repo
time_t timestamp; // Original timestamp
} ManifestFile;

5
test/src/common/harnessManifest.c
View File

@ -64,6 +64,7 @@ hrnManifestFileAdd(Manifest *const manifest, const HrnManifestFile hrnManifestFi
FUNCTION_HARNESS_PARAM(UINT64, hrnManifestFile.blockIncrSize);
FUNCTION_HARNESS_PARAM(UINT64, hrnManifestFile.blockIncrMapSize);
FUNCTION_HARNESS_PARAM(UINT64, hrnManifestFile.size);
FUNCTION_HARNESS_PARAM(UINT64, hrnManifestFile.sizeOriginal);
FUNCTION_HARNESS_PARAM(UINT64, hrnManifestFile.sizeRepo);
FUNCTION_HARNESS_PARAM(TIME, hrnManifestFile.timestamp);
FUNCTION_HARNESS_END();
@ -87,10 +88,14 @@ hrnManifestFileAdd(Manifest *const manifest, const HrnManifestFile hrnManifestFi
.blockIncrChecksumSize = hrnManifestFile.blockIncrChecksumSize,
.blockIncrMapSize = hrnManifestFile.blockIncrMapSize,
.size = hrnManifestFile.size,
.sizeOriginal = hrnManifestFile.sizeOriginal,
.sizeRepo = hrnManifestFile.sizeRepo,
.timestamp = hrnManifestFile.timestamp,
};
if (manifestFile.sizeOriginal == 0)
manifestFile.sizeOriginal = manifestFile.size;
if (hrnManifestFile.mode == 0)
manifestFile.mode = 0600;
else

1
test/src/common/harnessManifest.h
View File

@ -46,6 +46,7 @@ typedef struct HrnManifestFile
size_t blockIncrChecksumSize;
uint64_t blockIncrMapSize;
uint64_t size;
uint64_t sizeOriginal;
uint64_t sizeRepo;
time_t timestamp;
} HrnManifestFile;

3
test/src/module/command/backupTest.c
View File

@ -3148,7 +3148,8 @@ testRun(void)
",\"timestamp\":1571200002}\n"
"pg_data/base/1/1={\"size\":0,\"timestamp\":1571200000}\n"
"pg_data/base/1/2={\"checksum\":\"54ceb91256e8190e474aa752a6e0650a2df5ba37\",\"size\":2,\"timestamp\":1571200000}\n"
"pg_data/base/1/3={\"checksum\":\"3c01bdbb26f358bab27f267924aa2c9a03fcfdb8\",\"size\":3,\"timestamp\":1571200000}\n"
"pg_data/base/1/3={\"checksum\":\"3c01bdbb26f358bab27f267924aa2c9a03fcfdb8\",\"size\":3,\"szo\":4"
",\"timestamp\":1571200000}\n"
"pg_data/global/pg_control={\"size\":8192,\"timestamp\":1571200000}\n"
"pg_data/pg_xlog/0000000105DA69C000000000={\"size\":16777216,\"timestamp\":1571200002}\n"
"pg_data/postgresql.conf={\"checksum\":\"e3db315c260e79211b7b52587123b7aa060f30ab\",\"size\":11"

10
test/src/module/info/manifestTest.c
View File

@ -1245,8 +1245,9 @@ testRun(void)
.group = "test", .user = "test");
HRN_MANIFEST_FILE_ADD(
manifestPrior, .name = MANIFEST_TARGET_PGDATA "/FILE2", .copy = true, .size = 4, .sizeRepo = 4, .timestamp = 1482182860,
.reference = "20190101-010101F_20190202-010101D", .checksumSha1 = "ddddddddddbbbbbbbbbbccccccccccaaaaaaaaaa");
manifestPrior, .name = MANIFEST_TARGET_PGDATA "/FILE2", .copy = true, .size = 6, .sizeOriginal = 4, .sizeRepo = 4,
.timestamp = 1482182860, .reference = "20190101-010101F_20190202-010101D",
.checksumSha1 = "ddddddddddbbbbbbbbbbccccccccccaaaaaaaaaa");
TEST_RESULT_VOID(
manifestBuildIncr(manifest, manifestPrior, backupTypeIncr, STRDEF("000000040000000400000004")),
@ -1273,7 +1274,8 @@ testRun(void)
"\n"
"[target:file]\n"
"pg_data/FILE1={\"size\":6,\"timestamp\":1482182861}\n"
"pg_data/FILE2={\"size\":6,\"timestamp\":1482182860}\n"
"pg_data/FILE2={\"checksum\":\"ddddddddddbbbbbbbbbbccccccccccaaaaaaaaaa\""
",\"reference\":\"20190101-010101F_20190202-010101D\",\"repo-size\":4,\"size\":6,\"timestamp\":1482182860}\n"
TEST_MANIFEST_FILE_DEFAULT
"\n"
"[target:path]\n"
@ -1576,7 +1578,7 @@ testRun(void)
",\"rck\":\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\"reference\":\"20190818-084502F_20190819-084506D\"" \
",\"size\":4,\"timestamp\":1565282114}\n" \
"pg_data/base/16384/17000={\"bi\":4,\"bni\":1,\"checksum\":\"e0101dd8ffb910c9c202ca35b5f828bcb9697bed\"" \
",\"checksum-page\":false,\"checksum-page-error\":[1],\"repo-size\":4096,\"size\":8192" \
",\"checksum-page\":false,\"checksum-page-error\":[1],\"repo-size\":4096,\"size\":8192,\"szo\":16384" \
",\"timestamp\":1565282114}\n" \
"pg_data/base/16384/PG_VERSION={\"bni\":1,\"bno\":1,\"checksum\":\"184473f470864e067ee3a22e64b47b0a1c356f29\"" \
",\"group\":\"group2\",\"size\":4,\"timestamp\":1565282115,\"user\":false}\n" \