Bug#32912868: dict_table_x_lock_indexes() can cause deadlock with concurrent purge operations

Confirmed carefully also other possibility with index latch order and implicit latch order.
This bug has the both 2 aspects.

[A] Indexes latch order in same table is [secondary --> primary]
 We should be along with description in sync0type.h.
 The current dict_table_x_lock_indexes() violates the order.
 Better to be fixed.

[B] Even if dict_table_x_lock_indexes() is atomic operation,
 still might cause deadlock with
 "[secondary index page]-->[primary index latch]-->[primary index page]"
 operation. (if dict_table_x_lock_indexes() caller will access to [secondary index page].)
 That seems to be allowed order, and included to purge and rollback paths.
 So, dict_table_x_lock_indexes() should exclude other threads' concurrent accesses.
 - User threads: already excluded by mysqld MDL
 - Purge: ## needs to stop during getting latch ##
 - Rollback after recovery: ## needed to be ended already ##

Fixing [B] is enough to fix this bug.
But also [A] is better to be fixed for future fail-tolerance.

(for 5.7 only)
"TRUNCATE TABLE" path also calls dict_table_x_lock_indexes(), and fixed.

Change-Id: I0673af125aef2f4cc14d8b86a3816c71c5becc8a

Author: Yasufumi Kinoshita

mysql-test/suite/innodb/r/innodb-truncate-debug.result

@@ -112,6 +112,6 @@ SET DEBUG_SYNC= 'now SIGNAL finish_scan';
 connection con1
 connection default
 drop table t1;
-Pattern "InnoDB: Record with space id \d+ belongs to table which is being truncated or tablespace which is missing therefore skipping this undo record." found
+Pattern "InnoDB: Record with space id \d+ belongs to table which is being truncated or tablespace which is missing therefore skipping this undo record." not found
 # restart server
 # restart:

mysql-test/suite/innodb/t/innodb-truncate-debug.test

@@ -179,6 +179,9 @@ disconnect con1;
 
 drop table t1;
 
+# Note: Because of fix for bug#32912868,
+#       truncate table doesn't allow concurrent purge operations.
+#       So, this test might not work as intended, or not needed.
 let SEARCH_FILE = $MYSQLTEST_VARDIR/log/mysqld.1.err;
 let SEARCH_PATTERN = InnoDB: Record with space id \d+ belongs to table which is being truncated or tablespace which is missing therefore skipping this undo record.;
 --source include/search_pattern.inc

storage/innobase/dict/dict0dict.cc

@@ -92,6 +92,9 @@ ib_warn_row_too_big(const dict_table_t*	table);
 #include "sync0sync.h"
 #include "trx0undo.h"
 #include "ut0new.h"
+#ifdef UNIV_DEBUG
+#include "trx0purge.h"
+#endif /* !UNIV_DEBUG */
 
 #include <vector>
 #include <algorithm>
@@ -7188,3 +7191,17 @@ uint32_t dict_vcol_base_is_foreign_key(dict_v_col_t *vcol,
 	}
 	return foreign_col_count;
 }
+
+#ifndef UNIV_HOTBACKUP
+#ifdef UNIV_DEBUG
+/** Validate no active background threads to cause purge or rollback
+operations. */
+void
+dict_validate_no_purge_rollback_threads() {
+	/* No concurrent background threads to access to the table */
+	ut_ad(trx_purge_state() == PURGE_STATE_STOP
+	      || trx_purge_state() == PURGE_STATE_DISABLED);
+	ut_ad(!trx_rollback_or_clean_is_active);
+}
+#endif /* UNIV_DEBUG */
+#endif /* !UNIV_HOTBACKUP */

storage/innobase/include/dict0dict.h

@@ -371,6 +371,12 @@ dict_table_analyze_index_lock(
 void
 dict_table_analyze_index_unlock(
 	dict_table_t*	table);
+#ifdef UNIV_DEBUG
+/** Validate no active background threads to cause purge or rollback
+operations. */
+void
+dict_validate_no_purge_rollback_threads();
+#endif /* UNIV_DEBUG */
 #endif /* !UNIV_HOTBACKUP */
 /**********************************************************************//**
 Adds system columns to a table object. */

storage/innobase/include/dict0dict.ic

@@ -1083,16 +1083,20 @@ dict_table_x_lock_indexes(
 	dict_table_t*	table)	/*!< in: table */
 {
 	dict_index_t*   index;
+	dict_index_t*   clustered_index;
 
 	ut_a(table);
 	ut_ad(mutex_own(&dict_sys->mutex));
+	ut_d(dict_validate_no_purge_rollback_threads());
 
 	/* Loop through each index of the table and lock them */
-	for (index = dict_table_get_first_index(table);
+	clustered_index = dict_table_get_first_index(table);
+	for (index = dict_table_get_next_index(clustered_index);
 	     index != NULL;
 	     index = dict_table_get_next_index(index)) {
 		rw_lock_x_lock(dict_index_get_lock(index));
 	}
+	rw_lock_x_lock(dict_index_get_lock(clustered_index));
 }
 /*********************************************************************//**
 Returns true if the particular FTS index in the table is still syncing

storage/innobase/include/trx0roll.h

@@ -207,6 +207,12 @@ trx_roll_savepoints_free(
 	trx_named_savept_t*	savep);	/*!< in: free all savepoints > this one;
 					if this is NULL, free all savepoints
 					of trx */
+/*******************************************************************//**
+Waits for rollback after recovery end. */
+UNIV_INLINE
+void
+trx_rollback_or_clean_wait();
+
 /** Rollback node states */
 enum roll_node_state {
 	ROLL_NODE_NONE = 0,		/*!< Unknown state */

storage/innobase/include/trx0roll.ic

@@ -68,3 +68,12 @@ trx_roll_check_undo_rec_ordering(
 }
 #endif /* UNIV_DEBUG */
 
+/*******************************************************************//**
+Waits for rollback after recovery end. */
+UNIV_INLINE
+void
+trx_rollback_or_clean_wait() {
+	while (trx_rollback_or_clean_is_active) {
+		os_thread_sleep(100000);
+	}
+}

storage/innobase/log/log0log.cc

@@ -2096,10 +2096,10 @@ logs_empty_and_mark_files_at_shutdown(void)
 
 	ib::info() << "Starting shutdown...";
 
-	while (srv_fast_shutdown == 0 && trx_rollback_or_clean_is_active) {
+	if (srv_fast_shutdown == 0) {
 		/* we should wait until rollback after recovery end
 		for slow shutdown */
-		os_thread_sleep(100000);
+		trx_rollback_or_clean_wait();
 	}
 
 	/* Wait until the master thread and all other operations are idle: our

storage/innobase/row/row0quiesce.cc

@@ -42,6 +42,7 @@ Created 2012-02-08 by Sunny Bains.
 #include "ibuf0ibuf.h"
 #include "srv0start.h"
 #include "trx0purge.h"
+#include "trx0roll.h"
 #include "fsp0sysspace.h"
 
 #include <my_aes.h>
@@ -913,10 +914,23 @@ row_quiesce_set_state(
 			    " FTS auxiliary tables will not be flushed.");
 	}
 
+	/* We should wait until rollback after recovery end,
+	to lock the table consistently. */
+	trx_rollback_or_clean_wait();
+
+	if (trx_purge_state() != PURGE_STATE_DISABLED) {
+		/* We should stop purge to lock the table consistently. */
+		trx_purge_stop();
+	}
+
 	row_mysql_lock_data_dictionary(trx);
 
 	dict_table_x_lock_indexes(table);
 
+	if (trx_purge_state() != PURGE_STATE_DISABLED) {
+		trx_purge_run();
+	}
+
 	switch (state) {
 	case QUIESCE_START:
 		break;

storage/innobase/row/row0trunc.cc

@@ -43,6 +43,8 @@ Created 2013-04-12 Sunny Bains
 #include "srv0start.h"
 #include "row0trunc.h"
 #include "os0file.h"
+#include "trx0purge.h"
+#include "trx0roll.h"
 #include <vector>
 
 bool	truncate_t::s_fix_up_active = false;
@@ -1235,6 +1237,10 @@ row_truncate_complete(
 	TruncateLogger*		&logger,
 	dberr_t			err)
 {
+	if (trx_purge_state() != PURGE_STATE_DISABLED) {
+		trx_purge_run();
+	}
+
 	bool	is_file_per_table = dict_table_is_file_per_table(table);
 
 	if (table->memcached_sync_count == DICT_TABLE_IN_DDL) {
@@ -1843,6 +1849,15 @@ row_truncate_table_for_mysql(
 		trx_start_for_ddl(trx, TRX_DICT_OP_TABLE);
 	}
 
+	/* We should wait until rollback after recovery end,
+	to lock the table consistently. */
+	trx_rollback_or_clean_wait();
+
+	if (trx_purge_state() != PURGE_STATE_DISABLED) {
+		/* We should stop purge to lock the table consistently. */
+		trx_purge_stop();
+	}
+
 	/* Step-3: Validate ownership of needed locks (Exclusive lock).
 	Ownership will also ensure there is no active SQL queries, INSERT,
 	SELECT, .....*/