Commit 303448bc authored by Marko Mäkelä's avatar Marko Mäkelä

MDEV-27931: buf_page_is_corrupted() wrongly claims corruption

In commit 437da7bc (MDEV-19534),
the default value of the global variable srv_checksum_algorithm
in innochecksum was changed from SRV_CHECKSUM_ALGORITHM_INNODB
to implied 0 (innodb_checksum_algorithm=crc32). As a result,
the function buf_page_is_corrupted() would by default invoke
buf_calc_page_crc32() in innochecksum, and crc32_inited would hold.

This would cause "innochecksum" to fail on a particular page.

The actual problem is older, introduced in 2011 in
mysql/mysql-server@17e497bdb793bc6b8360aa1c626dcd8bb5cfad1b
(MySQL 5.6.3). It should affect the validation of pages of old
data files that were written with innodb_checksum_algorithm=innodb.
When using innodb_checksum_algorithm=crc32 (the default setting
since MariaDB Server 10.2), some valid pages would be rejected
only because exactly one of the two checksum fields accidentally
matches the innodb_checksum_algorithm=crc32 value.

buf_page_is_corrupted(): Simplify the logic of non-strict
checksum validation, by always invoking buf_calc_page_crc32().
Remove a bogus condition that if only one of the checksum fields
contains the value returned by buf_calc_page_crc32(), the page
is corrupted.
parent 7af133cc
...@@ -977,8 +977,6 @@ buf_page_is_corrupted( ...@@ -977,8 +977,6 @@ buf_page_is_corrupted(
#endif #endif
size_t checksum_field1 = 0; size_t checksum_field1 = 0;
size_t checksum_field2 = 0; size_t checksum_field2 = 0;
uint32_t crc32 = 0;
bool crc32_inited = false;
ulint page_type = mach_read_from_2(read_buf + FIL_PAGE_TYPE); ulint page_type = mach_read_from_2(read_buf + FIL_PAGE_TYPE);
...@@ -1104,8 +1102,13 @@ buf_page_is_corrupted( ...@@ -1104,8 +1102,13 @@ buf_page_is_corrupted(
case SRV_CHECKSUM_ALGORITHM_STRICT_NONE: case SRV_CHECKSUM_ALGORITHM_STRICT_NONE:
return !buf_page_is_checksum_valid_none( return !buf_page_is_checksum_valid_none(
read_buf, checksum_field1, checksum_field2); read_buf, checksum_field1, checksum_field2);
case SRV_CHECKSUM_ALGORITHM_NONE:
/* should have returned false earlier */
break;
case SRV_CHECKSUM_ALGORITHM_CRC32: case SRV_CHECKSUM_ALGORITHM_CRC32:
case SRV_CHECKSUM_ALGORITHM_INNODB: case SRV_CHECKSUM_ALGORITHM_INNODB:
const uint32_t crc32 = buf_calc_page_crc32(read_buf);
if (buf_page_is_checksum_valid_none(read_buf, if (buf_page_is_checksum_valid_none(read_buf,
checksum_field1, checksum_field2)) { checksum_field1, checksum_field2)) {
#ifdef UNIV_INNOCHECKSUM #ifdef UNIV_INNOCHECKSUM
...@@ -1121,7 +1124,7 @@ buf_page_is_corrupted( ...@@ -1121,7 +1124,7 @@ buf_page_is_corrupted(
" crc32 = " UINT32PF "; recorded = " ULINTPF ";\n", " crc32 = " UINT32PF "; recorded = " ULINTPF ";\n",
cur_page_num, cur_page_num,
buf_calc_page_new_checksum(read_buf), buf_calc_page_new_checksum(read_buf),
buf_calc_page_crc32(read_buf), crc32,
checksum_field1); checksum_field1);
} }
#endif /* UNIV_INNOCHECKSUM */ #endif /* UNIV_INNOCHECKSUM */
...@@ -1138,84 +1141,33 @@ buf_page_is_corrupted( ...@@ -1138,84 +1141,33 @@ buf_page_is_corrupted(
!= mach_read_from_4(read_buf + FIL_PAGE_LSN) != mach_read_from_4(read_buf + FIL_PAGE_LSN)
&& checksum_field2 != BUF_NO_CHECKSUM_MAGIC) { && checksum_field2 != BUF_NO_CHECKSUM_MAGIC) {
if (curr_algo == SRV_CHECKSUM_ALGORITHM_CRC32) {
DBUG_EXECUTE_IF( DBUG_EXECUTE_IF(
"page_intermittent_checksum_mismatch", { "page_intermittent_checksum_mismatch", {
static int page_counter; static int page_counter;
if (page_counter++ == 2) { if (page_counter++ == 2) return true;
checksum_field2++;
}
}); });
crc32 = buf_page_check_crc32(read_buf, if ((checksum_field1 != crc32
checksum_field2); || checksum_field2 != crc32)
crc32_inited = true;
if (checksum_field2 != crc32
&& checksum_field2 && checksum_field2
!= buf_calc_page_old_checksum(read_buf)) { != buf_calc_page_old_checksum(read_buf)) {
return true; return true;
} }
} else {
ut_ad(curr_algo
== SRV_CHECKSUM_ALGORITHM_INNODB);
if (checksum_field2
!= buf_calc_page_old_checksum(read_buf)) {
crc32 = buf_page_check_crc32(
read_buf, checksum_field2);
crc32_inited = true;
if (checksum_field2 != crc32) {
return true;
}
}
}
}
if (checksum_field1 == 0
|| checksum_field1 == BUF_NO_CHECKSUM_MAGIC) {
} else if (curr_algo == SRV_CHECKSUM_ALGORITHM_CRC32) {
if (!crc32_inited) {
crc32 = buf_page_check_crc32(
read_buf, checksum_field2);
crc32_inited = true;
} }
if (checksum_field1 != crc32 switch (checksum_field1) {
case 0:
case BUF_NO_CHECKSUM_MAGIC:
break;
default:
if ((checksum_field1 != crc32
|| checksum_field2 != crc32)
&& checksum_field1 && checksum_field1
!= buf_calc_page_new_checksum(read_buf)) { != buf_calc_page_new_checksum(read_buf)) {
return true; return true;
} }
} else {
ut_ad(curr_algo == SRV_CHECKSUM_ALGORITHM_INNODB);
if (checksum_field1
!= buf_calc_page_new_checksum(read_buf)) {
if (!crc32_inited) {
crc32 = buf_page_check_crc32(
read_buf, checksum_field2);
crc32_inited = true;
}
if (checksum_field1 != crc32) {
return true;
}
}
} }
if (crc32_inited
&& ((checksum_field1 == crc32
&& checksum_field2 != crc32)
|| (checksum_field1 != crc32
&& checksum_field2 == crc32))) {
return true;
}
break;
case SRV_CHECKSUM_ALGORITHM_NONE:
/* should have returned false earlier */
break; break;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment