diff --git a/mysql-test/r/partition_sync.result b/mysql-test/r/partition_sync.result index 41ca19426fe..fbea2e0273a 100644 --- a/mysql-test/r/partition_sync.result +++ b/mysql-test/r/partition_sync.result @@ -1,2 +1,34 @@ # Disabled until Bug#46654 False deadlock on concurrent DML/DDL # with partitions, inconsistent behavior is backported +# +# Bug #46654 False deadlock on concurrent DML/DDL +# with partitions, inconsistent behavior +# +DROP TABLE IF EXISTS tbl_with_partitions; +CREATE TABLE tbl_with_partitions ( i INT ) +PARTITION BY HASH(i); +INSERT INTO tbl_with_partitions VALUES (1); +# Connection 3 +LOCK TABLE tbl_with_partitions READ; +# Connection 1 +# Access table with disabled autocommit +SET AUTOCOMMIT = 0; +SELECT * FROM tbl_with_partitions; +i +1 +# Connection 2 +# Alter table, abort after prepare +set session debug="+d,abort_copy_table"; +ALTER TABLE tbl_with_partitions ADD COLUMN f INT; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +# Connection 1 +# Try accessing the table after Alter aborted. +# This used to give ER_LOCK_DEADLOCK. +SELECT * FROM tbl_with_partitions; +i +1 +# Connection 3 +UNLOCK TABLES; +# Connection 1 +# Cleanup +DROP TABLE tbl_with_partitions; diff --git a/mysql-test/t/partition_sync.test b/mysql-test/t/partition_sync.test index 5d2b25e87f3..672e4cc0562 100644 --- a/mysql-test/t/partition_sync.test +++ b/mysql-test/t/partition_sync.test @@ -39,6 +39,57 @@ #DROP TABLE t1; +--echo # +--echo # Bug #46654 False deadlock on concurrent DML/DDL +--echo # with partitions, inconsistent behavior +--echo # + +--disable_warnings +DROP TABLE IF EXISTS tbl_with_partitions; +--enable_warnings + +CREATE TABLE tbl_with_partitions ( i INT ) + PARTITION BY HASH(i); +INSERT INTO tbl_with_partitions VALUES (1); + +connect(con2,localhost,root); +connect(con3,localhost,root); + +--echo # Connection 3 +connection con3; +LOCK TABLE tbl_with_partitions READ; + +--echo # Connection 1 +--echo # Access table with disabled autocommit +connection default; +SET AUTOCOMMIT = 0; +SELECT * FROM tbl_with_partitions; + +--echo # Connection 2 +--echo # Alter table, abort after prepare +connection con2; +set session debug="+d,abort_copy_table"; +--error ER_LOCK_WAIT_TIMEOUT +ALTER TABLE tbl_with_partitions ADD COLUMN f INT; + +--echo # Connection 1 +--echo # Try accessing the table after Alter aborted. +--echo # This used to give ER_LOCK_DEADLOCK. +connection default; +SELECT * FROM tbl_with_partitions; + +--echo # Connection 3 +connection con3; +UNLOCK TABLES; + +--echo # Connection 1 +--echo # Cleanup +connection default; +disconnect con2; +disconnect con3; +DROP TABLE tbl_with_partitions; + + # Check that all connections opened by test cases in this file are really # gone so execution of other tests won't be affected by their presence. --source include/wait_until_count_sessions.inc diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 7a3adc89ea9..9672647f30b 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1530,8 +1530,8 @@ bool close_thread_table(THD *thd, TABLE **table_ptr) *table_ptr=table->next; table->mdl_ticket= NULL; - if (table->needs_reopen() || - thd->version != refresh_version || !table->db_stat || + if (table->s->needs_reopen() || + thd->version != refresh_version || table->needs_reopen() || table_def_shutdown_in_progress) { free_cache_entry(table); @@ -8186,13 +8186,13 @@ bool mysql_notify_thread_having_shared_lock(THD *thd, THD *in_use) thd_table= thd_table->next) { /* - Check for TABLE::db_stat is needed since in some places we call + Check for TABLE::needs_reopen() is needed since in some places we call handler::close() for table instance (and set TABLE::db_stat to 0) and do not remove such instances from the THD::open_tables for some time, during which other thread can see those instances (e.g. see partitioning code). */ - if (thd_table->db_stat) + if (!thd_table->needs_reopen()) signalled|= mysql_lock_abort_for_thread(thd, thd_table); } pthread_mutex_unlock(&LOCK_open); diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index 1966c7d8b39..94f5e84fb10 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -820,7 +820,7 @@ void mysql_ha_flush(THD *thd) if (hash_tables->table && (hash_tables->table->mdl_ticket && hash_tables->table->mdl_ticket->has_pending_conflicting_lock() || - hash_tables->table->needs_reopen())) + hash_tables->table->s->needs_reopen())) mysql_ha_close_table(thd, hash_tables); } diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 8fd704c4f71..9a351085b3a 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2657,7 +2657,7 @@ bool Delayed_insert::handle_inserts(void) thd_proc_info(&thd, "insert"); max_rows= delayed_insert_limit; - if (thd.killed || table->needs_reopen()) + if (thd.killed || table->s->needs_reopen()) { thd.killed= THD::KILL_CONNECTION; max_rows= ULONG_MAX; // Do as much as possible diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc index 52657deed83..bcf81a49f56 100644 --- a/sql/sql_partition.cc +++ b/sql/sql_partition.cc @@ -4183,15 +4183,13 @@ bool mysql_unpack_partition(THD *thd, We need to free any memory objects allocated on item_free_list by the parser since we are keeping the old info from the first parser call in CREATE TABLE. - We'll ensure that this object isn't put into table cache also - just to ensure we don't get into strange situations with the - item objects. + + This table object can not be used any more. However, since + this is CREATE TABLE, we know that it will be destroyed by the + caller, and rely on that. */ thd->free_items(); part_info= thd->work_part_info; - tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED, - table->s->db.str, - table->s->table_name.str); *work_part_info_used= true; } table->part_info= part_info; @@ -4484,17 +4482,11 @@ uint prep_alter_part_table(THD *thd, TABLE *table, Alter_info *alter_info, /* We are going to manipulate the partition info on the table object - so we need to ensure that the table instances cached and all other - instances are properly closed. + so we need to ensure that the table instance is removed from the + table cache. */ if (table->part_info) - { - pthread_mutex_lock(&LOCK_open); - tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED, - table->s->db.str, - table->s->table_name.str); - pthread_mutex_unlock(&LOCK_open); - } + table->m_needs_reopen= TRUE; thd->work_part_info= thd->lex->part_info; if (thd->work_part_info && diff --git a/sql/sql_table.cc b/sql/sql_table.cc index e9474d9add6..556c0e65eee 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -7049,6 +7049,10 @@ view_err: new_table->timestamp_field_type= TIMESTAMP_NO_AUTO_SET; new_table->next_number_field=new_table->found_next_number_field; thd_proc_info(thd, "copy to tmp table"); + DBUG_EXECUTE_IF("abort_copy_table", { + my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0)); + goto err_new_table_cleanup; + }); error= copy_data_between_tables(table, new_table, alter_info->create_list, ignore, order_num, order, &copied, &deleted, diff --git a/sql/table.h b/sql/table.h index b44c2e9f3be..4b85d8a64f4 100644 --- a/sql/table.h +++ b/sql/table.h @@ -294,6 +294,8 @@ TABLE_CATEGORY get_table_category(const LEX_STRING *db, struct TABLE_share; +extern ulong refresh_version; + /* This structure is shared between different table objects. There is one instance of table share per one table in the database. @@ -503,6 +505,14 @@ struct TABLE_SHARE return table_map_id; } + + /* + Must all TABLEs be reopened? + */ + inline bool needs_reopen() + { + return version != refresh_version; + } /** Convert unrelated members of TABLE_SHARE to one enum representing its type. @@ -605,8 +615,6 @@ struct TABLE_SHARE }; -extern ulong refresh_version; - /* Information for one open table */ enum index_hint_type { @@ -804,6 +812,7 @@ public: my_bool insert_or_update; /* Can be used by the handler */ my_bool alias_name_used; /* true if table_name is alias */ my_bool get_fields_in_item_tree; /* Signal to fix_field */ + my_bool m_needs_reopen; REGINFO reginfo; /* field connections */ MEM_ROOT mem_root; @@ -853,7 +862,7 @@ public: Is this instance of the table should be reopen? */ inline bool needs_reopen() - { return s->version != refresh_version; } + { return !db_stat || m_needs_reopen; } };