MDEV-13051 MySQL#86607 InnoDB crash after failed ADD INDEX and table_definition_cache eviction

There are two bugs related to failed ADD INDEX and
the InnoDB table cache eviction.

dict_table_close(): Try dropping failed ADD INDEX when releasing
the last table handle, not when releasing the last-but-one.

dict_table_remove_from_cache_low(): Do not invoke
row_merge_drop_indexes() after freeing all index metadata.
Instead, directly invoke row_merge_drop_indexes_dict() to
remove the metadata from the persistent data dictionary
and to free the index pages.

mysql-test/suite/innodb/r/table_definition_cache_debug.result

+SET @save_tdc= @@GLOBAL.table_definition_cache;
+SET @save_toc= @@GLOBAL.table_open_cache;
+SET GLOBAL table_definition_cache= 400;
+SET GLOBAL table_open_cache= 1024;
+CREATE TABLE to_be_evicted(a INT PRIMARY KEY, b INT NOT NULL) ENGINE=InnoDB;
+INSERT INTO to_be_evicted VALUES(1,2),(2,1);
+SET DEBUG_SYNC = 'row_log_apply_before SIGNAL scanned WAIT_FOR got_duplicate';
+ALTER TABLE to_be_evicted ADD UNIQUE INDEX(b);
+SET DEBUG_SYNC = 'now WAIT_FOR scanned';
+BEGIN;
+INSERT INTO to_be_evicted VALUES(3, 2);
+SET DEBUG_SYNC = 'now SIGNAL got_duplicate';
+ERROR 23000: Duplicate entry '2' for key 'b'
+COMMIT;
+SET DEBUG_SYNC = RESET;
+FLUSH TABLES;

mysql-test/suite/innodb/t/table_definition_cache_debug.test

+--source include/have_innodb.inc
+--source include/have_debug.inc
+--source include/have_debug_sync.inc
+
+SET @save_tdc= @@GLOBAL.table_definition_cache;
+SET @save_toc= @@GLOBAL.table_open_cache;
+
+# InnoDB plugin essentially ignores table_definition_cache size
+# and hard-wires it to 400, which also is the minimum allowed value.
+SET GLOBAL table_definition_cache= 400;
+SET GLOBAL table_open_cache= 1024;
+
+CREATE TABLE to_be_evicted(a INT PRIMARY KEY, b INT NOT NULL) ENGINE=InnoDB;
+INSERT INTO to_be_evicted VALUES(1,2),(2,1);
+
+connect(ddl,localhost,root,,);
+SET DEBUG_SYNC = 'row_log_apply_before SIGNAL scanned WAIT_FOR got_duplicate';
+--send
+ALTER TABLE to_be_evicted ADD UNIQUE INDEX(b);
+
+connection default;
+SET DEBUG_SYNC = 'now WAIT_FOR scanned';
+
+# During the ADD UNIQUE INDEX, start a transaction that inserts a duplicate
+# and then hogs the table lock, so that the unique index cannot be dropped.
+BEGIN;
+INSERT INTO to_be_evicted VALUES(3, 2);
+SET DEBUG_SYNC = 'now SIGNAL got_duplicate';
+
+connection ddl;
+--error ER_DUP_ENTRY
+reap;
+
+disconnect ddl;
+connection default;
+# Release the table lock.
+COMMIT;
+SET DEBUG_SYNC = RESET;
+
+# Allow cache eviction.
+FLUSH TABLES;
+--disable_query_log
+
+# Pollute the cache with many tables, so that our table will be evicted.
+let $N=1000;
+let $loop=$N;
+while ($loop)
+{
+  eval CREATE TABLE t_$loop(id INT)ENGINE=InnoDB;
+  dec $loop;
+}
+
+# Hopefully let InnoDB evict the tables.
+sleep 10;
+
+let $loop=$N;
+while ($loop)
+{
+  eval DROP TABLE t_$loop;
+  dec $loop;
+}
+
+SET GLOBAL table_definition_cache= @save_tdc;
+SET GLOBAL table_open_cache= @save_toc;
+
+DROP TABLE to_be_evicted;

storage/innobase/dict/dict0dict.cc

@@ -2,7 +2,7 @@
 
 Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved.
 Copyright (c) 2012, Facebook Inc.
-Copyright (c) 2014, 2015, MariaDB Corporation.
+Copyright (c) 2014, 2017, 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
@@ -570,15 +570,14 @@ dict_table_close(
 	ut_ad(mutex_own(&dict_sys->mutex));
 	ut_a(table->n_ref_count > 0);
 
-	--table->n_ref_count;
+	const bool last_handle = !--table->n_ref_count;
 
 	/* Force persistent stats re-read upon next open of the table
 	so that FLUSH TABLE can be used to forcibly fetch stats from disk
 	if they have been manually modified. We reset table->stat_initialized
 	only if table reference count is 0 because we do not want too frequent
 	stats re-reads (e.g. in other cases than FLUSH TABLE). */
-	if (strchr(table->name, '/') != NULL
-	    && table->n_ref_count == 0
+	if (last_handle && strchr(table->name, '/') != NULL
 	    && dict_stats_is_persistent_enabled(table)) {
 
 		dict_stats_deinit(table);
@@ -598,11 +597,8 @@ dict_table_close(
 
 	if (!dict_locked) {
 		table_id_t	table_id	= table->id;
-		ibool		drop_aborted;
-
-		drop_aborted = try_drop
+		const bool	drop_aborted	= last_handle && try_drop
 			&& table->drop_aborted
-			&& table->n_ref_count == 1
 			&& dict_table_get_first_index(table);
 
 		mutex_exit(&dict_sys->mutex);
@@ -2087,8 +2083,9 @@ dict_table_remove_from_cache_low(
 	}
 
 	if (lru_evict && table->drop_aborted) {
-		/* Do as dict_table_try_drop_aborted() does. */
-
+		/* When evicting the table definition,
+		drop the orphan indexes from the data dictionary
+		and free the index pages. */
 		trx_t* trx = trx_allocate_for_background();
 
 		ut_ad(mutex_own(&dict_sys->mutex));
@@ -2099,12 +2096,7 @@ dict_table_remove_from_cache_low(
 		trx->dict_operation_lock_mode = RW_X_LATCH;
 
 		trx_set_dict_operation(trx, TRX_DICT_OP_INDEX);
-
-		/* Silence a debug assertion in row_merge_drop_indexes(). */
-		ut_d(table->n_ref_count++);
-		row_merge_drop_indexes(trx, table, TRUE);
-		ut_d(table->n_ref_count--);
-		ut_ad(table->n_ref_count == 0);
+		row_merge_drop_indexes_dict(trx, table->id);
 		trx_commit_for_mysql(trx);
 		trx->dict_operation_lock_mode = 0;
 		trx_free_for_background(trx);

storage/xtradb/dict/dict0dict.cc

@@ -570,15 +570,14 @@ dict_table_close(
 	ut_ad(mutex_own(&dict_sys->mutex));
 	ut_a(table->n_ref_count > 0);
 
-	--table->n_ref_count;
+	const bool last_handle = !--table->n_ref_count;
 
 	/* Force persistent stats re-read upon next open of the table
 	so that FLUSH TABLE can be used to forcibly fetch stats from disk
 	if they have been manually modified. We reset table->stat_initialized
 	only if table reference count is 0 because we do not want too frequent
 	stats re-reads (e.g. in other cases than FLUSH TABLE). */
-	if (strchr(table->name, '/') != NULL
-	    && table->n_ref_count == 0
+	if (last_handle && strchr(table->name, '/') != NULL
 	    && dict_stats_is_persistent_enabled(table)) {
 
 		dict_stats_deinit(table);
@@ -598,11 +597,8 @@ dict_table_close(
 
 	if (!dict_locked) {
 		table_id_t	table_id	= table->id;
-		ibool		drop_aborted;
-
-		drop_aborted = try_drop
+		const bool	drop_aborted	= last_handle && try_drop
 			&& table->drop_aborted
-			&& table->n_ref_count == 1
 			&& dict_table_get_first_index(table);
 
 		mutex_exit(&dict_sys->mutex);
@@ -2096,8 +2092,9 @@ dict_table_remove_from_cache_low(
 	}
 
 	if (lru_evict && table->drop_aborted) {
-		/* Do as dict_table_try_drop_aborted() does. */
-
+		/* When evicting the table definition,
+		drop the orphan indexes from the data dictionary
+		and free the index pages. */
 		trx_t* trx = trx_allocate_for_background();
 
 		ut_ad(mutex_own(&dict_sys->mutex));
@@ -2108,12 +2105,8 @@ dict_table_remove_from_cache_low(
 		trx->dict_operation_lock_mode = RW_X_LATCH;
 
 		trx_set_dict_operation(trx, TRX_DICT_OP_INDEX);
+		row_merge_drop_indexes_dict(trx, table->id);
 
-		/* Silence a debug assertion in row_merge_drop_indexes(). */
-		ut_d(table->n_ref_count++);
-		row_merge_drop_indexes(trx, table, TRUE);
-		ut_d(table->n_ref_count--);
-		ut_ad(table->n_ref_count == 0);
 		trx_commit_for_mysql(trx);
 		trx->dict_operation_lock_mode = 0;
 		trx_free_for_background(trx);