Follow-up to MDEV-13407 innodb.drop_table_background failed in buildbot with...

Follow-up to MDEV-13407 innodb.drop_table_background failed in buildbot with "Tablespace for table exists"

The InnoDB background DROP TABLE queue is something that we should
really remove, but are unable to until we remove dict_operation_lock
so that DDL and DML operations can be combined in a single transaction.

Because the queue is not persistent, it is not crash-safe. We should
in some way ensure that the deferred-dropped tables will be dropped
after server restart.

The existence of two separate transactions complicates the error handling
of CREATE TABLE...SELECT. We should really not break locks in DROP TABLE.

Our solution to these problems is to rename the table to a temporary
name, and to drop such-named tables on InnoDB startup. Also, the
queue will use table IDs instead of names from now on.

check-testcase.test: Ignore #sql-ib*.ibd files, because tables may enter
the background DROP TABLE queue shortly before the test finishes.

innodb.drop_table_background: Test CREATE...SELECT and the creation of
tables whose file name starts with #sql-ib.

innodb.alter_crash: Adjust the recovery, now that the #sql-ib tables
will be dropped on InnoDB startup.

row_mysql_drop_garbage_tables(): New function, to drop all #sql-ib tables
on InnoDB startup.

row_drop_table_for_mysql_in_background(): Remove an unnecessary and
misplaced call to log_buffer_flush_to_disk(). (The call should have been
after the transaction commit. We do not care about flushing the redo log
here, because the table would be dropped again at server startup.)

Remove the entry from the list after the table no longer exists.

If server shutdown has been initiated, empty the list without actually
dropping any tables. They will be dropped again on startup.

row_drop_table_for_mysql(): Do not call lock_remove_all_on_table().
Instead, if locks exist, defer the DROP TABLE until they do not exist.
If the table name does not start with #sql-ib, rename it to that prefix
before adding it to the background DROP TABLE queue.

mysql-test/include/check-testcase.test

@@ -82,7 +82,10 @@ call mtr.check_testcase();
 
 let $datadir=`select @@datadir`;
 list_files $datadir mysql_upgrade_info;
-list_files $datadir/test #sql*;
+list_files_write_file $datadir.tempfiles.txt $datadir/test #sql*;
+--replace_regex /#sql-ib[0-9a-f]+-[0-9a-f]+\.ibd\n//
+cat_file $datadir.tempfiles.txt;
+remove_file $datadir.tempfiles.txt;
 list_files $datadir/mysql #sql*;
 
 --enable_query_log

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

@@ -44,10 +44,9 @@ SET DEBUG_DBUG='+d,innodb_alter_commit_crash_after_commit';
 ALTER TABLE t1 ADD PRIMARY KEY (f2, f1);
 ERROR HY000: Lost connection to MySQL server during query
 # Restart mysqld after the crash and reconnect.
-# Manual *.frm recovery begin.
-# Manual recovery end
-FLUSH TABLES;
-# Drop the orphaned original table.
+SELECT * FROM information_schema.innodb_sys_tables
+WHERE table_id = ID;
+TABLE_ID	NAME	FLAG	N_COLS	SPACE	FILE_FORMAT	ROW_FORMAT	ZIP_PAGE_SIZE	SPACE_TYPE
 # Files in datadir after manual recovery.
 t1.frm
 t1.ibd
@@ -83,11 +82,9 @@ SET DEBUG_DBUG='+d,innodb_alter_commit_crash_before_commit';
 ALTER TABLE t2 ADD PRIMARY KEY (f2, f1);
 ERROR HY000: Lost connection to MySQL server during query
 # Startup the server after the crash
-# Read and remember the temporary table name
-# Manual *.frm recovery begin. The dictionary was not updated
-# and the files were not renamed. The rebuilt table
-# was left behind on purpose, to faciliate data recovery.
-# Manual recovery end
+SELECT * FROM information_schema.innodb_sys_tables
+WHERE name LIKE 'test/#sql-ib%';
+TABLE_ID	NAME	FLAG	N_COLS	SPACE	FILE_FORMAT	ROW_FORMAT	ZIP_PAGE_SIZE	SPACE_TYPE
 # Drop the orphaned rebuilt table.
 SHOW TABLES;
 Tables_in_test
@@ -123,10 +120,10 @@ SET DEBUG_DBUG='+d,innodb_alter_commit_crash_after_commit';
 ALTER TABLE t1 ADD INDEX (b), CHANGE c d int, ALGORITHM=INPLACE;
 ERROR HY000: Lost connection to MySQL server during query
 # Restart mysqld after the crash and reconnect.
-# Manual *.frm recovery begin.
-# Manual recovery end
+SELECT * FROM information_schema.innodb_sys_tables
+WHERE table_id = ID;
+TABLE_ID	NAME	FLAG	N_COLS	SPACE	FILE_FORMAT	ROW_FORMAT	ZIP_PAGE_SIZE	SPACE_TYPE
 FLUSH TABLES;
-# Drop the orphaned original table.
 # Files in datadir after manual recovery.
 t1.frm
 t1.ibd

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

@@ -3,7 +3,22 @@ KEY(c1), KEY(c2), KEY(c2,c1),
 KEY(c3), KEY(c3,c1), KEY(c3,c2), KEY(c3,c2,c1),
 KEY(c4), KEY(c4,c1), KEY(c4,c2), KEY(c4,c2,c1),
 KEY(c4,c3), KEY(c4,c3,c1), KEY(c4,c3,c2), KEY(c4,c3,c2,c1)) ENGINE=InnoDB;
+CREATE TABLE `#mysql50##sql-ib-foo`(a SERIAL) ENGINE=InnoDB;
+INSERT INTO t (c1) VALUES (1),(2),(1);
 SET DEBUG_DBUG='+d,row_drop_table_add_to_background';
+CREATE TABLE target (PRIMARY KEY(c1)) ENGINE=InnoDB SELECT * FROM t;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+SELECT * from target;
+ERROR 42S02: Table 'test.target' doesn't exist
 DROP TABLE t;
 CREATE TABLE t (a INT) ENGINE=InnoDB;
 DROP TABLE t;
+DROP TABLE target;
+ERROR 42S02: Unknown table 'test.target'
+CREATE TABLE target (a INT) ENGINE=InnoDB;
+DROP TABLE target;
+SELECT * FROM `#mysql50##sql-ib-foo`;
+ERROR 42S02: Table 'test.#mysql50##sql-ib-foo' doesn't exist in engine
+DROP TABLE `#mysql50##sql-ib-foo`;
+Warnings:
+Warning	1932	Table 'test.#mysql50##sql-ib-foo' doesn't exist in engine

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

@@ -75,28 +75,22 @@ ALTER TABLE t1 ADD PRIMARY KEY (f2, f1);
 --echo # Restart mysqld after the crash and reconnect.
 --source include/start_mysqld.inc
 
-let $temp_table_name = `SELECT SUBSTR(name, 6)
-	FROM information_schema.innodb_sys_tables
-	WHERE table_id = $orig_table_id`;
-
---echo # Manual *.frm recovery begin.
-
---move_file $MYSQLD_DATADIR/test/t1.frm $MYSQLD_DATADIR/test/$temp_table_name.frm
-
+let TABLENAME_INC= $MYSQLTEST_VARDIR/tmp/tablename.inc;
 perl;
-my @frm_file = glob "$ENV{'datadir'}/test/#sql-*.frm";
-my $t1_frm = "$ENV{'datadir'}/test/t1.frm";
-rename($frm_file[0], $t1_frm);
+die unless open OUT, ">$ENV{TABLENAME_INC}";
+chdir "$ENV{'datadir'}/test";
+my @frm_file = map { substr($_, 0, -4) } glob "#sql-*.frm";
+print OUT 'let $tablename=', $frm_file[0], ';';
+close OUT or die;
 EOF
+source $TABLENAME_INC;
+remove_file $TABLENAME_INC;
 
---echo # Manual recovery end
-
-FLUSH TABLES;
+--replace_result $orig_table_id ID
+eval SELECT * FROM information_schema.innodb_sys_tables
+WHERE table_id = $orig_table_id;
 
---echo # Drop the orphaned original table.
---disable_query_log
-eval DROP TABLE `#mysql50#$temp_table_name`;
---enable_query_log
+move_file $datadir/test/$tablename.frm $datadir/test/t1.frm;
 
 --echo # Files in datadir after manual recovery.
 --list_files $MYSQLD_DATADIR/test
@@ -134,16 +128,9 @@ ALTER TABLE t2 ADD PRIMARY KEY (f2, f1);
 --echo # Startup the server after the crash
 --source include/start_mysqld.inc
 
---echo # Read and remember the temporary table name
-let $temp_table_name = `SELECT SUBSTRING(name,6)
-	FROM information_schema.innodb_sys_tables
-	WHERE name LIKE "test/#sql-ib$orig_table_id%"`;
-
---echo # Manual *.frm recovery begin. The dictionary was not updated
---echo # and the files were not renamed. The rebuilt table
---echo # was left behind on purpose, to faciliate data recovery.
+SELECT * FROM information_schema.innodb_sys_tables
+WHERE name LIKE 'test/#sql-ib%';
 
-let TABLENAME_INC= $MYSQLTEST_VARDIR/tmp/tablename.inc;
 perl;
 die unless open OUT, ">$ENV{TABLENAME_INC}";
 chdir "$ENV{'datadir'}/test";
@@ -154,8 +141,6 @@ EOF
 source $TABLENAME_INC;
 remove_file $TABLENAME_INC;
 
---echo # Manual recovery end
-
 --echo # Drop the orphaned rebuilt table.
 --disable_query_log
 eval DROP TABLE `#mysql50#$tablename`;
@@ -198,28 +183,24 @@ ALTER TABLE t1 ADD INDEX (b), CHANGE c d int, ALGORITHM=INPLACE;
 --echo # Restart mysqld after the crash and reconnect.
 --source include/start_mysqld.inc
 
-let $temp_table_name = `SELECT SUBSTR(name, 6)
-	FROM information_schema.innodb_sys_tables
-	WHERE table_id = $orig_table_id`;
-
---echo # Manual *.frm recovery begin.
---move_file $MYSQLD_DATADIR/test/t1.frm $MYSQLD_DATADIR/test/$temp_table_name.frm
+--replace_result $orig_table_id ID
+eval SELECT * FROM information_schema.innodb_sys_tables
+WHERE table_id = $orig_table_id;
 
 perl;
-my @frm_file = glob "$ENV{'datadir'}/test/#sql-*.frm";
-my $t1_frm = "$ENV{'datadir'}/test/t1.frm";
-rename($frm_file[0], $t1_frm);
+die unless open OUT, ">$ENV{TABLENAME_INC}";
+chdir "$ENV{'datadir'}/test";
+my @frm_file = map { substr($_, 0, -4) } glob "#sql-*.frm";
+print OUT 'let $tablename=', $frm_file[0], ';';
+close OUT or die;
 EOF
 
---echo # Manual recovery end
+source $TABLENAME_INC;
+remove_file $TABLENAME_INC;
+move_file $datadir/test/$tablename.frm $datadir/test/t1.frm;
 
 FLUSH TABLES;
 
---echo # Drop the orphaned original table.
---disable_query_log
-eval DROP TABLE `#mysql50#$temp_table_name`;
---enable_query_log
-
 --echo # Files in datadir after manual recovery.
 --list_files $MYSQLD_DATADIR/test
 

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

@@ -9,6 +9,9 @@ KEY(c3), KEY(c3,c1), KEY(c3,c2), KEY(c3,c2,c1),
 KEY(c4), KEY(c4,c1), KEY(c4,c2), KEY(c4,c2,c1),
 KEY(c4,c3), KEY(c4,c3,c1), KEY(c4,c3,c2), KEY(c4,c3,c2,c1)) ENGINE=InnoDB;
 
+CREATE TABLE `#mysql50##sql-ib-foo`(a SERIAL) ENGINE=InnoDB;
+INSERT INTO t (c1) VALUES (1),(2),(1);
+
 let $n= 10;
 
 SET DEBUG_DBUG='+d,row_drop_table_add_to_background';
@@ -24,7 +27,18 @@ while ($i) {
   dec $i;
 }
 --enable_query_log
+--error ER_DUP_ENTRY
+CREATE TABLE target (PRIMARY KEY(c1)) ENGINE=InnoDB SELECT * FROM t;
+--error ER_NO_SUCH_TABLE
+SELECT * from target;
 DROP TABLE t;
 --source include/restart_mysqld.inc
 CREATE TABLE t (a INT) ENGINE=InnoDB;
 DROP TABLE t;
+--error ER_BAD_TABLE_ERROR
+DROP TABLE target;
+CREATE TABLE target (a INT) ENGINE=InnoDB;
+DROP TABLE target;
+--error ER_NO_SUCH_TABLE_IN_ENGINE
+SELECT * FROM `#mysql50##sql-ib-foo`;
+DROP TABLE `#mysql50##sql-ib-foo`;

storage/innobase/include/row0mysql.h

@@ -420,6 +420,10 @@ ulint
 row_get_background_drop_list_len_low(void);
 /*======================================*/
 
+/** Drop garbage tables during recovery. */
+void
+row_mysql_drop_garbage_tables();
+
 /*********************************************************************//**
 Sets an exclusive lock on a table.
 @return error code or DB_SUCCESS */

storage/innobase/log/log0recv.cc

@@ -3410,6 +3410,8 @@ recv_recovery_rollback_active(void)
 
 		/* Drop partially created indexes. */
 		row_merge_drop_temp_indexes();
+		/* Drop garbage tables. */
+		row_mysql_drop_garbage_tables();
 
 		/* Drop any auxiliary tables that were not dropped when the
 		parent table was dropped. This can happen if the parent table

storage/innobase/row/row0mysql.cc

@@ -64,6 +64,7 @@ Created 9/17/2000 Heikki Tuuri
 #include "trx0roll.h"
 #include "trx0undo.h"
 #include "row0ext.h"
+#include "srv0start.h"
 #include "ut0new.h"
 
 #include <algorithm>
@@ -2776,12 +2777,6 @@ row_drop_table_for_mysql_in_background(
 
 	error = row_drop_table_for_mysql(name, trx, FALSE, FALSE);
 
-	/* Flush the log to reduce probability that the .frm files and
-	the InnoDB data dictionary get out-of-sync if the user runs
-	with innodb_flush_log_at_trx_commit = 0 */
-
-	log_buffer_flush_to_disk();
-
 	trx_commit_for_mysql(trx);
 
 	trx_free_for_background(trx);
@@ -2819,8 +2814,11 @@ row_drop_tables_for_mysql_in_background(void)
 		return(n_tables + n_tables_dropped);
 	}
 
-	table = dict_table_open_on_id(drop->table_id, FALSE,
-				      DICT_TABLE_OP_OPEN_ONLY_IF_CACHED);
+	/* On fast shutdown, just empty the list without dropping tables. */
+	table = srv_shutdown_state == SRV_SHUTDOWN_NONE || !srv_fast_shutdown
+		? dict_table_open_on_id(drop->table_id, FALSE,
+					DICT_TABLE_OP_OPEN_ONLY_IF_CACHED)
+		: NULL;
 
 	if (!table) {
 		n_tables_dropped++;
@@ -2875,6 +2873,74 @@ row_get_background_drop_list_len_low(void)
 	return(len);
 }
 
+/** Drop garbage tables during recovery. */
+void
+row_mysql_drop_garbage_tables()
+{
+	mem_heap_t*	heap = mem_heap_create(FN_REFLEN);
+	btr_pcur_t	pcur;
+	mtr_t		mtr;
+	trx_t*		trx = trx_allocate_for_background();
+	trx->op_info = "dropping garbage tables";
+	row_mysql_lock_data_dictionary(trx);
+
+	mtr.start();
+	btr_pcur_open_at_index_side(
+		true, dict_table_get_first_index(dict_sys->sys_tables),
+		BTR_SEARCH_LEAF, &pcur, true, 0, &mtr);
+
+	for (;;) {
+		const rec_t*	rec;
+		const byte*	field;
+		ulint		len;
+		const char*	table_name;
+
+		btr_pcur_move_to_next_user_rec(&pcur, &mtr);
+
+		if (!btr_pcur_is_on_user_rec(&pcur)) {
+			break;
+		}
+
+		rec = btr_pcur_get_rec(&pcur);
+		if (rec_get_deleted_flag(rec, 0)) {
+			continue;
+		}
+
+		field = rec_get_nth_field_old(rec, 0/*NAME*/, &len);
+		if (len == UNIV_SQL_NULL || len == 0) {
+			/* Corrupted SYS_TABLES.NAME */
+			continue;
+		}
+
+		table_name = mem_heap_strdupl(
+			heap,
+			reinterpret_cast<const char*>(field), len);
+		if (strstr(table_name, "/" TEMP_FILE_PREFIX_INNODB)) {
+			btr_pcur_store_position(&pcur, &mtr);
+			btr_pcur_commit_specify_mtr(&pcur, &mtr);
+
+			if (dict_load_table(table_name, true,
+					    DICT_ERR_IGNORE_ALL)) {
+				row_drop_table_for_mysql(
+					table_name, trx, FALSE, FALSE);
+				trx_commit_for_mysql(trx);
+			}
+
+			mtr.start();
+			btr_pcur_restore_position(BTR_SEARCH_LEAF,
+						  &pcur, &mtr);
+		}
+
+		mem_heap_empty(heap);
+	}
+
+	btr_pcur_close(&pcur);
+	mtr.commit();
+	row_mysql_unlock_data_dictionary(trx);
+	trx_free_for_background(trx);
+	mem_heap_free(heap);
+}
+
 /*********************************************************************//**
 If a table is not yet in the drop list, adds the table to the list of tables
 which the master thread drops in background. We need this on Unix because in
@@ -3645,11 +3711,7 @@ row_drop_table_for_mysql(
 	}
 
 
-	DBUG_EXECUTE_IF("row_drop_table_add_to_background",
-		row_add_table_to_background_drop_list(table->id);
-		err = DB_SUCCESS;
-		goto funct_exit;
-	);
+	DBUG_EXECUTE_IF("row_drop_table_add_to_background", goto defer;);
 
 	/* TODO: could we replace the counter n_foreign_key_checks_running
 	with lock checks on the table? Acquire here an exclusive lock on the
@@ -3658,17 +3720,22 @@ row_drop_table_for_mysql(
 	checks take an IS or IX lock on the table. */
 
 	if (table->n_foreign_key_checks_running > 0) {
-		if (row_add_table_to_background_drop_list(table->id)) {
-			ib::info() << "You are trying to drop table "
-				<< table->name
-				<< " though there is a foreign key check"
-				" running on it. Adding the table to the"
-				" background drop queue.";
+defer:
+		if (!strstr(table->name.m_name, "/" TEMP_FILE_PREFIX_INNODB)) {
+			heap = mem_heap_create(FN_REFLEN);
+			const char* tmp_name
+				= dict_mem_create_temporary_tablename(
+					heap, table->name.m_name, table->id);
+			ib::info() << "Deferring DROP TABLE " << table->name
+				   << "; renaming to " << tmp_name;
+			err = row_rename_table_for_mysql(
+				table->name.m_name, tmp_name, trx, false);
+		} else {
+			err = DB_SUCCESS;
+		}
+		if (err == DB_SUCCESS) {
+			row_add_table_to_background_drop_list(table->id);
 		}
-
-		/* We return DB_SUCCESS to MySQL though the drop will
-		happen lazily later */
-		err = DB_SUCCESS;
 		goto funct_exit;
 	}
 
@@ -3689,26 +3756,9 @@ row_drop_table_for_mysql(
 	/* Wait on background threads to stop using table */
 	fil_wait_crypt_bg_threads(table);
 
-	if (table->get_ref_count() == 0) {
-		lock_remove_all_on_table(table, TRUE);
-		ut_a(table->n_rec_locks == 0);
-	} else if (table->get_ref_count() > 0 || table->n_rec_locks > 0) {
-		if (row_add_table_to_background_drop_list(table->id)) {
-			ib::info() << "MySQL is trying to drop table "
-				<< table->name
-				<< " though there are still open handles to"
-				" it. Adding the table to the background drop"
-				" queue.";
-
-			/* We return DB_SUCCESS to MySQL though the drop will
-			happen lazily later */
-			err = DB_SUCCESS;
-		} else {
-			/* The table is already in the background drop list */
-			err = DB_ERROR;
-		}
-
-		goto funct_exit;
+	if (table->get_ref_count() > 0 || table->n_rec_locks > 0
+	    || lock_table_has_locks(table)) {
+		goto defer;
 	}
 
 	/* The "to_be_dropped" marks table that is to be dropped, but
@@ -3718,11 +3768,6 @@ row_drop_table_for_mysql(
 	and it is free to be dropped */
 	table->to_be_dropped = false;
 
-	/* If we get this far then the table to be dropped must not have
-	any table or record locks on it. */
-
-	ut_a(!lock_table_has_locks(table));
-
 	switch (trx_get_dict_operation(trx)) {
 	case TRX_DICT_OP_NONE:
 		trx_set_dict_operation(trx, TRX_DICT_OP_TABLE);