db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-01-04 03:49:14 +02:00
Code Issues Releases Activity

Prevent invalid recovery when backup_label removed.

Browse Source 
If backup_label is removed from a restored backup then PostgreSQL will instead use checkpoint information from pg_control to attempt (what is thinks is) crash recovery. This will nearly always result in a corrupt cluster because the checkpoint will not be from the beginning of the backup, and even if it is, the end point will not be specified, which could lead to recovery stopping too early.

To prevent this, invalidate the checkpoint LSN in pg_control on restore. If backup_label is removed then recovery will still fail because PostgreSQL will not be able to find the invalid checkpoint. The LSN of the checkpoint is not logged but it will be visible in pg_controldata output as 0/DEAD. This value is invalid because PostgreSQL always skips the first WAL segment when initializing a cluster.
This commit is contained in:
David Steele 2024-03-10 17:08:42 +13:00 committed by GitHub
parent 960b43589d
commit e634fd85ce
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 228 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
doc/xml/release/2024/2.51.xml
View File

@ -74,6 +74,17 @@
<p>Detect files that have not changed during non-delta incremental backup.</p>
</release-item>
<release-item>
<github-pull-request id="2232"/>
<release-item-contributor-list>
<release-item-contributor id="david.steele"/>
<release-item-reviewer id="stephen.frost"/>
</release-item-contributor-list>
<p>Prevent invalid recovery when <file>backup_label</file> removed.</p>
</release-item>
<release-item>
<github-pull-request id="2277"/>

10
src/command/archive/get/get.c
View File

@ -375,9 +375,17 @@ archiveGetCheck(const StringList *archiveRequestList)
// List of warnings
StringList *warnList = strLstNew();
// Get pg control info
// Get pg_control info. Error if the invalid checkpoint written by restore is detected and backup_label is not present.
PgControl controlInfo = pgControlFromFile(storagePg(), cfgOptionStrNull(cfgOptPgVersionForce));
if (controlInfo.checkpoint == PG_CONTROL_CHECKPOINT_INVALID && !storageExistsP(storagePg(), STRDEF(PG_FILE_BACKUPLABEL)))
{
THROW(
FormatError,
"pg_control from backup is not valid without backup_label\n"
"HINT: was the backup_label file removed?");
}
// Build list of repos/archiveIds where WAL may be found
List *cacheRepoList = lstNewP(sizeof(ArchiveGetFindCacheRepo));

16
src/command/restore/restore.c
View File

@ -2612,6 +2612,22 @@ cmdRestore(void)
// from being started.
if (storageExistsP(storagePg(), STRDEF(PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL "." STORAGE_FILE_TEMP_EXT)))
{
// Invalidate the checkpoint in pg_control so the cluster cannot be started without backup_label
if (manifestData(jobData.manifest)->backupOptionOnline)
{
Buffer *const pgControlBuffer = storageGetP(
storageNewReadP(storagePg(), STRDEF(PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL "." STORAGE_FILE_TEMP_EXT)));
const ManifestFile pgControlFile = manifestFileFind(
jobData.manifest, STRDEF(MANIFEST_TARGET_PGDATA "/" PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL));
pgControlCheckpointInvalidate(pgControlBuffer, cfgOptionStrNull(cfgOptPgVersionForce));
storagePutP(
storageNewWriteP(
storagePgWrite(), STRDEF(PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL "." STORAGE_FILE_TEMP_EXT),
.modeFile = pgControlFile.mode, .timeModified = pgControlFile.timestamp),
pgControlBuffer);
}
LOG_INFO(
"restore " PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL " (performed last to ensure aborted restores cannot be started)");

148
src/postgres/interface.c
View File

@ -71,6 +71,9 @@ typedef struct PgInterface
// Get control crc offset
size_t (*controlCrcOffset)(void);
// Invalidate control checkpoint
void (*controlCheckpointInvalidate)(unsigned char *);
// Get the control version for this version of PostgreSQL
uint32_t (*controlVersion)(void);
@ -191,6 +194,88 @@ pgWalSegmentSizeCheck(unsigned int pgVersion, unsigned int walSegmentSize)
FUNCTION_TEST_RETURN_VOID();
}
/***********************************************************************************************************************************
Validate the CRC in pg_control. If the version has been forced this may required scanning pg_control for the offset. Return the
offset found so it can be used to update the CRC.
***********************************************************************************************************************************/
static size_t
pgControlCrcValidate(const Buffer *const controlFile, const PgInterface *const interface, const bool pgVersionForce)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(BUFFER, controlFile);
FUNCTION_LOG_PARAM_P(VOID, interface);
FUNCTION_LOG_PARAM(BOOL, pgVersionForce);
FUNCTION_LOG_END();
ASSERT(controlFile != NULL);
ASSERT(interface != NULL);
size_t result = interface->controlCrcOffset();
do
{
// Calculate CRC and retrieve expected CRC
const uint32_t crcCalculated =
interface->version > PG_VERSION_94 ?
crc32cOne(bufPtrConst(controlFile), result) : crc32One(bufPtrConst(controlFile), result);
const uint32_t crcExpected = *((uint32_t *)(bufPtrConst(controlFile) + result));
// If CRC does not match
if (crcCalculated != crcExpected)
{
// If version is forced then the CRC might be later in the file (assuming the fork added extra fields to pg_control).
// Increment the offset by CRC data size and continue to try again.
if (pgVersionForce)
{
result += sizeof(uint32_t);
if (result <= bufUsed(controlFile) - sizeof(uint32_t))
continue;
}
// If no retry then error
THROW_FMT(
ChecksumError,
"calculated " PG_FILE_PGCONTROL " checksum does not match expected value\n"
"HINT: calculated 0x%x but expected value is 0x%x\n"
"%s"
"HINT: is " PG_FILE_PGCONTROL " corrupt?\n"
"HINT: does " PG_FILE_PGCONTROL " have a different layout than expected?",
crcCalculated, crcExpected,
pgVersionForce ? "HINT: checksum values may be misleading due to forced version scan\n" : "");
}
// Do not retry if the CRC is valid
break;
}
while (true);
FUNCTION_LOG_RETURN(SIZE, result);
}
/***********************************************************************************************************************************
Update the CRC in pg_control
***********************************************************************************************************************************/
static void
pgControlCrcUpdate(Buffer *const controlFile, const unsigned int pgVersion, const size_t crcOffset)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(BUFFER, controlFile);
FUNCTION_LOG_PARAM(UINT, pgVersion);
FUNCTION_LOG_PARAM(SIZE, crcOffset);
FUNCTION_LOG_END();
ASSERT(controlFile != NULL);
ASSERT(pgVersion != 0);
ASSERT(crcOffset != 0);
*((uint32_t *)(bufPtr(controlFile) + crcOffset)) =
pgVersion > PG_VERSION_94 ?
crc32cOne(bufPtrConst(controlFile), crcOffset) : crc32One(bufPtrConst(controlFile), crcOffset);
FUNCTION_LOG_RETURN_VOID();
}
/**********************************************************************************************************************************/
static PgControl
pgControlFromBuffer(const Buffer *controlFile, const String *const pgVersionForce)
@ -237,46 +322,8 @@ pgControlFromBuffer(const Buffer *controlFile, const String *const pgVersionForc
PgControl result = interface->control(bufPtrConst(controlFile));
result.version = interface->version;
// Check CRC
size_t crcOffset = interface->controlCrcOffset();
do
{
// Calculate CRC and retrieve expected CRC
const uint32_t crcCalculated =
result.version > PG_VERSION_94 ?
crc32cOne(bufPtrConst(controlFile), crcOffset) : crc32One(bufPtrConst(controlFile), crcOffset);
const uint32_t crcExpected = *((uint32_t *)(bufPtrConst(controlFile) + crcOffset));
// If CRC does not match
if (crcCalculated != crcExpected)
{
// If version is forced then the CRC might be later in the file (assuming the fork added extra fields to pg_control).
// Increment the offset by CRC data size and continue to try again.
if (pgVersionForce != NULL)
{
crcOffset += sizeof(uint32_t);
if (crcOffset <= bufUsed(controlFile) - sizeof(uint32_t))
continue;
}
// If no retry then error
THROW_FMT(
ChecksumError,
"calculated " PG_FILE_PGCONTROL " checksum does not match expected value\n"
"HINT: calculated 0x%x but expected value is 0x%x\n"
"%s"
"HINT: is " PG_FILE_PGCONTROL " corrupt?\n"
"HINT: does " PG_FILE_PGCONTROL " have a different layout than expected?",
crcCalculated, crcExpected,
pgVersionForce == NULL ? "" : "HINT: checksum values may be misleading due to forced version scan\n");
}
// Do not retry if the CRC is valid
break;
}
while (true);
// Validate the CRC
pgControlCrcValidate(controlFile, interface, pgVersionForce != NULL);
// Check the segment size
pgWalSegmentSizeCheck(result.version, result.walSegmentSize);
@ -389,6 +436,27 @@ pgControlFromFile(const Storage *const storage, const String *const pgVersionFor
FUNCTION_LOG_RETURN(PG_CONTROL, result);
}
/**********************************************************************************************************************************/
FN_EXTERN void
pgControlCheckpointInvalidate(Buffer *const buffer, const String *const pgVersionForce)
{
FUNCTION_LOG_BEGIN(logLevelDebug);
FUNCTION_LOG_PARAM(BUFFER, buffer);
FUNCTION_LOG_PARAM(STRING, pgVersionForce);
FUNCTION_LOG_END();
ASSERT(buffer != NULL);
const PgControl pgControl = pgControlFromBuffer(buffer, pgVersionForce);
const PgInterface *const interface = pgInterfaceVersion(pgControl.version);
const size_t crcOffset = pgControlCrcValidate(buffer, interface, pgVersionForce);
pgInterfaceVersion(pgControl.version)->controlCheckpointInvalidate(bufPtr(buffer));
pgControlCrcUpdate(buffer, interface->version, crcOffset);
FUNCTION_LOG_RETURN_VOID();
}
/**********************************************************************************************************************************/
FN_EXTERN uint32_t
pgControlVersion(unsigned int pgVersion)

8
src/postgres/interface.h
View File

@ -94,6 +94,11 @@ WAL segment size is supported for versions below 11.
***********************************************************************************************************************************/
#define PG_WAL_SEGMENT_SIZE_DEFAULT ((unsigned int)(16 * 1024 * 1024))
/***********************************************************************************************************************************
Checkpoint written into pg_control on restore. This will prevent PostgreSQL from starting if backup_label is not present.
***********************************************************************************************************************************/
#define PG_CONTROL_CHECKPOINT_INVALID 0xDEAD
/***********************************************************************************************************************************
PostgreSQL Control File Info
***********************************************************************************************************************************/
@ -138,6 +143,9 @@ FN_EXTERN bool pgDbIsSystemId(unsigned int id);
FN_EXTERN Buffer *pgControlBufferFromFile(const Storage *storage, const String *pgVersionForce);
FN_EXTERN PgControl pgControlFromFile(const Storage *storage, const String *pgVersionForce);
// Invalidate checkpoint record in pg_control
FN_EXTERN void pgControlCheckpointInvalidate(Buffer *buffer, const String *pgVersionForce);
// Get the control version for a PostgreSQL version
FN_EXTERN uint32_t pgControlVersion(unsigned int pgVersion);

16
src/postgres/interface/version.intern.h
View File

@ -92,6 +92,22 @@ Get control crc offset
#endif
/***********************************************************************************************************************************
Invalidate control checkpoint. PostgreSQL skips the first segment so any LSN in that segment is invalid.
***********************************************************************************************************************************/
#if PG_VERSION > PG_VERSION_MAX
#elif PG_VERSION >= PG_VERSION_93
#define PG_INTERFACE_CONTROL_CHECKPOINT_INVALIDATE(version) \
static void \
pgInterfaceControlCheckpointInvalidate##version(unsigned char *const controlFile) \
{ \
((ControlFileData *)controlFile)->checkPoint = PG_CONTROL_CHECKPOINT_INVALID; \
}
#endif
/***********************************************************************************************************************************
Get the control version
***********************************************************************************************************************************/

21
test/src/module/command/archiveGetTest.c
View File

@ -135,10 +135,28 @@ testRun(void)
.remove = true);
TEST_STORAGE_LIST_EMPTY(storageSpool(), STORAGE_SPOOL_ARCHIVE_IN);
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("pg_control from backup is not valid without backup_label");
HRN_PG_CONTROL_PUT(storagePgWrite(), PG_VERSION_10, .checkpoint = PG_CONTROL_CHECKPOINT_INVALID);
strLstAddZ(argList, "000000010000000100000001");
HRN_CFG_LOAD(cfgCmdArchiveGet, argList, .role = cfgCmdRoleAsync);
TEST_ERROR(
cmdArchiveGetAsync(), FormatError,
"pg_control from backup is not valid without backup_label\n"
"HINT: was the backup_label file removed?");
TEST_RESULT_LOG(
"P00 INFO: get 1 WAL file(s) from archive: 000000010000000100000001");
TEST_STORAGE_LIST(storageSpoolWrite(), STORAGE_SPOOL_ARCHIVE_IN, "global.error\n", .remove = true);
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("no segments to find");
HRN_PG_CONTROL_PUT(storagePgWrite(), PG_VERSION_10);
// Success not that backup_label exists
HRN_STORAGE_PUT_EMPTY(storagePgWrite(), PG_FILE_BACKUPLABEL);
HRN_INFO_PUT(
storageRepoWrite(), INFO_ARCHIVE_PATH_FILE,
@ -148,7 +166,6 @@ testRun(void)
"[db:history]\n"
"1={\"db-id\":" HRN_PG_SYSTEMID_10_Z ",\"db-version\":\"10\"}\n");
strLstAddZ(argList, "000000010000000100000001");
HRN_CFG_LOAD(cfgCmdArchiveGet, argList, .role = cfgCmdRoleAsync);
TEST_RESULT_VOID(cmdArchiveGetAsync(), "get async");

23
test/src/module/command/restoreTest.c
View File

@ -2503,6 +2503,8 @@ testRun(void)
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("incremental delta");
const char *pgControlSha1 = NULL;
argList = strLstNew();
hrnCfgArgRawZ(argList, cfgOptStanza, "test1");
hrnCfgArgRaw(argList, cfgOptRepoPath, repoPath);
@ -2548,13 +2550,12 @@ testRun(void)
HRN_MANIFEST_PATH_ADD(manifest, .name = TEST_PGDATA PG_PATH_GLOBAL);
// global/pg_control
Buffer *fileBuffer = bufNew(8192);
memset(bufPtr(fileBuffer), 255, bufSize(fileBuffer));
bufUsedSet(fileBuffer, bufSize(fileBuffer));
Buffer *fileBuffer = hrnPgControlToBuffer(0, 0, (PgControl){.version = PG_VERSION_10});
pgControlSha1 = strZ(strNewEncode(encodingHex, cryptoHashOne(hashTypeSha1, fileBuffer)));
HRN_MANIFEST_FILE_ADD(
manifest, .name = TEST_PGDATA PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL, .size = 8192, .timestamp = 1482182860,
.checksumSha1 = "5e2b96c19c4f5c63a5afa2de504d29fe64a4c908");
.checksumSha1 = pgControlSha1);
HRN_STORAGE_PUT(storageRepoWrite(), TEST_REPO_PATH PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL, fileBuffer);
// global/888
@ -2834,7 +2835,7 @@ testRun(void)
TEST_RESULT_VOID(cmdRestore(), "successful restore");
TEST_RESULT_LOG(
TEST_RESULT_LOG_FMT(
"P00 INFO: repo1: restore backup set 20161219-212741F_20161219-212918I\n"
"P00 INFO: map link 'pg_hba.conf' to '../config/pg_hba.conf'\n"
"P00 INFO: map link 'pg_wal' to '../wal'\n"
@ -2865,7 +2866,7 @@ testRun(void)
"P01 DETAIL: restore file " TEST_PATH "/pg/base/16384/16385 (16KB, [PCT]) checksum"
" d74e5f7ebe52a3ed468ba08c5b6aefaccd1ca88f\n"
"P01 DETAIL: restore file " TEST_PATH "/pg/global/pg_control.pgbackrest.tmp (8KB, [PCT])"
" checksum 5e2b96c19c4f5c63a5afa2de504d29fe64a4c908\n"
" checksum %s\n"
"P01 DETAIL: restore file " TEST_PATH "/pg/base/1/2 (8KB, [PCT]) checksum 4d7b2a36c5387decf799352a3751883b7ceb96aa\n"
"P01 DETAIL: restore file " TEST_PATH "/pg/postgresql.conf (15B, [PCT]) checksum"
" 98b8abb2e681e2a5a7d8ab082c0a79727887558d\n"
@ -2911,7 +2912,8 @@ testRun(void)
"P00 DETAIL: sync path '" TEST_PATH "/pg/pg_tblspc/1/PG_10_201707211'\n"
"P00 INFO: restore global/pg_control (performed last to ensure aborted restores cannot be started)\n"
"P00 DETAIL: sync path '" TEST_PATH "/pg/global'\n"
"P00 INFO: restore size = [SIZE], file total = 23");
"P00 INFO: restore size = [SIZE], file total = 23",
pgControlSha1);
TEST_STORAGE_LIST(
storagePg(), NULL,
@ -3028,7 +3030,7 @@ testRun(void)
TEST_RESULT_VOID(cmdRestore(), "successful restore");
TEST_RESULT_LOG(
TEST_RESULT_LOG_FMT(
"P00 INFO: repo2: restore backup set 20161219-212741F_20161219-212918I, recovery will start at [TIME]\n"
"P00 INFO: map link 'pg_hba.conf' to '../config/pg_hba.conf'\n"
"P00 INFO: map link 'pg_wal' to '../wal'\n"
@ -3058,7 +3060,7 @@ testRun(void)
"P01 DETAIL: restore file " TEST_PATH "/pg/base/16384/16385 - exists and matches backup (16KB, [PCT])"
" checksum d74e5f7ebe52a3ed468ba08c5b6aefaccd1ca88f\n"
"P01 DETAIL: restore file " TEST_PATH "/pg/global/pg_control.pgbackrest.tmp (8KB, [PCT])"
" checksum 5e2b96c19c4f5c63a5afa2de504d29fe64a4c908\n"
" checksum %s\n"
"P01 DETAIL: restore file " TEST_PATH "/pg/base/1/2 (8KB, [PCT]) checksum 4d7b2a36c5387decf799352a3751883b7ceb96aa\n"
"P01 DETAIL: restore file " TEST_PATH "/pg/postgresql.conf - exists and matches backup (15B, [PCT])"
" checksum 98b8abb2e681e2a5a7d8ab082c0a79727887558d\n"
@ -3104,7 +3106,8 @@ testRun(void)
"P00 DETAIL: sync path '" TEST_PATH "/pg/pg_tblspc/1/PG_10_201707211'\n"
"P00 INFO: restore global/pg_control (performed last to ensure aborted restores cannot be started)\n"
"P00 DETAIL: sync path '" TEST_PATH "/pg/global'\n"
"P00 INFO: restore size = [SIZE], file total = 23");
"P00 INFO: restore size = [SIZE], file total = 23",
pgControlSha1);
// Check stanza archive spool path was removed
TEST_STORAGE_LIST_EMPTY(storageSpool(), STORAGE_PATH_ARCHIVE);

28
test/src/module/postgres/interfaceTest.c
View File

@ -282,6 +282,34 @@ testRun(void)
TEST_RESULT_UINT(info.systemId, 0xAAAA0AAAA, "check system id");
TEST_RESULT_UINT(info.version, PG_VERSION_14, "check version");
TEST_RESULT_UINT(info.catalogVersion, 202007201, "check catalog version");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("Invalidate checkpoint");
control = hrnPgControlToBuffer(0, 0, (PgControl){.version = PG_VERSION_13, .systemId = 0xAAAA0AAAA, .checkpoint = 777});
info = pgControlFromBuffer(control, NULL);
TEST_RESULT_UINT(info.checkpoint, 777, "check checkpoint");
TEST_RESULT_VOID(pgControlCheckpointInvalidate(control, NULL), "invalidate checkpoint");
info = pgControlFromBuffer(control, NULL);
TEST_RESULT_UINT(info.checkpoint, 0xDEAD, "check invalid checkpoint");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("Invalidate checkpoint on force control version");
// Start with an invalid crc but write a valid one at a greater offset to test the forced scan
control = hrnPgControlToBuffer(
0, 0xFADEFADE, (PgControl){.version = PG_VERSION_94, .systemId = 0xAAAA0AAAA, .checkpoint = 777});
crcOffset = pgInterfaceVersion(PG_VERSION_94)->controlCrcOffset() + sizeof(uint32_t) * 2;
*((uint32_t *)(bufPtr(control) + crcOffset)) = crc32One(bufPtrConst(control), crcOffset);
info = pgControlFromBuffer(control, STRDEF(PG_VERSION_94_Z));
TEST_RESULT_UINT(info.checkpoint, 777, "check checkpoint");
TEST_RESULT_VOID(pgControlCheckpointInvalidate(control, STRDEF(PG_VERSION_94_Z)), "invalidate checkpoint");
info = pgControlFromBuffer(control, STRDEF(PG_VERSION_94_Z));
TEST_RESULT_UINT(info.checkpoint, 0xDEAD, "check invalid checkpoint");
}
// *****************************************************************************************************************************