PS-6113: If ANALYZE TABLE runs more than 600 seconds diagnostic query…

… may crash server

https://jira.percona.com/browse/PS-6113

Locking of table statistics limited to the time when statistics related table members are changed. While calculation is done, statistics are not locked, so can be queried.

Index analysis protected by new mutex as it cannot be done concurrently, because it shares the same index object which is modified during analysis.

mysql-test/r/percona_bug_ps6113.result

@@ -0,0 +1,12 @@
+CREATE TABLE t1(a int, index inda(a)) ENGINE=INNODB;
+INSERT INTO t1 VALUES(1);
+SET DEBUG_SYNC='innodb_dict_stats_update_persistent SIGNAL analyze.running WAIT_FOR analyze.finish';
+ANALYZE TABLE t1;
+SET DEBUG_SYNC='now WAIT_FOR analyze.running';
+SELECT ENGINE,SUM(DATA_LENGTH+INDEX_LENGTH),COUNT(ENGINE),SUM(DATA_LENGTH),SUM(INDEX_LENGTH) FROM information_schema.TABLES WHERE TABLE_SCHEMA NOT IN ('information_schema', 'performance_schema', 'mysql') AND ENGINE='InnoDB';
+ENGINE	SUM(DATA_LENGTH+INDEX_LENGTH)	COUNT(ENGINE)	SUM(DATA_LENGTH)	SUM(INDEX_LENGTH)
+InnoDB	49152	2	32768	16384
+SET DEBUG_SYNC='now SIGNAL analyze.finish';
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	OK
+DROP TABLE t1;

mysql-test/t/percona_bug_ps6113.test

@@ -0,0 +1,29 @@
+#
+# Bug #97828
+# PS-6113: If ANALYZE TABLE runs more than 600 seconds diagnostic query may crash server
+#
+
+--source include/have_innodb.inc
+--source include/count_sessions.inc
+
+CREATE TABLE t1(a int, index inda(a)) ENGINE=INNODB;
+INSERT INTO t1 VALUES(1);
+
+SET DEBUG_SYNC='innodb_dict_stats_update_persistent SIGNAL analyze.running WAIT_FOR analyze.finish';
+--send ANALYZE TABLE t1
+
+# ANALYZE TABLE is running. Query for stats.
+
+--connect(con1, localhost, root)
+SET DEBUG_SYNC='now WAIT_FOR analyze.running';
+
+SELECT ENGINE,SUM(DATA_LENGTH+INDEX_LENGTH),COUNT(ENGINE),SUM(DATA_LENGTH),SUM(INDEX_LENGTH) FROM information_schema.TABLES WHERE TABLE_SCHEMA NOT IN ('information_schema', 'performance_schema', 'mysql') AND ENGINE='InnoDB';
+
+# let the ANALYZE TABLE to finish
+SET DEBUG_SYNC='now SIGNAL analyze.finish';
+--disconnect con1
+
+--connection default
+--reap
+DROP TABLE t1;
+

storage/innobase/dict/dict0dict.cc

@@ -914,6 +914,43 @@ dict_table_autoinc_unlock(
 {
 	mutex_exit(table->autoinc_mutex);
 }
+
+/** Create and initialize the analyze index lock for a given table.
+This lock is used to serialize two concurrent analyze index operations
+@param[in]	table_void	table whose analyze_index latch to create */
+static
+void
+dict_table_analyze_index_alloc(
+	void*	table_void)
+{
+	dict_table_t*	table = static_cast<dict_table_t*>(table_void);
+	table->analyze_index_mutex = UT_NEW_NOKEY(AnalyzeIndexMutex());
+	ut_a(table->analyze_index_mutex != NULL);
+	mutex_create(LATCH_ID_ANALYZE_INDEX_MUTEX, table->analyze_index_mutex);
+}
+
+/** Acquire the analyze index lock.
+@param[in]	table table whose analyze_index latch to lock */
+void
+dict_table_analyze_index_lock(
+	dict_table_t*	table)
+{
+	os_once::do_or_wait_for_done(
+		&table->analyze_index_mutex_created,
+		dict_table_analyze_index_alloc, table);
+
+	mutex_enter(table->analyze_index_mutex);
+}
+
+/** Release the analyze index lock.
+@param[in]	table table whose analyze_index latch to unlock */
+void
+dict_table_analyze_index_unlock(
+	dict_table_t*	table)
+{
+	ut_a(table->analyze_index_mutex != NULL);
+	mutex_exit(table->analyze_index_mutex);
+}
 #endif /* !UNIV_HOTBACKUP */
 
 /** Looks for column n in an index.

storage/innobase/dict/dict0mem.cc

@@ -150,6 +150,8 @@ dict_mem_table_create(
 	/* lazy creation of table autoinc latch */
 	dict_table_autoinc_create_lazy(table);
 
+	dict_table_analyze_index_create_lazy(table);
+
 	table->autoinc = 0;
 	table->sess_row_id = 0;
 	table->sess_trx_id = 0;
@@ -206,6 +208,7 @@ dict_mem_table_free(
 		}
 	}
 #ifndef UNIV_HOTBACKUP
+	dict_table_analyze_index_destroy(table);
 	dict_table_autoinc_destroy(table);
 #endif /* UNIV_HOTBACKUP */
 

storage/innobase/dict/dict0stats.cc

@@ -1900,6 +1900,8 @@ dict_stats_analyze_index(
 	ulint		size;
 	DBUG_ENTER("dict_stats_analyze_index");
 
+	ut_ad(!rw_lock_own(index->table->stats_latch, RW_X_LATCH));
+
 	DBUG_PRINT("info", ("index: %s, online status: %d", index->name(),
 			    dict_index_get_online_status(index)));
 
@@ -2198,7 +2200,9 @@ dict_stats_update_persistent(
 
 	DEBUG_PRINTF("%s(table=%s)\n", __func__, table->name);
 
-	dict_table_stats_lock(table, RW_X_LATCH);
+	dict_table_analyze_index_lock(table);
+
+	DEBUG_SYNC_C("innodb_dict_stats_update_persistent");
 
 	/* analyze the clustered index first */
 
@@ -2209,8 +2213,8 @@ dict_stats_update_persistent(
 	    || (index->type | DICT_UNIQUE) != (DICT_CLUSTERED | DICT_UNIQUE)) {
 
 		/* Table definition is corrupt */
-		dict_table_stats_unlock(table, RW_X_LATCH);
 		dict_stats_empty_table(table);
+		dict_table_analyze_index_unlock(table);
 
 		return(DB_CORRUPTION);
 	}
@@ -2221,13 +2225,13 @@ dict_stats_update_persistent(
 
 	ulint	n_unique = dict_index_get_n_unique(index);
 
-	table->stat_n_rows = index->stat_n_diff_key_vals[n_unique - 1];
+	ib_uint64_t stat_n_rows_tmp = index->stat_n_diff_key_vals[n_unique - 1];
 
-	table->stat_clustered_index_size = index->stat_index_size;
+	ulint stat_clustered_index_size_tmp = index->stat_index_size;
 
 	/* analyze other indexes from the table, if any */
 
-	table->stat_sum_of_other_index_sizes = 0;
+	ulint stat_sum_of_other_index_sizes_tmp = 0;
 
 	for (index = dict_table_get_next_index(index);
 	     index != NULL;
@@ -2249,10 +2253,18 @@ dict_stats_update_persistent(
 			dict_stats_analyze_index(index);
 		}
 
-		table->stat_sum_of_other_index_sizes
+		stat_sum_of_other_index_sizes_tmp
 			+= index->stat_index_size;
 	}
 
+	dict_table_stats_lock(table, RW_X_LATCH);
+
+	table->stat_n_rows = stat_n_rows_tmp;
+
+	table->stat_clustered_index_size = stat_clustered_index_size_tmp;
+
+	table->stat_sum_of_other_index_sizes = stat_sum_of_other_index_sizes_tmp;
+
 	table->stats_last_recalc = ut_time_monotonic();
 
 	table->stat_modified_counter = 0;
@@ -2263,6 +2275,8 @@ dict_stats_update_persistent(
 
 	dict_table_stats_unlock(table, RW_X_LATCH);
 
+	dict_table_analyze_index_unlock(table);
+
 	return(DB_SUCCESS);
 }
 
@@ -3072,10 +3086,13 @@ dict_stats_update_for_index(
 	if (dict_stats_is_persistent_enabled(index->table)) {
 
 		if (dict_stats_persistent_storage_check(false)) {
-			dict_table_stats_lock(index->table, RW_X_LATCH);
+			dict_table_analyze_index_lock(index->table);
 			dict_stats_analyze_index(index);
-			index->table->stat_sum_of_other_index_sizes += index->stat_index_size;
+			ulint stat_sum_of_other_index_sizes_tmp = index->stat_index_size;
+			dict_table_stats_lock(index->table, RW_X_LATCH);
+			index->table->stat_sum_of_other_index_sizes += stat_sum_of_other_index_sizes_tmp;
 			dict_table_stats_unlock(index->table, RW_X_LATCH);
+			dict_table_analyze_index_unlock(index->table);
 			dict_stats_save(index->table, &index->id);
 			DBUG_VOID_RETURN;
 		}

storage/innobase/handler/ha_innodb.cc

@@ -507,6 +507,7 @@ static PSI_mutex_info all_innodb_mutexes[] = {
 	PSI_KEY(row_drop_list_mutex),
 	PSI_KEY(master_key_id_mutex),
 	PSI_KEY(scrub_stat_mutex),
+	PSI_KEY(analyze_index_mutex),
 };
 # endif /* UNIV_PFS_MUTEX */
 

storage/innobase/include/dict0dict.h

@@ -360,6 +360,17 @@ dict_table_autoinc_unlock(
 /*======================*/
 	dict_table_t*	table);	/*!< in/out: table */
 
+/** Acquire the analyze index lock.
+@param[in]	table table whose analyze_index latch to lock */
+void
+dict_table_analyze_index_lock(
+	dict_table_t*	table);
+
+/** Release the analyze index lock.
+@param[in]	table table whose analyze_index latch to unlock */
+void
+dict_table_analyze_index_unlock(
+	dict_table_t*	table);
 #endif /* !UNIV_HOTBACKUP */
 /**********************************************************************//**
 Adds system columns to a table object. */

storage/innobase/include/dict0mem.h

@@ -1331,6 +1331,7 @@ the table, DML from memcached will be blocked. */
 #define DICT_TABLE_IN_DDL -1
 
 typedef ib_bpmutex_t AutoIncMutex;
+typedef ib_mutex_t AnalyzeIndexMutex;
 
 /** Data structure for a database table.  Most fields will be
 initialized to 0, NULL or FALSE in dict_mem_table_create(). */
@@ -1702,6 +1703,12 @@ struct dict_table_t {
 
 	/* @} */
 
+	/** Creation state of analyze_index member */
+	volatile os_once::state_t	analyze_index_mutex_created;
+
+	/** Mutex protecting the index during analyze. */
+	AnalyzeIndexMutex*			analyze_index_mutex;
+
 	/** Count of how many handles are opened to this table from memcached.
 	DDL on the table is NOT allowed until this count goes to zero. If
 	it is -1, then there's DDL on the table, DML from memcached will be
@@ -1796,6 +1803,31 @@ struct dict_foreign_add_to_referenced_table {
 	}
 };
 
+/** Request for lazy creation of the analyze index mutex of a given table.
+@param[in,out]	table	table whose mutex is to be created. */
+inline
+void
+dict_table_analyze_index_create_lazy(
+	dict_table_t*	table)
+{
+	table->analyze_index_mutex = NULL;
+	table->analyze_index_mutex_created = os_once::NEVER_DONE;
+}
+
+/** Destroy the analyze index mutex of the given table.
+@param[in,out]	table	table whose mutex to destroy */
+inline
+void
+dict_table_analyze_index_destroy(
+	dict_table_t*	table)
+{
+	if (table->analyze_index_mutex_created == os_once::DONE
+	    && table->analyze_index_mutex != NULL) {
+		mutex_free(table->analyze_index_mutex);
+		UT_DELETE(table->analyze_index_mutex);
+	}
+}
+
 /** Destroy the autoinc latch of the given table.
 This function is only called from either single threaded environment
 or from a thread that has not shared the table object with other threads.

storage/innobase/include/sync0sync.h

@@ -123,6 +123,7 @@ extern mysql_pfs_key_t	thread_mutex_key;
 extern mysql_pfs_key_t  zip_pad_mutex_key;
 extern mysql_pfs_key_t  row_drop_list_mutex_key;
 extern mysql_pfs_key_t	master_key_id_mutex_key;
+extern mysql_pfs_key_t	analyze_index_mutex_key;
 #endif /* UNIV_PFS_MUTEX */
 
 #ifdef UNIV_PFS_RWLOCK

storage/innobase/include/sync0types.h

@@ -273,6 +273,7 @@ enum latch_level_t {
 	SYNC_TREE_NODE_FROM_HASH,
 	SYNC_TREE_NODE_NEW,
 	SYNC_INDEX_TREE,
+	SYNC_ANALYZE_INDEX,
 
 	SYNC_IBUF_PESS_INSERT_MUTEX,
 	SYNC_IBUF_HEADER,
@@ -403,6 +404,7 @@ enum latch_id_t {
 	LATCH_ID_FIL_CRYPT_DATA_MUTEX,
 	LATCH_ID_FIL_CRYPT_THREADS_MUTEX,
 	LATCH_ID_FIL_CRYPT_START_ROTATE_MUTEX,
+	LATCH_ID_ANALYZE_INDEX_MUTEX,
 	LATCH_ID_TEST_MUTEX,
 	LATCH_ID_MAX = LATCH_ID_TEST_MUTEX
 };

storage/innobase/sync/sync0debug.cc

@@ -520,6 +520,7 @@ LatchDebug::LatchDebug()
 	LEVEL_MAP_INSERT(SYNC_TREE_NODE);
 	LEVEL_MAP_INSERT(SYNC_TREE_NODE_FROM_HASH);
 	LEVEL_MAP_INSERT(SYNC_TREE_NODE_NEW);
+	LEVEL_MAP_INSERT(SYNC_ANALYZE_INDEX);
 	LEVEL_MAP_INSERT(SYNC_INDEX_TREE);
 	LEVEL_MAP_INSERT(SYNC_IBUF_PESS_INSERT_MUTEX);
 	LEVEL_MAP_INSERT(SYNC_IBUF_HEADER);
@@ -946,6 +947,11 @@ LatchDebug::check_order(
 		basic_check(latches, level, SYNC_TREE_NODE - 1);
 		break;
 
+	case SYNC_ANALYZE_INDEX:
+
+		basic_check(latches, level, SYNC_ANALYZE_INDEX - 1);
+		break;
+
 	case SYNC_IBUF_TREE_NODE:
 
 		ut_a(find(latches, SYNC_IBUF_INDEX_TREE) != 0
@@ -1605,6 +1611,8 @@ sync_latch_meta_init()
 	LATCH_ADD_MUTEX(FIL_CRYPT_START_ROTATE_MUTEX, SYNC_NO_ORDER_CHECK,
 	    		PFS_NOT_INSTRUMENTED);
 
+	LATCH_ADD_MUTEX(ANALYZE_INDEX_MUTEX, SYNC_ANALYZE_INDEX,
+			analyze_index_mutex_key);
 
 	latch_id_t	id = LATCH_ID_NONE;
 

storage/innobase/sync/sync0sync.cc

@@ -111,6 +111,7 @@ mysql_pfs_key_t	thread_mutex_key;
 mysql_pfs_key_t zip_pad_mutex_key;
 mysql_pfs_key_t row_drop_list_mutex_key;
 mysql_pfs_key_t	master_key_id_mutex_key;
+mysql_pfs_key_t	analyze_index_mutex_key;
 
 #endif /* UNIV_PFS_MUTEX */