[Commits] 5ac026e60a8: MDEV-13591: InnoDB: Database page corruption on disk or a failed file read and assertion failure

jan jan.lindstrom at mariadb.com
Mon Aug 28 09:48:38 EEST 2017


revision-id: 5ac026e60a86dba42a3e76323ceaab41be7d098b (mariadb-10.1.26-11-g5ac026e60a8)
parent(s): 33bbe6ff5fc26da0672b257e9ed8a98c77e397f9
author: Jan Lindström
committer: Jan Lindström
timestamp: 2017-08-28 09:45:54 +0300
message:

MDEV-13591: InnoDB: Database page corruption on disk or a failed file read and assertion failure

Problem is that page 0 and its possible enrryption information
is not read for undo tablespaces.

fil_crypt_get_latest_key_version(): Do not send event to
encryption threads if event does not yet exists. Seen
on regression testing.

fil_read_first_page: Add new parameter does page belong to
undo tablespace and if it does, we do not read FSP_HEADER.

srv_undo_tablespace_open : Read first page of the tablespace
to get crypt_data if it exists and pass it to fil_space_create.

Tested using innodb_encryption with combinations with
innodb-undo-tablespaces.

---
 .../suite/encryption/r/innodb_encryption.result    |  2 +
 .../suite/encryption/t/innodb_encryption.test      |  4 ++
 storage/innobase/fil/fil0crypt.cc                  |  7 +++-
 storage/innobase/fil/fil0fil.cc                    | 49 ++++++++++++----------
 storage/innobase/include/fil0fil.h                 |  5 ++-
 storage/innobase/srv/srv0start.cc                  | 29 ++++++++++---
 storage/xtradb/fil/fil0crypt.cc                    |  7 +++-
 storage/xtradb/fil/fil0fil.cc                      | 49 ++++++++++++----------
 storage/xtradb/include/fil0fil.h                   |  9 ++--
 storage/xtradb/srv/srv0start.cc                    | 29 ++++++++++---
 10 files changed, 130 insertions(+), 60 deletions(-)

diff --git a/mysql-test/suite/encryption/r/innodb_encryption.result b/mysql-test/suite/encryption/r/innodb_encryption.result
index 26c77499d25..ce494098d44 100644
--- a/mysql-test/suite/encryption/r/innodb_encryption.result
+++ b/mysql-test/suite/encryption/r/innodb_encryption.result
@@ -1,3 +1,5 @@
+call mtr.add_suppression("InnoDB: New log files created");
+call mtr.add_suppression("InnoDB: Creating foreign key constraint system tables.");
 SET @start_global_value = @@global.innodb_encryption_threads;
 SHOW VARIABLES LIKE 'innodb_encrypt%';
 Variable_name	Value
diff --git a/mysql-test/suite/encryption/t/innodb_encryption.test b/mysql-test/suite/encryption/t/innodb_encryption.test
index 50aca2a7260..6e9f80aac0c 100644
--- a/mysql-test/suite/encryption/t/innodb_encryption.test
+++ b/mysql-test/suite/encryption/t/innodb_encryption.test
@@ -3,10 +3,14 @@
 #
 -- source include/have_innodb.inc
 -- source include/have_example_key_management_plugin.inc
+-- source include/innodb_undo_tablespaces.inc
 
 # embedded does not support restart
 -- source include/not_embedded.inc
 
+call mtr.add_suppression("InnoDB: New log files created");
+call mtr.add_suppression("InnoDB: Creating foreign key constraint system tables.");
+
 SET @start_global_value = @@global.innodb_encryption_threads;
 
 SHOW VARIABLES LIKE 'innodb_encrypt%';
diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc
index b5b762c2cd9..0d53160ec28 100644
--- a/storage/innobase/fil/fil0crypt.cc
+++ b/storage/innobase/fil/fil0crypt.cc
@@ -191,7 +191,12 @@ fil_crypt_get_latest_key_version(
 				crypt_data->min_key_version,
 				key_version,
 				srv_fil_crypt_rotate_key_age)) {
-			os_event_set(fil_crypt_threads_event);
+			/* Below event seen as NULL-pointer at startup
+			when new database was created and we create a
+			checkpoint. Only seen when debugging. */
+			if (fil_crypt_threads_inited) {
+				os_event_set(fil_crypt_threads_event);
+			}
 		}
 	}
 
diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
index 6abc7f0e1c2..559ba26542f 100644
--- a/storage/innobase/fil/fil0fil.cc
+++ b/storage/innobase/fil/fil0fil.cc
@@ -2322,8 +2322,10 @@ the first page of a first data file at database startup.
 					data files
 @param[out]	flushed_lsn		flushed lsn value
 @param[out]	crypt_data		encryption crypt data
- at retval NULL on success, or if innodb_force_recovery is set
- at return pointer to an error message string */
+ at param[in]	check_first_page	true if first page contents
+					should be checked
+ at return NULL on success, or if innodb_force_recovery is set
+ at retval pointer to an error message string */
 UNIV_INTERN
 const char*
 fil_read_first_page(
@@ -2336,7 +2338,8 @@ fil_read_first_page(
 	ulint*		max_arch_log_no,
 #endif /* UNIV_LOG_ARCHIVE */
 	lsn_t*		flushed_lsn,
-	fil_space_crypt_t**   crypt_data)
+	fil_space_crypt_t**   crypt_data,
+	bool		check_first_page)
 {
 	byte*		buf;
 	byte*		page;
@@ -2358,27 +2361,31 @@ fil_read_first_page(
 	*flags and *space_id as they were read from the first file and
 	do not validate the first page. */
 	if (!one_read_already) {
-		*space_id = fsp_header_get_space_id(page);
-		*flags = fsp_header_get_flags(page);
-
-		if (flushed_lsn) {
-			*flushed_lsn = mach_read_from_8(page +
-				       FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION);
-		}
+		/* Undo tablespace does not contain correct FSP_HEADER,
+		and actually we really need to read only crypt_data. */
+		if (check_first_page) {
+			*space_id = fsp_header_get_space_id(page);
+			*flags = fsp_header_get_flags(page);
+
+			if (flushed_lsn) {
+				*flushed_lsn = mach_read_from_8(page +
+					FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION);
+			}
 
-		if (!fsp_flags_is_valid(*flags, *space_id)) {
-			ulint cflags = fsp_flags_convert_from_101(*flags);
-			if (cflags == ULINT_UNDEFINED) {
-				ib_logf(IB_LOG_LEVEL_ERROR,
-					"Invalid flags 0x%x in tablespace %u",
-					unsigned(*flags), unsigned(*space_id));
-				return "invalid tablespace flags";
-			} else {
-				*flags = cflags;
+			if (!fsp_flags_is_valid(*flags, *space_id)) {
+				ulint cflags = fsp_flags_convert_from_101(*flags);
+				if (cflags == ULINT_UNDEFINED) {
+					ib_logf(IB_LOG_LEVEL_ERROR,
+						"Invalid flags 0x%x in tablespace %u",
+						unsigned(*flags), unsigned(*space_id));
+					return "invalid tablespace flags";
+				} else {
+					*flags = cflags;
+				}
 			}
-		}
 
-		check_msg = fil_check_first_page(page, *space_id, *flags);
+			check_msg = fil_check_first_page(page, *space_id, *flags);
+		}
 
 		/* Possible encryption crypt data is also stored only to first page
 		of the first datafile. */
diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h
index e481ea3a8aa..38250949380 100644
--- a/storage/innobase/include/fil0fil.h
+++ b/storage/innobase/include/fil0fil.h
@@ -802,6 +802,8 @@ the first page of a first data file at database startup.
 					data files
 @param[out]	flushed_lsn		flushed lsn value
 @param[out]	crypt_data		encryption crypt data
+ at param[in]	check_first_page	true if first page contents
+					should be checked
 @retval NULL on success, or if innodb_force_recovery is set
 @return pointer to an error message string */
 UNIV_INTERN
@@ -816,7 +818,8 @@ fil_read_first_page(
 	ulint*		max_arch_log_no,
 #endif /* UNIV_LOG_ARCHIVE */
 	lsn_t*		flushed_lsn,
-	fil_space_crypt_t**   crypt_data)
+	fil_space_crypt_t**   crypt_data,
+	bool		check_first_page=true)
 	MY_ATTRIBUTE((warn_unused_result));
 #endif /* !UNIV_HOTBACKUP */
 
diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc
index 0880f93c7fa..703ab0c9dca 100644
--- a/storage/innobase/srv/srv0start.cc
+++ b/storage/innobase/srv/srv0start.cc
@@ -850,7 +850,7 @@ open_or_create_data_files(
 	ibool		one_created	= FALSE;
 	os_offset_t	size;
 	ulint		flags;
-	ulint		space;
+	ulint		space=0;
 	ulint		rounded_size_pages;
 	char		name[10000];
 	fil_space_crypt_t*    crypt_data=NULL;
@@ -1332,11 +1332,30 @@ srv_undo_tablespace_open(
 		size = os_file_get_size(fh);
 		ut_a(size != (os_offset_t) -1);
 
+		/* Load the tablespace into InnoDB's internal
+		data structures. */
+
+		const char* check_msg;
+		fil_space_crypt_t* crypt_data = NULL;
+
+		/* Set the compressed page size to 0 (non-compressed) */
+		flags = FSP_FLAGS_PAGE_SSIZE();
+
+		/* Read first page to find out does the crypt_info
+		exists on undo tablespace. */
+		check_msg = fil_read_first_page(
+				fh, FALSE, &flags, &space,
+				NULL, &crypt_data, false);
+
 		ret = os_file_close(fh);
 		ut_a(ret);
 
-		/* Load the tablespace into InnoDB's internal
-		data structures. */
+		if (check_msg) {
+			ib_logf(IB_LOG_LEVEL_ERROR,
+				"%s in data file %s",
+				check_msg, name);
+			return (err);
+		}
 
 		/* We set the biggest space id to the undo tablespace
 		because InnoDB hasn't opened any other tablespace apart
@@ -1344,10 +1363,8 @@ srv_undo_tablespace_open(
 
 		fil_set_max_space_id_if_bigger(space);
 
-		/* Set the compressed page size to 0 (non-compressed) */
-		flags = FSP_FLAGS_PAGE_SSIZE();
 		fil_space_create(name, space, flags, FIL_TABLESPACE,
-				NULL /* no encryption */,
+				crypt_data,
 				true /* create */);
 
 		ut_a(fil_validate());
diff --git a/storage/xtradb/fil/fil0crypt.cc b/storage/xtradb/fil/fil0crypt.cc
index b5b762c2cd9..0d53160ec28 100644
--- a/storage/xtradb/fil/fil0crypt.cc
+++ b/storage/xtradb/fil/fil0crypt.cc
@@ -191,7 +191,12 @@ fil_crypt_get_latest_key_version(
 				crypt_data->min_key_version,
 				key_version,
 				srv_fil_crypt_rotate_key_age)) {
-			os_event_set(fil_crypt_threads_event);
+			/* Below event seen as NULL-pointer at startup
+			when new database was created and we create a
+			checkpoint. Only seen when debugging. */
+			if (fil_crypt_threads_inited) {
+				os_event_set(fil_crypt_threads_event);
+			}
 		}
 	}
 
diff --git a/storage/xtradb/fil/fil0fil.cc b/storage/xtradb/fil/fil0fil.cc
index dbf6501b183..2c3d26d6302 100644
--- a/storage/xtradb/fil/fil0fil.cc
+++ b/storage/xtradb/fil/fil0fil.cc
@@ -2372,8 +2372,10 @@ the first page of a first data file at database startup.
 @param[out]	space_id		tablepspace ID
 @param[out]	flushed_lsn		flushed lsn value
 @param[out]	crypt_data		encryption crypt data
- at retval NULL on success, or if innodb_force_recovery is set
- at return pointer to an error message string */
+ at param[in]	check_first_page	true if first page contents
+					should be checked
+ at return NULL on success, or if innodb_force_recovery is set
+ at retval pointer to an error message string */
 UNIV_INTERN
 const char*
 fil_read_first_page(
@@ -2382,7 +2384,8 @@ fil_read_first_page(
 	ulint*		flags,
 	ulint*		space_id,
 	lsn_t*		flushed_lsn,
-	fil_space_crypt_t**   crypt_data)
+	fil_space_crypt_t**   crypt_data,
+	bool		check_first_page)
 {
 	byte*		buf;
 	byte*		page;
@@ -2418,28 +2421,32 @@ fil_read_first_page(
 	*flags and *space_id as they were read from the first file and
 	do not validate the first page. */
 	if (!one_read_already) {
-		*space_id = fsp_header_get_space_id(page);
-		*flags = fsp_header_get_flags(page);
-
-		if (flushed_lsn) {
-			*flushed_lsn = mach_read_from_8(page +
+		/* Undo tablespace does not contain correct FSP_HEADER,
+		and actually we really need to read only crypt_data. */
+		if (check_first_page) {
+			*space_id = fsp_header_get_space_id(page);
+			*flags = fsp_header_get_flags(page);
+
+			if (flushed_lsn) {
+				*flushed_lsn = mach_read_from_8(page +
 				       FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION);
-		}
+			}
 
-		if (!fsp_flags_is_valid(*flags, *space_id)) {
-			ulint cflags = fsp_flags_convert_from_101(*flags);
-			if (cflags == ULINT_UNDEFINED) {
-				ib_logf(IB_LOG_LEVEL_ERROR,
-					"Invalid flags 0x%x in tablespace %u",
-					unsigned(*flags), unsigned(*space_id));
-				return "invalid tablespace flags";
-			} else {
-				*flags = cflags;
+			if (!fsp_flags_is_valid(*flags, *space_id)) {
+				ulint cflags = fsp_flags_convert_from_101(*flags);
+				if (cflags == ULINT_UNDEFINED) {
+					ib_logf(IB_LOG_LEVEL_ERROR,
+						"Invalid flags 0x%x in tablespace %u",
+						unsigned(*flags), unsigned(*space_id));
+					return "invalid tablespace flags";
+				} else {
+					*flags = cflags;
+				}
 			}
-		}
 
-		if (!(IS_XTRABACKUP() && srv_backup_mode)) {
-			check_msg = fil_check_first_page(page, *space_id, *flags);
+			if (!(IS_XTRABACKUP() && srv_backup_mode)) {
+				check_msg = fil_check_first_page(page, *space_id, *flags);
+			}
 		}
 
 		/* Possible encryption crypt data is also stored only to first page
diff --git a/storage/xtradb/include/fil0fil.h b/storage/xtradb/include/fil0fil.h
index 6eab5db6883..a33cec65ed5 100644
--- a/storage/xtradb/include/fil0fil.h
+++ b/storage/xtradb/include/fil0fil.h
@@ -804,8 +804,10 @@ the first page of a first data file at database startup.
 @param[out]	space_id		tablepspace ID
 @param[out]	flushed_lsn		flushed lsn value
 @param[out]	crypt_data		encryption crypt data
- at retval NULL on success, or if innodb_force_recovery is set
- at return pointer to an error message string */
+ at param[in]	check_first_page	true if first page contents
+					should be checked
+ at return NULL on success, or if innodb_force_recovery is set
+ at retval pointer to an error message string */
 UNIV_INTERN
 const char*
 fil_read_first_page(
@@ -814,7 +816,8 @@ fil_read_first_page(
 	ulint*		flags,
 	ulint*		space_id,
 	lsn_t*		flushed_lsn,
-	fil_space_crypt_t**   crypt_data)
+	fil_space_crypt_t**   crypt_data,
+	bool		check_first_page=true)
 	MY_ATTRIBUTE((warn_unused_result));
 
 #endif /* !UNIV_HOTBACKUP */
diff --git a/storage/xtradb/srv/srv0start.cc b/storage/xtradb/srv/srv0start.cc
index aab0bc9282b..7bcb0b1b1ce 100644
--- a/storage/xtradb/srv/srv0start.cc
+++ b/storage/xtradb/srv/srv0start.cc
@@ -891,7 +891,7 @@ open_or_create_data_files(
 	bool		one_created	= false;
 	os_offset_t	size;
 	ulint		flags;
-	ulint		space;
+	ulint		space = 0;
 	ulint		rounded_size_pages;
 	char		name[10000];
 	fil_space_crypt_t*    crypt_data=NULL;
@@ -1369,11 +1369,30 @@ srv_undo_tablespace_open(
 		size = os_file_get_size(fh);
 		ut_a(size != (os_offset_t) -1);
 
+		/* Load the tablespace into InnoDB's internal
+		data structures. */
+
+		const char* check_msg;
+		fil_space_crypt_t* crypt_data = NULL;
+
+		/* Set the compressed page size to 0 (non-compressed) */
+		flags = FSP_FLAGS_PAGE_SSIZE();
+
+		/* Read first page to find out does the crypt_info
+		exists on undo tablespace. */
+		check_msg = fil_read_first_page(
+				fh, FALSE, &flags, &space,
+				NULL, &crypt_data, false);
+
 		ret = os_file_close(fh);
 		ut_a(ret);
 
-		/* Load the tablespace into InnoDB's internal
-		data structures. */
+		if (check_msg) {
+			ib_logf(IB_LOG_LEVEL_ERROR,
+				"%s in data file %s",
+				check_msg, name);
+			return (err);
+		}
 
 		/* We set the biggest space id to the undo tablespace
 		because InnoDB hasn't opened any other tablespace apart
@@ -1381,10 +1400,8 @@ srv_undo_tablespace_open(
 
 		fil_set_max_space_id_if_bigger(space);
 
-		/* Set the compressed page size to 0 (non-compressed) */
-		flags = FSP_FLAGS_PAGE_SSIZE();
 		fil_space_create(name, space, flags, FIL_TABLESPACE,
-				NULL /* no encryption */,
+				crypt_data,
 				true /* create */);
 
 		ut_a(fil_validate());


More information about the commits mailing list