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

Improve validation of zero pages.

Browse Source 
Checking that pd_upper == 0 is not enough since this field may be corrupted. Still use pd_upper as a quick check, but when it is zero proceed to check the rest of the page to ensure it is also all zeroes.
This commit is contained in:
David Steele 2022-02-23 13:17:14 -06:00 committed by GitHub
parent 9eec98c613
commit 53f1b25204
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 45 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
doc/xml/release.xml
View File

@ -78,8 +78,13 @@
<release-item>
<commit subject="Move command/backup-common tests in the command/backup module."/>
<commit subject="Retry on page checksum validation failure during backup.">
<github-issue id="952"/>
<github-pull-request id="1667"/>
</commit>
<commit subject="Improve validation of zero pages.">
<github-issue id="952"/>
<github-pull-request id="1669"/>
</commit>
<release-item-contributor-list>
<release-item-contributor id="david.steele"/>
@ -87,7 +92,7 @@
<release-item-reviewer id="david.christensen"/>
</release-item-contributor-list>
<p>Retry on page checksum validation failure during <cmd>backup</cmd>.</p>
<p>Retry on page validation failure during <cmd>backup</cmd>.</p>
</release-item>
<release-item>

42
src/command/backup/pageChecksum.c
View File

@ -95,24 +95,44 @@ pageChecksumProcess(THIS_VOID, const Buffer *input)
// Get a pointer to the page header
const PageHeaderData *const pageHeader = (const PageHeaderData *)(bufPtrConst(input) + pageIdx * PG_PAGE_SIZE_DEFAULT);
// Skip new pages ??? Improved to exactly match what PostgreSQL does in PageIsVerifiedExtended()
if (pageHeader->pd_upper == 0)
continue;
// Block number relative to all segments in the relation
const unsigned int blockNo = this->pageNoOffset + pageIdx;
// Only validate page checksum if the page is complete
if (this->align || pageIdx < pageTotal - 1)
{
// Make a copy of the page since it will be modified by the page checksum function
memcpy(this->pageBuffer, pageHeader, PG_PAGE_SIZE_DEFAULT);
// Skip new pages indicated by pd_upper == 0
bool pdUpperValid = true;
// Continue if the checksum matches
if (pageHeader->pd_checksum == pgPageChecksum(this->pageBuffer, blockNo))
continue;
if (pageHeader->pd_upper == 0)
{
// Check that the entire page is zero in case pd_upper is corrupted
for (unsigned int pageIdx = 0; pageIdx < PG_PAGE_SIZE_DEFAULT / sizeof(size_t); pageIdx++)
{
if (((size_t *)pageHeader)[pageIdx] != 0)
{
pdUpperValid = false;
break;
}
}
// On checksum mismatch retry the page
// If the entire page is zero it is valid
if (pdUpperValid)
continue;
}
// Only validate the checksum if pd_upper is non-zero to avoid an assertion from pg_checksum_page()
if (pdUpperValid)
{
// Make a copy of the page since it will be modified by the page checksum function
memcpy(this->pageBuffer, pageHeader, PG_PAGE_SIZE_DEFAULT);
// Continue if the checksum matches
if (pageHeader->pd_checksum == pgPageChecksum(this->pageBuffer, blockNo))
continue;
}
// On error retry the page
bool changed = false;
MEM_CONTEXT_TEMP_BEGIN()
@ -125,7 +145,7 @@ pageChecksumProcess(THIS_VOID, const Buffer *input)
.limit = VARUINT64(PG_PAGE_SIZE_DEFAULT)));
// Check if the page has changed since it was last read
changed = !bufEq(pageRetry, BUF(this->pageBuffer, PG_PAGE_SIZE_DEFAULT));
changed = !bufEq(pageRetry, BUF(pageHeader, PG_PAGE_SIZE_DEFAULT));
}
MEM_CONTEXT_TEMP_END();

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

@ -2797,12 +2797,14 @@ testRun(void)
HRN_STORAGE_PUT(storagePgWrite(), PG_PATH_BASE "/1/2", relation, .timeModified = backupTimeStart);
// File with bad page checksums
relation = bufNew(PG_PAGE_SIZE_DEFAULT * 4);
relation = bufNew(PG_PAGE_SIZE_DEFAULT * 5);
memset(bufPtr(relation), 0, bufSize(relation));
*(PageHeaderData *)(bufPtr(relation) + (PG_PAGE_SIZE_DEFAULT * 0x00)) = (PageHeaderData){.pd_upper = 0xFF};
*(PageHeaderData *)(bufPtr(relation) + (PG_PAGE_SIZE_DEFAULT * 0x01)) = (PageHeaderData){.pd_upper = 0x00};
*(PageHeaderData *)(bufPtr(relation) + (PG_PAGE_SIZE_DEFAULT * 0x02)) = (PageHeaderData){.pd_upper = 0xFE};
*(PageHeaderData *)(bufPtr(relation) + (PG_PAGE_SIZE_DEFAULT * 0x03)) = (PageHeaderData){.pd_upper = 0xEF};
*(PageHeaderData *)(bufPtr(relation) + (PG_PAGE_SIZE_DEFAULT * 0x04)) = (PageHeaderData){.pd_upper = 0x00};
(bufPtr(relation) + (PG_PAGE_SIZE_DEFAULT * 0x04))[PG_PAGE_SIZE_DEFAULT - 1] = 0xFF;
bufUsedSet(relation, bufSize(relation));
HRN_STORAGE_PUT(storagePgWrite(), PG_PATH_BASE "/1/3", relation, .timeModified = backupTimeStart);
@ -2850,8 +2852,8 @@ testRun(void)
"P00 INFO: execute non-exclusive pg_start_backup(): backup begins after the next regular checkpoint completes\n"
"P00 INFO: backup start archive = 0000000105DB5DE000000000, lsn = 5db5de0/0\n"
"P00 INFO: check archive for segment 0000000105DB5DE000000000\n"
"P01 DETAIL: backup file " TEST_PATH "/pg1/base/1/3 (32KB, [PCT]) checksum [SHA1]\n"
"P00 WARN: invalid page checksums found in file " TEST_PATH "/pg1/base/1/3 at pages 0, 2-3\n"
"P01 DETAIL: backup file " TEST_PATH "/pg1/base/1/3 (40KB, [PCT]) checksum [SHA1]\n"
"P00 WARN: invalid page checksums found in file " TEST_PATH "/pg1/base/1/3 at pages 0, 2-4\n"
"P01 DETAIL: backup file " TEST_PATH "/pg1/base/1/4 (24KB, [PCT]) checksum [SHA1]\n"
"P00 WARN: invalid page checksum found in file " TEST_PATH "/pg1/base/1/4 at page 1\n"
"P01 DETAIL: backup file " TEST_PATH "/pg1/base/1/2 (8.5KB, [PCT]) checksum [SHA1]\n"
@ -2883,7 +2885,7 @@ testRun(void)
"pg_data/base/1 {path}\n"
"pg_data/base/1/1.gz {file, s=8192}\n"
"pg_data/base/1/2.gz {file, s=8704}\n"
"pg_data/base/1/3.gz {file, s=32768}\n"
"pg_data/base/1/3.gz {file, s=40960}\n"
"pg_data/base/1/4.gz {file, s=24576}\n"
"pg_data/global {path}\n"
"pg_data/global/pg_control.gz {file, s=8192}\n"
@ -2914,8 +2916,8 @@ testRun(void)
",\"size\":8192,\"timestamp\":1572200000}\n"
"pg_data/base/1/2={\"checksum\":\"2deafa7ae60279a54a09422b985a8025f5e125fb\",\"checksum-page\":false"
",\"size\":8704,\"timestamp\":1572200000}\n"
"pg_data/base/1/3={\"checksum\":\"%s\",\"checksum-page\":false,\"checksum-page-error\":[0,[2,3]]"
",\"size\":32768,\"timestamp\":1572200000}\n"
"pg_data/base/1/3={\"checksum\":\"%s\",\"checksum-page\":false,\"checksum-page-error\":[0,[2,4]]"
",\"size\":40960,\"timestamp\":1572200000}\n"
"pg_data/base/1/4={\"checksum\":\"%s\",\"checksum-page\":false,\"checksum-page-error\":[1],\"size\":24576"
",\"timestamp\":1572200000}\n"
"pg_data/global/pg_control={\"size\":8192,\"timestamp\":1572200000}\n"