MDEV-18733 MariaDB slow start after crash recovery

If InnoDB crash recovery was needed, the InnoDB function srv_start()
would invoke extra validation, reading something from every InnoDB
data file. This should be unnecessary now that MDEV-14717 made
RENAME operations crash-safe inside InnoDB (which can be
disabled in MariaDB 10.2 by setting innodb_safe_truncate=OFF).

dict_check_sys_tables(): Skip tables that would be dropped by
row_mysql_drop_garbage_tables(). Perform extra validation only
if innodb_safe_truncate=OFF, innodb_force_recovery=0 and
crash recovery was needed.

dict_load_table_one(): Validate the root page of the table.
In this way, we can deny access to corrupted or mismatching tables
not only after crash recovery, but also after a clean shutdown.

4 files changed, 50 insertions, 35 deletions

diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc
index ca0a52a3e90..9a3ee7de275 100644
--- a/storage/innobase/buf/buf0buf.cc
+++ b/storage/innobase/buf/buf0buf.cc
@@ -4622,7 +4622,9 @@ evict_from_pool:
 				buf_pool_mutex_exit(buf_pool);
 				rw_lock_x_unlock(&fix_block->lock);
 
-				*err = DB_PAGE_CORRUPTED;
+				if (err) {
+					*err = DB_PAGE_CORRUPTED;
+				}
 				return NULL;
 			}
 		}
diff --git a/storage/innobase/dict/dict0load.cc b/storage/innobase/dict/dict0load.cc
index 8b0a0fa1030..304a536505c 100644
--- a/storage/innobase/dict/dict0load.cc
+++ b/storage/innobase/dict/dict0load.cc
@@ -1360,14 +1360,9 @@ dict_sys_tables_rec_read(
 /** Load and check each non-predefined tablespace mentioned in SYS_TABLES.
 Search SYS_TABLES and check each tablespace mentioned that has not
 already been added to the fil_system.  If it is valid, add it to the
-file_system list.  Perform extra validation on the table if recovery from
-the REDO log occurred.
-@param[in]	validate	Whether to do validation on the table.
+file_system list.
 @return the highest space ID found. */
-UNIV_INLINE
-ulint
-dict_check_sys_tables(
-	bool		validate)
+static ulint dict_check_sys_tables()
 {
 	ulint		max_space_id = 0;
 	btr_pcur_t	pcur;
@@ -1390,6 +1385,10 @@ dict_check_sys_tables(
 	sys_datafiles = dict_table_get_low("SYS_DATAFILES");
 	ut_a(sys_datafiles != NULL);
 
+	const bool validate = recv_needed_recovery
+		&& !srv_safe_truncate
+		&& !srv_force_recovery;
+
 	for (rec = dict_startscan_system(&pcur, &mtr, SYS_TABLES);
 	     rec != NULL;
 	     rec = dict_getnext_system(&pcur, &mtr)) {
@@ -1420,15 +1419,23 @@ dict_check_sys_tables(
 					      &table_id, &space_id,
 					      &n_cols, &flags, &flags2)
 		    || space_id == TRX_SYS_SPACE) {
+next:
 			ut_free(table_name.m_name);
 			continue;
 		}
 
+		if (srv_safe_truncate
+		    && strstr(table_name.m_name, "/" TEMP_FILE_PREFIX "-")) {
+			/* This table will be dropped by
+			row_mysql_drop_garbage_tables().
+			We do not care if the file exists. */
+			goto next;
+		}
+
 		if (flags2 & DICT_TF2_DISCARDED) {
 			ib::info() << "Ignoring tablespace for " << table_name
 				<< " because the DISCARD flag is set .";
-			ut_free(table_name.m_name);
-			continue;
+			goto next;
 		}
 
 		/* For tables or partitions using .ibd files, the flag
@@ -1503,11 +1510,8 @@ space_id information in the data dictionary to what we find in the
 tablespace file. In addition, more validation will be done if recovery
 was needed and force_recovery is not set.
 
-We also scan the biggest space id, and store it to fil_system.
-@param[in]	validate	true if recovery was needed */
-void
-dict_check_tablespaces_and_store_max_id(
-	bool	validate)
+We also scan the biggest space id, and store it to fil_system. */
+void dict_check_tablespaces_and_store_max_id()
 {
 	mtr_t	mtr;
 
@@ -1528,7 +1532,7 @@ dict_check_tablespaces_and_store_max_id(
 	/* Open all tablespaces referenced in SYS_TABLES.
 	This will update SYS_TABLESPACES and SYS_DATAFILES if it
 	finds any file-per-table tablespaces not already there. */
-	max_space_id = dict_check_sys_tables(validate);
+	max_space_id = dict_check_sys_tables();
 	fil_set_max_space_id_if_bigger(max_space_id);
 
 	mutex_exit(&dict_sys->mutex);
@@ -3063,8 +3067,31 @@ err_exit:
 
 			if (table->space
 			    && !fil_space_get_size(table->space)) {
+corrupted:
 				table->corrupted = true;
 				table->file_unreadable = true;
+			} else {
+				const page_id_t page_id(
+					table->space,
+					dict_table_get_first_index(table)
+					->page);
+				mtr.start();
+				buf_block_t* block = buf_page_get(
+					page_id,
+					dict_table_page_size(table),
+					RW_S_LATCH, &mtr);
+				const bool corrupted = !block
+					|| page_get_space_id(block->frame)
+					!= page_id.space()
+					|| page_get_page_no(block->frame)
+					!= page_id.page_no()
+					|| mach_read_from_2(FIL_PAGE_TYPE
+							    + block->frame)
+					!= FIL_PAGE_INDEX;
+				mtr.commit();
+				if (corrupted) {
+					goto corrupted;
+				}
 			}
 		}
 	} else {
diff --git a/storage/innobase/include/dict0load.h b/storage/innobase/include/dict0load.h
index b35add02d9d..b58f0e72038 100644
--- a/storage/innobase/include/dict0load.h
+++ b/storage/innobase/include/dict0load.h
@@ -1,7 +1,7 @@
 /*****************************************************************************
 
 Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
-Copyright (c) 2017, MariaDB Corporation.
+Copyright (c) 2017, 2019, MariaDB Corporation.
 
 This program is free software; you can redistribute it and/or modify it under
 the terms of the GNU General Public License as published by the Free Software
@@ -76,11 +76,8 @@ space_id information in the data dictionary to what we find in the
 tablespace file. In addition, more validation will be done if recovery
 was needed and force_recovery is not set.
 
-We also scan the biggest space id, and store it to fil_system.
-@param[in]	validate	true if recovery was needed */
-void
-dict_check_tablespaces_and_store_max_id(
-	bool		validate);
+We also scan the biggest space id, and store it to fil_system. */
+void dict_check_tablespaces_and_store_max_id();
 
 /********************************************************************//**
 Finds the first table name in the given database.
diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc
index b12e6a6fb18..5973e635b0f 100644
--- a/storage/innobase/srv/srv0start.cc
+++ b/storage/innobase/srv/srv0start.cc
@@ -2486,19 +2486,8 @@ files_checked:
 			every table in the InnoDB data dictionary that has
 			an .ibd file.
 
-			We also determine the maximum tablespace id used.
-
-			The 'validate' flag indicates that when a tablespace
-			is opened, we also read the header page and validate
-			the contents to the data dictionary. This is time
-			consuming, especially for databases with lots of ibd
-			files.  So only do it after a crash and not forcing
-			recovery.  Open rw transactions at this point is not
-			a good reason to validate. */
-			bool validate = recv_needed_recovery
-				&& srv_force_recovery == 0;
-
-			dict_check_tablespaces_and_store_max_id(validate);
+			We also determine the maximum tablespace id used. */
+			dict_check_tablespaces_and_store_max_id();
 		}
 
 		/* Fix-up truncate of table if server crashed while truncate