#At file:///export/home/z/mysql-azalea-bugfixing-bug42546/ based on revid:alik@stripped
2804 Jon Olav Hauglid 2009-06-23
Bug#42546 Backup: RESTORE fails, thinking it finds an existing table
The problem occured when a MDL locking conflict happened for a non-existent
table between a CREATE and a INSERT statement. The code for CREATE
interpreted this lock conflict to mean that the table existed,
which meant that the statement failed when it should not have.
This patch changes the code for CREATE and CREATE LIKE when not in
LOCK TABLES mode, to wait until it can get an exclusive metadata lock
on the table name. This means that for the test case in the bug
description, CREATE will wait until INSERT completes.
The patch also changes the way CREATE and CREATE LIKE checks for
ER_TABLE_EXISTS_ERROR. This is now done by first checking the TDC
and then for a .frm-file.
For CREATE LIKE, the patch changes the locking order. Now, the
exclusive metadata lock on the target table is taken first before
the shared metadata lock on the source table. This is done to
prevent deadlock.
Note that the problem remains for LOCK TABLES mode where we cannot
wait for exclusive metadata lock for fear of deadlock. It also
remains for ALTER TABLE RENAME where changing the locking order
is difficult.
Test case based on the bug description added to create.test.
modified:
mysql-test/r/create.result
mysql-test/t/create.test
sql/sql_base.cc
sql/sql_table.cc
=== modified file 'mysql-test/r/create.result'
--- a/mysql-test/r/create.result 2009-05-28 21:29:31 +0000
+++ b/mysql-test/r/create.result 2009-06-23 12:08:59 +0000
@@ -371,7 +371,7 @@ ERROR 42S01: Table 't3' already exists
create table non_existing_database.t1 like t1;
ERROR 42000: Unknown database 'non_existing_database'
create table t3 like non_existing_table;
-ERROR 42S02: Table 'test.non_existing_table' doesn't exist
+ERROR 42S01: Table 't3' already exists
create temporary table t3 like t1;
ERROR 42S01: Table 't3' already exists
drop table t1, t2, t3;
@@ -1934,3 +1934,29 @@ END ;|
ERROR 42000: This version of MySQL doesn't yet support 'multiple triggers with the same action time and event for one table'
DROP TABLE t1;
DROP TABLE B;
+#
+# Bug#42546 - Backup: RESTORE fails, thinking it finds an existing table
+#
+DROP TABLE IF EXISTS t1;
+# Start insert on the not-yet existing table
+# Wait after taking the MDL
+SET DEBUG_SYNC= 'after_open_table_mdl_shared SIGNAL locked WAIT_FOR finish
+ EXECUTE 2';
+INSERT INTO t1 VALUES(1,"def");
+# Need to sync twice. Number 1 is consumed by General Log.
+SET DEBUG_SYNC= 'now WAIT_FOR locked';
+SET DEBUG_SYNC= 'now SIGNAL finish';
+SET DEBUG_SYNC= 'now WAIT_FOR locked';
+# Now INSERT has a MDL on the non-existent table t1.
+#
+# Continue the INSERT once CREATE waits for exclusive lock
+SET DEBUG_SYNC= 'mdl_acquire_exclusive_locks_wait SIGNAL finish';
+# Try to create that table.
+CREATE TABLE t1 (c1 INT, c2 VARCHAR(100), KEY(c1)) ENGINE=MyISAM;
+# Insert fails
+ERROR 42S02: Table 'test.t1' doesn't exist
+SET DEBUG_SYNC= 'RESET';
+SHOW TABLES;
+DROP TABLE IF EXISTS t1;
+Tables_in_test
+t1
=== modified file 'mysql-test/t/create.test'
--- a/mysql-test/t/create.test 2009-05-28 21:29:31 +0000
+++ b/mysql-test/t/create.test 2009-06-23 12:08:59 +0000
@@ -1,3 +1,5 @@
+--source include/have_debug_sync.inc
+
#
# Check some special create statements.
#
@@ -306,7 +308,7 @@ create table t3 like t1;
create table t3 like mysqltest.t3;
--error 1049
create table non_existing_database.t1 like t1;
---error ER_NO_SUCH_TABLE
+--error ER_TABLE_EXISTS_ERROR
create table t3 like non_existing_table;
--error 1050
create temporary table t3 like t1;
@@ -1600,3 +1602,51 @@ END ;|
DROP TABLE t1;
DROP TABLE B;
+
+###########################################################################
+
+--echo #
+--echo # Bug#42546 - Backup: RESTORE fails, thinking it finds an existing table
+--echo #
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+connect(con1, localhost, root,,);
+connection con1;
+
+--echo # Start insert on the not-yet existing table
+--echo # Wait after taking the MDL
+SET DEBUG_SYNC= 'after_open_table_mdl_shared SIGNAL locked WAIT_FOR finish
+ EXECUTE 2';
+--send INSERT INTO t1 VALUES(1,"def")
+
+connection default;
+
+--echo # Need to sync twice. Number 1 is consumed by General Log.
+SET DEBUG_SYNC= 'now WAIT_FOR locked';
+SET DEBUG_SYNC= 'now SIGNAL finish';
+SET DEBUG_SYNC= 'now WAIT_FOR locked';
+--echo # Now INSERT has a MDL on the non-existent table t1.
+
+--echo #
+--echo # Continue the INSERT once CREATE waits for exclusive lock
+SET DEBUG_SYNC= 'mdl_acquire_exclusive_locks_wait SIGNAL finish';
+--echo # Try to create that table.
+--send CREATE TABLE t1 (c1 INT, c2 VARCHAR(100), KEY(c1)) ENGINE=MyISAM
+
+--echo # Insert fails
+connection con1;
+--error ER_NO_SUCH_TABLE
+--reap
+
+connection default;
+
+SET DEBUG_SYNC= 'RESET';
+
+SHOW TABLES;
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2009-06-17 07:30:19 +0000
+++ b/sql/sql_base.cc 2009-06-23 12:08:59 +0000
@@ -2570,6 +2570,7 @@ bool open_table(THD *thd, TABLE_LIST *ta
DEBUG_SYNC(thd, "before_open_table_wait_refresh");
DBUG_RETURN(TRUE);
}
+ DEBUG_SYNC(thd, "after_open_table_mdl_shared");
}
/*
=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc 2009-06-02 11:46:19 +0000
+++ b/sql/sql_table.cc 2009-06-23 12:08:59 +0000
@@ -4001,7 +4001,7 @@ static bool lock_table_name_if_not_cache
const char *table_name,
MDL_request **mdl_request)
{
- bool conflict;
+ bool conflict;
if (!(*mdl_request= MDL_request::create(0, db, table_name, thd->mem_root)))
return TRUE;
@@ -4019,7 +4019,7 @@ static bool lock_table_name_if_not_cache
/* Probably OOM. */
return TRUE;
}
- else
+ else
*mdl_request= NULL;
}
else
@@ -4069,27 +4069,58 @@ bool mysql_create_table(THD *thd, const
if (!(create_info->options & HA_LEX_CREATE_TMP_TABLE))
{
- if (lock_table_name_if_not_cached(thd, db, table_name, &target_mdl_request))
+ /*
+ Take exclusive metadata lock on the table name. If we are in LOCK TABLES mode,
+ we cannot wait in case of lock conflict for fear of deadlock.
+ */
+ if (thd->locked_tables_mode)
{
- result= TRUE;
- goto unlock;
+ if (lock_table_name_if_not_cached(thd, db, table_name, &target_mdl_request))
+ {
+ result= TRUE;
+ goto unlock;
+ }
}
- if (!target_mdl_request)
+ else
{
- if (create_info->options & HA_LEX_CREATE_IF_NOT_EXISTS)
+ if (!(target_mdl_request= MDL_request::create(0, db, table_name, thd->mem_root)))
{
- push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
- ER_TABLE_EXISTS_ERROR, ER(ER_TABLE_EXISTS_ERROR),
- table_name);
- create_info->table_existed= 1;
- result= FALSE;
+ result= TRUE;
+ goto unlock;
}
- else
+ target_mdl_request->set_type(MDL_EXCLUSIVE);
+ thd->mdl_context.add_request(target_mdl_request);
+ if (thd->mdl_context.acquire_exclusive_locks())
{
- my_error(ER_TABLE_EXISTS_ERROR,MYF(0),table_name);
- result= TRUE;
+ result= TRUE;
+ goto unlock;
+ }
+ DEBUG_SYNC(thd, "locked_table_name");
+ }
+
+ /* Check if the table exists already */
+ if (!target_mdl_request->ticket->get_cached_object())
+ {
+ char dst_path[FN_REFLEN];
+ uint dst_path_length= build_table_filename(dst_path, sizeof(dst_path),
+ db, table_name, reg_ext, 0);
+ if (!access(dst_path, F_OK))
+ {
+ if (create_info->options & HA_LEX_CREATE_IF_NOT_EXISTS)
+ {
+ push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
+ ER_TABLE_EXISTS_ERROR, ER(ER_TABLE_EXISTS_ERROR),
+ table_name);
+ create_info->table_existed= 1;
+ result= FALSE;
+ }
+ else
+ {
+ my_error(ER_TABLE_EXISTS_ERROR,MYF(0),table_name);
+ result= TRUE;
+ }
+ goto unlock;
}
- goto unlock;
}
}
result= mysql_create_table_no_lock(thd, db, table_name, create_info,
@@ -5159,24 +5190,12 @@ bool mysql_create_like_table(THD* thd, T
if (!(create_info->options & HA_LEX_CREATE_TMP_TABLE))
global_schema_lock_guard.lock();
- /*
- By opening source table and thus acquiring shared metadata lock on it
- we guarantee that it exists and no concurrent DDL operation will mess
- with it. Later we also take an exclusive metadata lock on target table
- name, which makes copying of .frm file, call to ha_create_table() and
- binlogging atomic against concurrent DML and DDL operations on target
- table. Thus by holding both these "locks" we ensure that our statement
- is properly isolated from all concurrent operations which matter.
- */
- if (open_tables(thd, &src_table, ¬_used, 0))
- DBUG_RETURN(TRUE);
+ /* Used by create-big.test */
+ DBUG_EXECUTE_IF("sleep_create_like_before_check_if_exists", my_sleep(6000000););
- strxmov(src_path, src_table->table->s->path.str, reg_ext, NullS);
-
- DBUG_EXECUTE_IF("sleep_create_like_before_check_if_exists", my_sleep(6000000););
-
- /*
- Check that destination tables does not exist. Note that its name
+ /*
+ Get exclusive metadata lock on the target table and
+ check that destination table does not exist. Note that its name
was already checked when it was added to the table list.
*/
if (create_info->options & HA_LEX_CREATE_TMP_TABLE)
@@ -5188,14 +5207,35 @@ bool mysql_create_like_table(THD* thd, T
}
else
{
- if (lock_table_name_if_not_cached(thd, db, table_name, &target_mdl_request))
- goto err;
- if (!target_mdl_request)
- goto table_exists;
- dst_path_length= build_table_filename(dst_path, sizeof(dst_path),
- db, table_name, reg_ext, 0);
- if (!access(dst_path, F_OK))
- goto table_exists;
+ /*
+ Take exclusive metadata lock on the table name. If we are in LOCK TABLES mode,
+ we cannot wait in case of lock conflict for fear of deadlock.
+ */
+ if (thd->locked_tables_mode)
+ {
+ if (lock_table_name_if_not_cached(thd, db, table_name, &target_mdl_request))
+ goto err;
+ }
+ else
+ {
+ if (!(target_mdl_request= MDL_request::create(0, db, table_name, thd->mem_root)))
+ goto err;
+ target_mdl_request->set_type(MDL_EXCLUSIVE);
+ thd->mdl_context.add_request(target_mdl_request);
+ if (thd->mdl_context.acquire_exclusive_locks())
+ goto err;
+ DEBUG_SYNC(thd, "locked_table_name");
+ }
+
+ /* Check if the table exists already */
+ if (!target_mdl_request->ticket->get_cached_object())
+ {
+ dst_path_length= build_table_filename(dst_path, sizeof(dst_path),
+ db, table_name, reg_ext, 0);
+ if (!access(dst_path, F_OK))
+ goto table_exists;
+ }
+
/*
Make the metadata lock available to open_table() called to
reopen the table down the road.
@@ -5203,6 +5243,20 @@ bool mysql_create_like_table(THD* thd, T
table->mdl_request= target_mdl_request;
}
+ /*
+ By opening source table and thus acquiring shared metadata lock on it
+ we guarantee that it exists and no concurrent DDL operation will mess
+ with it. We already have an exclusive metadata lock on target table
+ name, which makes copying of .frm file, call to ha_create_table() and
+ binlogging atomic against concurrent DML and DDL operations on target
+ table. Thus by holding both these "locks" we ensure that our statement
+ is properly isolated from all concurrent operations which matter.
+ */
+ if (open_tables(thd, &src_table, ¬_used, 0))
+ DBUG_RETURN(TRUE);
+
+ strxmov(src_path, src_table->table->s->path.str, reg_ext, NullS);
+
DBUG_EXECUTE_IF("sleep_create_like_before_copy", my_sleep(6000000););
/*
Attachment: [text/bzr-bundle] bzr/jon.hauglid@sun.com-20090623120859-ygeg0ljuc60h1hbi.bundle
