MDEV-11799 InnoDB can abort if the doublewrite buffer

contains a bad and a good copy

Clean up the InnoDB doublewrite buffer code.

buf_dblwr_init_or_load_pages(): Do not add empty pages to the buffer.

buf_dblwr_process(): Do consider changes to pages that are all zero.
Do not abort when finding a corrupted copy of a page in the doublewrite
buffer, because there could be multiple copies in the doublewrite buffer,
and only one of them needs to be good.

2 files changed, 188 insertions, 255 deletions

diff --git a/storage/innobase/buf/buf0dblwr.cc b/storage/innobase/buf/buf0dblwr.cc
index 1cf856a731f..0b42e38a655 100644
--- a/storage/innobase/buf/buf0dblwr.cc
+++ b/storage/innobase/buf/buf0dblwr.cc
@@ -455,8 +455,11 @@ buf_dblwr_init_or_load_pages(
 			os_file_write(path, file, page,
 				      source_page_no * UNIV_PAGE_SIZE,
 				      UNIV_PAGE_SIZE);
-		} else if (load_corrupt_pages) {
-
+		} else if (load_corrupt_pages
+			   && !buf_page_is_zeroes(page, FIL_PAGE_DATA)) {
+			/* Each valid page header must contain some
+			nonzero bytes, such as FIL_PAGE_OFFSET
+			or FIL_PAGE_LSN. */
 			recv_dblwr.add(page);
 		}
 
@@ -492,8 +495,6 @@ buf_dblwr_process()
 
 	for (std::list<byte*>::iterator i = recv_dblwr.pages.begin();
 	     i != recv_dblwr.pages.end(); ++i, ++page_no_dblwr ) {
-		bool is_compressed = false;
-
 		page = *i;
 		page_no  = mach_read_from_4(page + FIL_PAGE_OFFSET);
 		space_id = mach_read_from_4(page + FIL_PAGE_SPACE_ID);
@@ -501,139 +502,104 @@ buf_dblwr_process()
 		if (!fil_tablespace_exists_in_mem(space_id)) {
 			/* Maybe we have dropped the single-table tablespace
 			and this page once belonged to it: do nothing */
+			continue;
+		}
 
-		} else if (!fil_check_adress_in_tablespace(space_id,
-							   page_no)) {
+		if (!fil_check_adress_in_tablespace(space_id, page_no)) {
 			ib_logf(IB_LOG_LEVEL_WARN,
-				"A page in the doublewrite buffer is not "
-				"within space bounds; space id %lu "
-				"page number %lu, page %lu in "
-				"doublewrite buf.",
-				(ulong) space_id, (ulong) page_no,
-				page_no_dblwr);
+				"A copy of page " ULINTPF ":" ULINTPF
+				" in the doublewrite buffer slot " ULINTPF
+				" is not within space bounds",
+				space_id, page_no, page_no_dblwr);
+			continue;
+		}
+
+		ulint	zip_size = fil_space_get_zip_size(space_id);
+		ut_ad(!buf_page_is_zeroes(page, zip_size));
+
+		/* Read in the actual page from the file */
+		fil_io(OS_FILE_READ,
+		       true,
+		       space_id,
+		       zip_size,
+		       page_no,
+		       0,
+		       zip_size ? zip_size : UNIV_PAGE_SIZE,
+		       read_buf,
+		       NULL,
+		       0);
+
+		const bool is_all_zero = buf_page_is_zeroes(
+			read_buf, zip_size);
+
+		if (is_all_zero) {
+			/* We will check if the copy in the
+			doublewrite buffer is valid. If not, we will
+			ignore this page (there should be redo log
+			records to initialize it). */
 		} else {
-			ulint	zip_size = fil_space_get_zip_size(space_id);
-
-			/* Read in the actual page from the file */
-			fil_io(OS_FILE_READ,
-				true,
-				space_id,
-				zip_size,
-				page_no,
-				0,
-				zip_size ? zip_size : UNIV_PAGE_SIZE,
-				read_buf,
-				NULL,
-				0);
-
-			/* Is page compressed ? */
-			is_compressed = fil_page_is_compressed_encrypted(read_buf) |
-				fil_page_is_compressed(read_buf);
-
-			/* If page was compressed, decompress it before we
-			check checksum. */
-			if (is_compressed) {
-				fil_decompress_page(NULL, read_buf, UNIV_PAGE_SIZE, NULL, true);
+			if (fil_page_is_compressed_encrypted(read_buf) ||
+			    fil_page_is_compressed(read_buf)) {
+				/* Decompress the page before
+				validating the checksum. */
+				fil_decompress_page(
+					NULL, read_buf, UNIV_PAGE_SIZE,
+					NULL, true);
 			}
 
-			if (fil_space_verify_crypt_checksum(read_buf, zip_size)) {
-				/* page is encrypted and checksum is OK */
-			} else if (buf_page_is_corrupted(true, read_buf, zip_size)) {
-
-				fprintf(stderr,
-					"InnoDB: Warning: database page"
-					" corruption or a failed\n"
-					"InnoDB: file read of"
-					" space %lu page %lu.\n"
-					"InnoDB: Trying to recover it from"
-					" the doublewrite buffer.\n",
-					(ulong) space_id, (ulong) page_no);
-
-				/* Is page compressed ? */
-				is_compressed = fil_page_is_compressed_encrypted(page) |
-					fil_page_is_compressed(page);
-
-				/* If page was compressed, decompress it before we
-				check checksum. */
-				if (is_compressed) {
-					fil_decompress_page(NULL, page, UNIV_PAGE_SIZE, NULL, true);
-				}
+			if (fil_space_verify_crypt_checksum(
+				   read_buf, zip_size)
+			   || !buf_page_is_corrupted(
+				   true, read_buf, zip_size)) {
+				/* The page is good; there is no need
+				to consult the doublewrite buffer. */
+				continue;
+			}
 
-				if (fil_space_verify_crypt_checksum(page, zip_size)) {
-					/* the doublewrite buffer page is encrypted and OK */
-				} else if (buf_page_is_corrupted(true,
-								 page,
-								 zip_size)) {
-					fprintf(stderr,
-						"InnoDB: Dump of the page:\n");
-					buf_page_print(
-						read_buf, zip_size,
-						BUF_PAGE_PRINT_NO_CRASH);
-					fprintf(stderr,
-						"InnoDB: Dump of"
-						" corresponding page"
-						" in doublewrite buffer:\n");
-					buf_page_print(
-						page, zip_size,
-						BUF_PAGE_PRINT_NO_CRASH);
-
-					fprintf(stderr,
-						"InnoDB: Also the page in the"
-						" doublewrite buffer"
-						" is corrupt.\n"
-						"InnoDB: Cannot continue"
-						" operation.\n"
-						"InnoDB: You can try to"
-						" recover the database"
-						" with the my.cnf\n"
-						"InnoDB: option:\n"
-						"InnoDB:"
-						" innodb_force_recovery=6\n");
-					ut_error;
-				}
+			/* We intentionally skip this message for
+			is_all_zero pages. */
+			ib_logf(IB_LOG_LEVEL_INFO,
+				"Trying to recover page " ULINTPF ":" ULINTPF
+				" from the doublewrite buffer.",
+				space_id, page_no);
+		}
 
-				/* Write the good page from the
-				doublewrite buffer to the intended
-				position */
-
-				fil_io(OS_FILE_WRITE,
-					true,
-					space_id,
-					zip_size,
-					page_no,
-					0,
-					zip_size ? zip_size : UNIV_PAGE_SIZE,
-					page,
-					NULL,
-					0);
-
-				ib_logf(IB_LOG_LEVEL_INFO,
-					"Recovered the page from"
-					" the doublewrite buffer.");
-
-			} else if (buf_page_is_zeroes(read_buf, zip_size)) {
-
-				if (!buf_page_is_zeroes(page, zip_size)
-				    && !buf_page_is_corrupted(true, page,
-							      zip_size)) {
-
-					/* Database page contained only
-					zeroes, while a valid copy is
-					available in dblwr buffer. */
-
-					fil_io(OS_FILE_WRITE,
-						true,
-						space_id,
-						zip_size,
-						page_no,
-						0,
-						zip_size ? zip_size : UNIV_PAGE_SIZE,
-						page,
-						NULL,
-						0);
-				}
+		/* Next, validate the doublewrite page. */
+		if (fil_page_is_compressed_encrypted(page) ||
+		    fil_page_is_compressed(page)) {
+			/* Decompress the page before
+			validating the checksum. */
+			fil_decompress_page(
+				NULL, page, UNIV_PAGE_SIZE, NULL, true);
+		}
+
+		if (!fil_space_verify_crypt_checksum(page, zip_size)
+		    && buf_page_is_corrupted(true, page, zip_size)) {
+			if (!is_all_zero) {
+				ib_logf(IB_LOG_LEVEL_WARN,
+					"A doublewrite copy of page "
+					ULINTPF ":" ULINTPF " is corrupted.",
+					space_id, page_no);
 			}
+			/* Theoretically we could have another good
+			copy for this page in the doublewrite
+			buffer. If not, we will report a fatal error
+			for a corrupted page somewhere else if that
+			page was truly needed. */
+			continue;
 		}
+
+		/* Write the good page from the doublewrite buffer to
+		the intended position. */
+
+		fil_io(OS_FILE_WRITE, true, space_id, zip_size, page_no, 0,
+		       zip_size ? zip_size : UNIV_PAGE_SIZE,
+		       page, NULL, 0);
+
+		ib_logf(IB_LOG_LEVEL_INFO,
+			"Recovered page " ULINTPF ":" ULINTPF " from"
+			" the doublewrite buffer.",
+			space_id, page_no);
 	}
 
 	ut_free(unaligned_read_buf);
diff --git a/storage/xtradb/buf/buf0dblwr.cc b/storage/xtradb/buf/buf0dblwr.cc
index 7f6b6caee9d..9c7eea6410a 100644
--- a/storage/xtradb/buf/buf0dblwr.cc
+++ b/storage/xtradb/buf/buf0dblwr.cc
@@ -455,8 +455,11 @@ buf_dblwr_init_or_load_pages(
 			os_file_write(path, file, page,
 				      source_page_no * UNIV_PAGE_SIZE,
 				      UNIV_PAGE_SIZE);
-		} else if (load_corrupt_pages) {
-
+		} else if (load_corrupt_pages
+			   && !buf_page_is_zeroes(page, FIL_PAGE_DATA)) {
+			/* Each valid page header must contain some
+			nonzero bytes, such as FIL_PAGE_OFFSET
+			or FIL_PAGE_LSN. */
 			recv_dblwr.add(page);
 		}
 
@@ -492,8 +495,6 @@ buf_dblwr_process()
 
 	for (std::list<byte*>::iterator i = recv_dblwr.pages.begin();
 	     i != recv_dblwr.pages.end(); ++i, ++page_no_dblwr ) {
-		bool is_compressed = false;
-
 		page = *i;
 		page_no  = mach_read_from_4(page + FIL_PAGE_OFFSET);
 		space_id = mach_read_from_4(page + FIL_PAGE_SPACE_ID);
@@ -501,138 +502,104 @@ buf_dblwr_process()
 		if (!fil_tablespace_exists_in_mem(space_id)) {
 			/* Maybe we have dropped the single-table tablespace
 			and this page once belonged to it: do nothing */
+			continue;
+		}
 
-		} else if (!fil_check_adress_in_tablespace(space_id,
-							   page_no)) {
+		if (!fil_check_adress_in_tablespace(space_id, page_no)) {
 			ib_logf(IB_LOG_LEVEL_WARN,
-				"A page in the doublewrite buffer is not "
-				"within space bounds; space id %lu "
-				"page number %lu, page %lu in "
-				"doublewrite buf.",
-				(ulong) space_id, (ulong) page_no,
-				page_no_dblwr);
+				"A copy of page " ULINTPF ":" ULINTPF
+				" in the doublewrite buffer slot " ULINTPF
+				" is not within space bounds",
+				space_id, page_no, page_no_dblwr);
+			continue;
+		}
+
+		ulint	zip_size = fil_space_get_zip_size(space_id);
+		ut_ad(!buf_page_is_zeroes(page, zip_size));
+
+		/* Read in the actual page from the file */
+		fil_io(OS_FILE_READ,
+		       true,
+		       space_id,
+		       zip_size,
+		       page_no,
+		       0,
+		       zip_size ? zip_size : UNIV_PAGE_SIZE,
+		       read_buf,
+		       NULL,
+		       0);
+
+		const bool is_all_zero = buf_page_is_zeroes(
+			read_buf, zip_size);
+
+		if (is_all_zero) {
+			/* We will check if the copy in the
+			doublewrite buffer is valid. If not, we will
+			ignore this page (there should be redo log
+			records to initialize it). */
 		} else {
-			ulint	zip_size = fil_space_get_zip_size(space_id);
-
-			/* Read in the actual page from the file */
-			fil_io(OS_FILE_READ,
-				true,
-				space_id,
-				zip_size,
-				page_no,
-				0,
-				zip_size ? zip_size : UNIV_PAGE_SIZE,
-				read_buf,
-				NULL,
-				0);
-
-			/* Is page compressed ? */
-			is_compressed = fil_page_is_compressed_encrypted(read_buf) |
-				fil_page_is_compressed(read_buf);
-
-			/* If page was compressed, decompress it before we
-			check checksum. */
-			if (is_compressed) {
-				fil_decompress_page(NULL, read_buf, UNIV_PAGE_SIZE, NULL, true);
+			if (fil_page_is_compressed_encrypted(read_buf) ||
+			    fil_page_is_compressed(read_buf)) {
+				/* Decompress the page before
+				validating the checksum. */
+				fil_decompress_page(
+					NULL, read_buf, UNIV_PAGE_SIZE,
+					NULL, true);
 			}
 
-			if (fil_space_verify_crypt_checksum(read_buf, zip_size)) {
-				/* page is encrypted and checksum is OK */
-			} else if (buf_page_is_corrupted(true, read_buf, zip_size)) {
-
-				fprintf(stderr,
-					"InnoDB: Database page"
-					" corruption or a failed\n"
-					"InnoDB: file read of"
-					" space %lu page %lu.\n"
-					"InnoDB: Trying to recover it from"
-					" the doublewrite buffer.\n",
-					(ulong) space_id, (ulong) page_no);
-
-				/* Is page compressed ? */
-				is_compressed = fil_page_is_compressed_encrypted(page) |
-					fil_page_is_compressed(page);
-
-				/* If page was compressed, decompress it before we
-				check checksum. */
-				if (is_compressed) {
-					fil_decompress_page(NULL, page, UNIV_PAGE_SIZE, NULL, true);
-				}
+			if (fil_space_verify_crypt_checksum(
+				   read_buf, zip_size)
+			   || !buf_page_is_corrupted(
+				   true, read_buf, zip_size)) {
+				/* The page is good; there is no need
+				to consult the doublewrite buffer. */
+				continue;
+			}
 
-				if (fil_space_verify_crypt_checksum(page, zip_size)) {
-					/* the doublewrite buffer page is encrypted and OK */
-				} else if (buf_page_is_corrupted(true,
-								 page,
-								 zip_size)) {
-					fprintf(stderr,
-						"InnoDB: Dump of the page:\n");
-					buf_page_print(
-						read_buf, zip_size,
-						BUF_PAGE_PRINT_NO_CRASH);
-					fprintf(stderr,
-						"InnoDB: Dump of"
-						" corresponding page"
-						" in doublewrite buffer:\n");
-					buf_page_print(
-						page, zip_size,
-						BUF_PAGE_PRINT_NO_CRASH);
-
-					fprintf(stderr,
-						"InnoDB: Also the page in the"
-						" doublewrite buffer"
-						" is corrupt.\n"
-						"InnoDB: Cannot continue"
-						" operation.\n"
-						"InnoDB: You can try to"
-						" recover the database"
-						" with the my.cnf\n"
-						"InnoDB: option:\n"
-						"InnoDB:"
-						" innodb_force_recovery=6\n");
-					ut_error;
-				}
+			/* We intentionally skip this message for
+			is_all_zero pages. */
+			ib_logf(IB_LOG_LEVEL_INFO,
+				"Trying to recover page " ULINTPF ":" ULINTPF
+				" from the doublewrite buffer.",
+				space_id, page_no);
+		}
 
-				/* Write the good page from the
-				doublewrite buffer to the intended
-				position */
-
-				fil_io(OS_FILE_WRITE,
-					true,
-					space_id,
-					zip_size,
-					page_no,
-					0,
-					zip_size ? zip_size : UNIV_PAGE_SIZE,
-					page,
-					NULL,
-					0);
-
-				ib_logf(IB_LOG_LEVEL_INFO,
-					"Recovered the page from"
-					" the doublewrite buffer.");
-
-			} else if (buf_page_is_zeroes(read_buf, zip_size)) {
-
-				if (!buf_page_is_zeroes(page, zip_size)
-				    && !buf_page_is_corrupted(true, page,
-							      zip_size)) {
-
-					/* Database page contained only
-					zeroes, while a valid copy is
-					available in dblwr buffer. */
-
-					fil_io(OS_FILE_WRITE,
-						true,
-						space_id,
-						zip_size,
-						page_no, 0,
-						zip_size ? zip_size : UNIV_PAGE_SIZE,
-						page,
-						NULL,
-						0);
-				}
+		/* Next, validate the doublewrite page. */
+		if (fil_page_is_compressed_encrypted(page) ||
+		    fil_page_is_compressed(page)) {
+			/* Decompress the page before
+			validating the checksum. */
+			fil_decompress_page(
+				NULL, page, UNIV_PAGE_SIZE, NULL, true);
+		}
+
+		if (!fil_space_verify_crypt_checksum(page, zip_size)
+		    && buf_page_is_corrupted(true, page, zip_size)) {
+			if (!is_all_zero) {
+				ib_logf(IB_LOG_LEVEL_WARN,
+					"A doublewrite copy of page "
+					ULINTPF ":" ULINTPF " is corrupted.",
+					space_id, page_no);
 			}
+			/* Theoretically we could have another good
+			copy for this page in the doublewrite
+			buffer. If not, we will report a fatal error
+			for a corrupted page somewhere else if that
+			page was truly needed. */
+			continue;
 		}
+
+		/* Write the good page from the doublewrite buffer to
+		the intended position. */
+
+		fil_io(OS_FILE_WRITE, true, space_id, zip_size, page_no, 0,
+		       zip_size ? zip_size : UNIV_PAGE_SIZE,
+		       page, NULL, 0);
+
+		ib_logf(IB_LOG_LEVEL_INFO,
+			"Recovered page " ULINTPF ":" ULINTPF " from"
+			" the doublewrite buffer.",
+			space_id, page_no);
 	}
 
 	ut_free(unaligned_read_buf);