diff --git a/doc/xml/release.xml b/doc/xml/release.xml index b6762f8ac..340c475a5 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -36,6 +36,10 @@

Perl error handler recognizes errors thrown from the C library.

+ + +

Page checksum module uses new C error handler.

+
@@ -58,6 +62,10 @@

Remove Debian test repo after PostgreSQL 10 release.

+ + +

Convert page checksum tests into C unit tests.

+
diff --git a/libc/t/data/page.bin b/libc/t/data/page.bin deleted file mode 100644 index 45286ed68..000000000 Binary files a/libc/t/data/page.bin and /dev/null differ diff --git a/libc/t/pageChecksum.t b/libc/t/pageChecksum.t deleted file mode 100644 index 0d0b64281..000000000 --- a/libc/t/pageChecksum.t +++ /dev/null @@ -1,131 +0,0 @@ -#################################################################################################################################### -# Page Checksum Tests -#################################################################################################################################### -use strict; -use warnings; -use Carp; -use English '-no_match_vars'; - -use Fcntl qw(O_RDONLY); - -# Set number of tests -use Test::More tests => 9; - -# Load the module -use pgBackRest::LibC qw(:checksum); - -sub pageBuild -{ - my $tPageSource = shift; - my $iWalId = shift; - my $iWalOffset = shift; - my $iBlockNo = shift; - - my $tPage = pack('I', $iWalId) . pack('I', $iWalOffset) . substr($tPageSource, 8); - my $iChecksum = pageChecksum($tPage, $iBlockNo, length($tPage)); - - return substr($tPage, 0, 8) . pack('S', $iChecksum) . substr($tPage, 10); -} - -# Test page-level checksums -{ - my $strPageFile = 't/data/page.bin'; - my $iPageSize = 8192; - my $iPageChecksum = 0x1B99; - - # Load the block into a buffer - sysopen(my $hFile, $strPageFile, O_RDONLY) - or confess "unable to open ${strPageFile}"; - - sysread($hFile, my $tBuffer, $iPageSize) == $iPageSize - or confess "unable to read 8192 bytes from ${strPageFile}"; - - close ($hFile); - - # Test the checksum - my $iPageChecksumTest = pageChecksum($tBuffer, 0, $iPageSize); - - ok ( - $iPageChecksumTest == $iPageChecksum, - 'page checksum test (' . sprintf('%X', $iPageChecksumTest) . - ') == page checksum (' . sprintf('%X', $iPageChecksum) . ')'); - - # Test the checksum on a different block no - $iPageChecksumTest = pageChecksum($tBuffer, 1, $iPageSize); - my $iPageChecksumBlockNo = $iPageChecksum + 1; - - ok ( - $iPageChecksumTest == $iPageChecksumBlockNo, - 'page checksum test (' . sprintf('%X', $iPageChecksumTest) . - ') == page checksum blockno (' . sprintf('%X', $iPageChecksumBlockNo) . ')'); - - # Now munge the block and make sure the checksum changes - $iPageChecksumTest = pageChecksum(pack('I', 1024) . substr($tBuffer, 4), 0, $iPageSize); - my $iPageChecksumMunge = 0xFCFF; - - ok ( - $iPageChecksumTest == $iPageChecksumMunge, - 'page checksum test (' . sprintf('%X', $iPageChecksumTest) . - ') == page checksum munge (' . sprintf('%X', $iPageChecksumMunge) . ')'); - - # Pass a valid page buffer - my $tBufferMulti = - $tBuffer . - substr($tBuffer, 0, 8) . pack('S', $iPageChecksum + 1) . substr($tBuffer, 10) . - substr($tBuffer, 0, 8) . pack('S', $iPageChecksum - 2) . substr($tBuffer, 10) . - substr($tBuffer, 0, 8) . pack('S', $iPageChecksum - 1) . substr($tBuffer, 10) . - substr($tBuffer, 0, 8) . pack('S', $iPageChecksum + 4) . substr($tBuffer, 10) . - substr($tBuffer, 0, 8) . pack('S', $iPageChecksum + 5) . substr($tBuffer, 10) . - substr($tBuffer, 0, 8) . pack('S', $iPageChecksum + 2) . substr($tBuffer, 10) . - substr($tBuffer, 0, 8) . pack('S', $iPageChecksum + 3) . substr($tBuffer, 10); - - ok (pageChecksumBufferTest($tBufferMulti, length($tBufferMulti), 0, $iPageSize, 0xFFFF, 0xFFFF), 'pass valid page buffer'); - - # Make sure that an invalid buffer size throws an error - eval - { - pageChecksumBufferTest($tBufferMulti, length($tBufferMulti) - 1, 0, $iPageSize, 0xFFFF, 0xFFFF); - }; - - ok (defined($EVAL_ERROR) && $EVAL_ERROR =~ 'buffer size 65535, page size 8192 are not divisible.*', 'invalid page buffer size'); - - # Allow page with an invalid checksum because LSN >= ignore LSN - $tBufferMulti = - pageBuild($tBuffer, 0, 0, 0) . - pageBuild($tBuffer, 0xFFFF, 0xFFFE, 0) . - pageBuild($tBuffer, 0, 0, 2); - - ok (pageChecksumBufferTest($tBufferMulti, length($tBufferMulti), 0, $iPageSize, 0xFFFF, 0xFFFE), 'skip invalid checksum'); - - # Reject an invalid page buffer (second block will error because the checksum will not contain the correct block no) - ok (!pageChecksumBufferTest($tBufferMulti, length($tBufferMulti), 0, $iPageSize, 0xFFFF, 0xFFFF), 'reject invalid page buffer'); - - # Find the rejected page in the buffer - my $iRejectedBlockNo = -1; - my $iExpectedBlockNo = 1; - - for (my $iIndex = 0; $iIndex < length($tBufferMulti) / $iPageSize; $iIndex++) - { - if (!pageChecksumTest(substr($tBufferMulti, $iIndex * $iPageSize, $iPageSize), $iIndex, $iPageSize, 0xFFFF, 0xFFFF)) - { - $iRejectedBlockNo = $iIndex; - } - } - - ok ($iRejectedBlockNo == $iExpectedBlockNo, "rejected blockno ${iRejectedBlockNo} equals expected ${iExpectedBlockNo}"); - - # Reject an misaligned page buffer - $tBufferMulti = - pageBuild($tBuffer, 0, 0, 0) . - substr(pageBuild($tBuffer, 0, 0, 1), 1); - - eval - { - pageChecksumBufferTest($tBufferMulti, length($tBufferMulti), 0, $iPageSize, 0xFFFF, 0xFFFF); - ok (0, 'misaligned test should have failed'); - } - or do - { - ok (1, 'misaligned test failed'); - }; -} diff --git a/libc/xs/postgres/pageChecksum.xs b/libc/xs/postgres/pageChecksum.xs index 997827716..74eb02e86 100644 --- a/libc/xs/postgres/pageChecksum.xs +++ b/libc/xs/postgres/pageChecksum.xs @@ -10,8 +10,14 @@ pageChecksum(page, blockNo, pageSize) U32 blockNo U32 pageSize CODE: - RETVAL = pageChecksum( - (const unsigned char *)page, blockNo, pageSize); + RETVAL = 0; + + ERROR_XS_BEGIN() + { + RETVAL = pageChecksum( + (const unsigned char *)page, blockNo, pageSize); + } + ERROR_XS_END(); OUTPUT: RETVAL @@ -23,8 +29,14 @@ pageChecksumTest(page, blockNo, pageSize, ignoreWalId, ignoreWalOffset) U32 ignoreWalId U32 ignoreWalOffset CODE: - RETVAL = pageChecksumTest( - (const unsigned char *)page, blockNo, pageSize, ignoreWalId, ignoreWalOffset); + RETVAL = false; + + ERROR_XS_BEGIN() + { + RETVAL = pageChecksumTest( + (const unsigned char *)page, blockNo, pageSize, ignoreWalId, ignoreWalOffset); + } + ERROR_XS_END(); OUTPUT: RETVAL @@ -37,7 +49,13 @@ pageChecksumBufferTest(pageBuffer, pageBufferSize, blockNoBegin, pageSize, ignor U32 ignoreWalId U32 ignoreWalOffset CODE: - RETVAL = pageChecksumBufferTest( - (const unsigned char *)pageBuffer, pageBufferSize, blockNoBegin, pageSize, ignoreWalId, ignoreWalOffset); + RETVAL = false; + + ERROR_XS_BEGIN() + { + RETVAL = pageChecksumBufferTest( + (const unsigned char *)pageBuffer, pageBufferSize, blockNoBegin, pageSize, ignoreWalId, ignoreWalOffset); + } + ERROR_XS_END(); OUTPUT: RETVAL diff --git a/src/postgres/pageChecksum.c b/src/postgres/pageChecksum.c index 1e3157a76..34570b1da 100644 --- a/src/postgres/pageChecksum.c +++ b/src/postgres/pageChecksum.c @@ -62,12 +62,10 @@ calculate a subset of the columns at a time and perform multiple passes to avoid is not used. Current coding also assumes that the compiler has the ability to unroll the inner loop to avoid loop overhead and minimize register spilling. For less sophisticated compilers it might be beneficial to manually unroll the inner loop. ***********************************************************************************************************************************/ -#include "EXTERN.h" -#include "perl.h" - #include -#include "common/type.h" +#include "common/error.h" +#include "postgres/pageChecksum.h" /*********************************************************************************************************************************** For historical reasons, the 64-bit LSN value is stored as two 32-bit values. @@ -211,9 +209,7 @@ pageChecksumBufferTest( { // If the buffer does not represent an even number of pages then error if (pageBufferSize % pageSize != 0 || pageBufferSize / pageSize == 0) - { - croak("buffer size %lu, page size %lu are not divisible", pageBufferSize, pageSize); - } + ERROR_THROW(AssertError, "buffer size %lu, page size %lu are not divisible", pageBufferSize, pageSize); // Loop through all pages in the buffer for (int pageIdx = 0; pageIdx < pageBufferSize / pageSize; pageIdx++) diff --git a/test/lib/pgBackRestTest/Common/DefineTest.pm b/test/lib/pgBackRestTest/Common/DefineTest.pm index b8a4338d7..8bb7ab125 100644 --- a/test/lib/pgBackRestTest/Common/DefineTest.pm +++ b/test/lib/pgBackRestTest/Common/DefineTest.pm @@ -174,6 +174,25 @@ my $oTestDef = }, ] }, + # PostgreSQL tests + { + &TESTDEF_NAME => 'postgres', + &TESTDEF_CONTAINER => true, + + &TESTDEF_TEST => + [ + { + &TESTDEF_NAME => 'page-checksum', + &TESTDEF_TOTAL => 3, + &TESTDEF_C => true, + + &TESTDEF_COVERAGE => + { + 'postgres/pageChecksum' => TESTDEF_COVERAGE_FULL, + }, + }, + ] + }, # Help tests { &TESTDEF_NAME => 'help', diff --git a/test/lib/pgBackRestTest/Common/JobTest.pm b/test/lib/pgBackRestTest/Common/JobTest.pm index d0954027c..c66aff8e4 100644 --- a/test/lib/pgBackRestTest/Common/JobTest.pm +++ b/test/lib/pgBackRestTest/Common/JobTest.pm @@ -246,9 +246,6 @@ sub run # Skip all files except .c files (including .auto.c) next if $strFile !~ /(?{substr($strFile, 0, length($strFile) - 2)})) { push(@stryCFile, "${strCSrcPath}/${strFile}"); diff --git a/test/src/module/postgres/pageChecksumTest.c b/test/src/module/postgres/pageChecksumTest.c new file mode 100644 index 000000000..763752f22 --- /dev/null +++ b/test/src/module/postgres/pageChecksumTest.c @@ -0,0 +1,105 @@ +/*********************************************************************************************************************************** +Test Page Checksums +***********************************************************************************************************************************/ + +/*********************************************************************************************************************************** +Page data for testing -- use 8192 for page size since this is the most common value +***********************************************************************************************************************************/ +#define TEST_PAGE_SIZE 8192 +#define TEST_PAGE_TOTAL 16 + +static unsigned char testPageBuffer[TEST_PAGE_TOTAL][TEST_PAGE_SIZE]; + +/*********************************************************************************************************************************** +Test Run +***********************************************************************************************************************************/ +void testRun() +{ + // ----------------------------------------------------------------------------------------------------------------------------- + if (testBegin("pageChecksum()")) + { + // Checksum for 0x00 fill, page 0x00 + memset(testPageBuffer[0], 0, TEST_PAGE_SIZE); + TEST_RESULT_U16_HEX(pageChecksum(testPageBuffer[0], 0, TEST_PAGE_SIZE), 0xC6AA, "check for 0x00 filled page, block 0"); + + // Checksum for 0xFF fill, page 0x00 + memset(testPageBuffer[0], 0xFF, TEST_PAGE_SIZE); + TEST_RESULT_U16_HEX(pageChecksum(testPageBuffer[0], 0, TEST_PAGE_SIZE), 0x0E1C, "check for 0xFF filled page, block 0"); + + // Checksum for 0xFF fill, page 0xFF + memset(testPageBuffer[0], 0xFF, TEST_PAGE_SIZE); + TEST_RESULT_U16_HEX(pageChecksum(testPageBuffer[0], 999, TEST_PAGE_SIZE), 0x0EC3, "check for 0xFF filled page, block 999"); + } + + // ----------------------------------------------------------------------------------------------------------------------------- + if (testBegin("pageChecksumTest()")) + { + // Zero the pages + memset(testPageBuffer, 0, TEST_PAGE_TOTAL * TEST_PAGE_SIZE); + + // Pages with pd_upper = 0 should always return true no matter the block no + TEST_RESULT_BOOL(pageChecksumTest(testPageBuffer[0], 0, TEST_PAGE_SIZE, 0, 0), true, "pd_upper is 0, block 0"); + TEST_RESULT_BOOL(pageChecksumTest(testPageBuffer[1], 999, TEST_PAGE_SIZE, 0, 0), true, "pd_upper is 0, block 999"); + + // Update pd_upper and check for failure no matter the block no + ((PageHeader)testPageBuffer[0])->pd_upper = 0x00FF; + TEST_RESULT_BOOL(pageChecksumTest(testPageBuffer[0], 0, TEST_PAGE_SIZE, 0xFFFF, 0xFFFF), false, "badchecksum, page 0"); + TEST_RESULT_BOOL( + pageChecksumTest(testPageBuffer[0], 9999, TEST_PAGE_SIZE, 0xFFFF, 0xFFFF), false, "badchecksum, page 9999"); + + // Update LSNs and check that page checksums past the ignore limits are successful + ((PageHeader)testPageBuffer[0])->pd_lsn.walid = 0x8888; + ((PageHeader)testPageBuffer[0])->pd_lsn.xrecoff = 0x8888; + + TEST_RESULT_BOOL( + pageChecksumTest(testPageBuffer[0], 0, TEST_PAGE_SIZE, 0x8888, 0x8888), true, "bad checksum past ignore limit"); + TEST_RESULT_BOOL( + pageChecksumTest(testPageBuffer[0], 0, TEST_PAGE_SIZE, 0x8889, 0x8889), false, "bad checksum before ignore limit"); + } + + // ----------------------------------------------------------------------------------------------------------------------------- + if (testBegin("pageChecksumBufferTest()")) + { + // Check that assertion fails if page buffer and page size are not divisible + TEST_ERROR( + pageChecksumBufferTest(testPageBuffer[0], TEST_PAGE_TOTAL * TEST_PAGE_SIZE - 1, 0, TEST_PAGE_SIZE, 0, 0), + AssertError, "buffer size 131071, page size 8192 are not divisible"); + + // Create pages that will pass the test (starting with block 0) + for (int pageIdx = 0; pageIdx < TEST_PAGE_TOTAL; pageIdx++) + { + // Don't fill with zero because zeroes will succeed on the pd_upper check + memset(testPageBuffer[pageIdx], 0x77, TEST_PAGE_SIZE); + + ((PageHeader)testPageBuffer[pageIdx])->pd_checksum = pageChecksum(testPageBuffer[pageIdx], pageIdx, TEST_PAGE_SIZE); + } + + TEST_RESULT_BOOL( + pageChecksumBufferTest(testPageBuffer[0], TEST_PAGE_TOTAL * TEST_PAGE_SIZE, 0, TEST_PAGE_SIZE, 0xFFFFFFFF, 0xFFFFFFFF), + true, "valid page buffer starting at block 0"); + + // Create pages that will pass the test (beginning with block <> 0) + int blockBegin = 999; + + for (int pageIdx = 0; pageIdx < TEST_PAGE_TOTAL; pageIdx++) + { + ((PageHeader)testPageBuffer[pageIdx])->pd_checksum = pageChecksum( + testPageBuffer[pageIdx], pageIdx + blockBegin, TEST_PAGE_SIZE); + } + + TEST_RESULT_BOOL( + pageChecksumBufferTest( + testPageBuffer[0], TEST_PAGE_TOTAL * TEST_PAGE_SIZE, blockBegin, TEST_PAGE_SIZE, 0xFFFFFFFF, 0xFFFFFFFF), + true, "valid page buffer starting at block 999"); + + // Break the checksum for a page and make sure it is found + int pageInvalid = 7; + assert(pageInvalid >= 0 && pageInvalid < TEST_PAGE_TOTAL); + ((PageHeader)testPageBuffer[pageInvalid])->pd_checksum = 0xEEEE; + + TEST_RESULT_BOOL( + pageChecksumBufferTest( + testPageBuffer[0], TEST_PAGE_TOTAL * TEST_PAGE_SIZE, blockBegin, TEST_PAGE_SIZE, 0xFFFFFFFF, 0xFFFFFFFF), + false, "invalid page buffer"); + } +}