From: Jon Olav Hauglid Date: November 16 2010 10:00am Subject: bzr push into mysql-5.5-runtime branch (jon.hauglid:3185 to 3186) Bug#57663 List-Archive: http://lists.mysql.com/commits/124006 X-Bug: 57663 Message-Id: <201011161001.oAFJXo58021445@acsinet15.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3186 Jon Olav Hauglid 2010-11-16 Bug #57663 Concurrent statement using stored function and DROP DATABASE breaks SBR This pre-requisite patch refactors the code for dropping tables, used by DROP TABLE and DROP DATABASE. The patch moves the code for acquiring metadata locks out of mysql_rm_table_part2() and makes it the responsibility of the caller. This in preparation of changing the DROP DATABASE implementation to acquire all metadata locks before any changes are made. mysql_rm_table_part2() is renamed mysql_rm_table_no_locks() to reflect the change. modified: mysql-test/r/partition_debug_sync.result mysql-test/t/partition_debug_sync.test sql/sql_db.cc sql/sql_table.cc sql/sql_table.h 3185 Jon Olav Hauglid 2010-11-16 [merge] Merge from mysql-5.5-bugteam to mysql-5.5-runtime No conflicts added: mysql-test/t/wl4435_generated.inc modified: CMakeLists.txt cmake/install_macros.cmake cmake/libutils.cmake cmake/mysql_add_executable.cmake cmake/mysql_version.cmake cmake/plugin.cmake libmysql/CMakeLists.txt libservices/CMakeLists.txt man/CMakeLists.txt mysql-test/CMakeLists.txt mysql-test/collections/default.weekly mysql-test/r/information_schema-big.result mysql-test/r/not_embedded_server.result mysql-test/r/partition.result mysql-test/r/ps.result mysql-test/r/variables-big.result mysql-test/suite/parts/inc/partition_auto_increment.inc mysql-test/suite/parts/r/partition_auto_increment_blackhole.result mysql-test/suite/parts/r/partition_auto_increment_innodb.result mysql-test/suite/parts/r/partition_auto_increment_memory.result mysql-test/suite/parts/r/partition_auto_increment_myisam.result mysql-test/t/disabled.def mysql-test/t/not_embedded_server.test mysql-test/t/partition.test mysql-test/t/ps.test mysql-test/t/variables-big.test mysys/mf_iocache.c packaging/WiX/create_msi.cmake.in scripts/CMakeLists.txt sql-bench/CMakeLists.txt sql/CMakeLists.txt sql/ha_partition.cc sql/handler.h sql/item.cc sql/log.cc sql/log.h sql/mysqld.cc sql/mysqld.h sql/sp_rcontext.h sql/sql_class.cc sql/sql_insert.cc sql/sql_prepare.cc sql/sql_update.cc storage/perfschema/pfs_global.h storage/perfschema/pfs_instr.cc storage/perfschema/pfs_instr.h storage/perfschema/pfs_instr_class.cc storage/perfschema/pfs_instr_class.h storage/perfschema/table_events_waits.cc support-files/CMakeLists.txt support-files/mysql.spec.sh tests/mysql_client_test.c === modified file 'mysql-test/r/partition_debug_sync.result' --- a/mysql-test/r/partition_debug_sync.result 2010-07-01 13:53:46 +0000 +++ b/mysql-test/r/partition_debug_sync.result 2010-11-16 10:00:12 +0000 @@ -25,7 +25,7 @@ ALTER TABLE t1 REMOVE PARTITIONING; # Con default SET DEBUG_SYNC= 'now WAIT_FOR removing_partitioning'; SET DEBUG_SYNC= 'mdl_acquire_lock_wait SIGNAL waiting_for_alter'; -SET DEBUG_SYNC= 'rm_table_part2_before_delete_table WAIT_FOR partitioning_removed'; +SET DEBUG_SYNC= 'rm_table_no_locks_before_delete_table WAIT_FOR partitioning_removed'; DROP TABLE IF EXISTS t1; # Con 1 SET SESSION debug= "-d,sleep_before_create_table_no_lock"; @@ -51,12 +51,12 @@ SET DEBUG_SYNC= 'alter_table_before_open SET DEBUG_SYNC= 'alter_table_before_rename_result_table WAIT_FOR delete_done'; ALTER TABLE t2 REMOVE PARTITIONING; # Con default -SET SESSION debug= "+d,sleep_before_part2_delete_table"; +SET SESSION debug= "+d,sleep_before_no_locks_delete_table"; SET DEBUG_SYNC= 'now WAIT_FOR removing_partitions'; -SET DEBUG_SYNC= 'rm_table_part2_before_delete_table SIGNAL waiting_for_alter'; -SET DEBUG_SYNC= 'rm_table_part2_before_binlog SIGNAL delete_done'; +SET DEBUG_SYNC= 'rm_table_no_locks_before_delete_table SIGNAL waiting_for_alter'; +SET DEBUG_SYNC= 'rm_table_no_locks_before_binlog SIGNAL delete_done'; DROP TABLE IF EXISTS t2; -SET SESSION debug= "-d,sleep_before_part2_delete_table"; +SET SESSION debug= "-d,sleep_before_no_locks_delete_table"; # Con 1 ERROR 42S02: Table 'test.t2' doesn't exist SET DEBUG_SYNC= 'RESET'; === modified file 'mysql-test/t/partition_debug_sync.test' --- a/mysql-test/t/partition_debug_sync.test 2010-07-01 13:53:46 +0000 +++ b/mysql-test/t/partition_debug_sync.test 2010-11-16 10:00:12 +0000 @@ -38,7 +38,7 @@ connection default; --echo # Con default SET DEBUG_SYNC= 'now WAIT_FOR removing_partitioning'; SET DEBUG_SYNC= 'mdl_acquire_lock_wait SIGNAL waiting_for_alter'; -SET DEBUG_SYNC= 'rm_table_part2_before_delete_table WAIT_FOR partitioning_removed'; +SET DEBUG_SYNC= 'rm_table_no_locks_before_delete_table WAIT_FOR partitioning_removed'; DROP TABLE IF EXISTS t1; --echo # Con 1 connection con1; @@ -70,12 +70,12 @@ SET DEBUG_SYNC= 'alter_table_before_rena --send ALTER TABLE t2 REMOVE PARTITIONING connection default; --echo # Con default -SET SESSION debug= "+d,sleep_before_part2_delete_table"; +SET SESSION debug= "+d,sleep_before_no_locks_delete_table"; SET DEBUG_SYNC= 'now WAIT_FOR removing_partitions'; -SET DEBUG_SYNC= 'rm_table_part2_before_delete_table SIGNAL waiting_for_alter'; -SET DEBUG_SYNC= 'rm_table_part2_before_binlog SIGNAL delete_done'; +SET DEBUG_SYNC= 'rm_table_no_locks_before_delete_table SIGNAL waiting_for_alter'; +SET DEBUG_SYNC= 'rm_table_no_locks_before_binlog SIGNAL delete_done'; DROP TABLE IF EXISTS t2; -SET SESSION debug= "-d,sleep_before_part2_delete_table"; +SET SESSION debug= "-d,sleep_before_no_locks_delete_table"; --echo # Con 1 connection con1; --error ER_NO_SUCH_TABLE === modified file 'sql/sql_db.cc' --- a/sql/sql_db.cc 2010-11-15 11:32:49 +0000 +++ b/sql/sql_db.cc 2010-11-16 10:00:12 +0000 @@ -28,6 +28,8 @@ #include "sql_acl.h" // SELECT_ACL, DB_ACLS, // acl_get, check_grant_db #include "log_event.h" // Query_log_event +#include "sql_base.h" // lock_table_names, tdc_remove_table +#include "sql_handler.h" // mysql_ha_rm_tables #include #include "sp.h" #include "events.h" @@ -944,6 +946,7 @@ static long mysql_rm_known_files(THD *th ulong found_other_files=0; char filePath[FN_REFLEN]; TABLE_LIST *tot_list=0, **tot_list_next_local, **tot_list_next_global; + TABLE_LIST *table; DBUG_ENTER("mysql_rm_known_files"); DBUG_PRINT("enter",("path: %s", org_path)); @@ -1040,8 +1043,39 @@ static long mysql_rm_known_files(THD *th } } } + + /* + Disable drop of enabled log tables, must be done before name locking. + This check is only needed if we are dropping the "mysql" database. + */ + if ((my_strcasecmp(system_charset_info, MYSQL_SCHEMA_NAME.str, db) == 0)) + { + for (table= tot_list; table; table= table->next_local) + { + if (check_if_log_table(table->db_length, table->db, + table->table_name_length, table->table_name, true)) + { + my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP"); + goto err; + } + } + } + + /* mysql_ha_rm_tables() requires a non-null TABLE_LIST. */ + if (tot_list) + mysql_ha_rm_tables(thd, tot_list); + + if (lock_table_names(thd, tot_list, NULL, thd->variables.lock_wait_timeout, + MYSQL_OPEN_SKIP_TEMPORARY)) + goto err; + + for (table= tot_list; table; table= table->next_local) + tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name, + false); + if (thd->killed || - (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1))) + (tot_list && mysql_rm_table_no_locks(thd, tot_list, true, + false, true, true))) goto err; my_dirend(dirp); === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2010-11-12 11:23:17 +0000 +++ b/sql/sql_table.cc 2010-11-16 10:00:12 +0000 @@ -1849,12 +1849,77 @@ bool mysql_rm_table(THD *thd,TABLE_LIST { bool error; Drop_table_error_handler err_handler; + TABLE_LIST *table; DBUG_ENTER("mysql_rm_table"); + /* Disable drop of enabled log tables, must be done before name locking */ + for (table= tables; table; table= table->next_local) + { + if (check_if_log_table(table->db_length, table->db, + table->table_name_length, table->table_name, true)) + { + my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP"); + DBUG_RETURN(true); + } + } + + mysql_ha_rm_tables(thd, tables); + + if (!drop_temporary) + { + if (!thd->locked_tables_mode) + { + if (lock_table_names(thd, tables, NULL, thd->variables.lock_wait_timeout, + MYSQL_OPEN_SKIP_TEMPORARY)) + DBUG_RETURN(true); + for (table= tables; table; table= table->next_local) + tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name, + false); + } + else + { + for (table= tables; table; table= table->next_local) + if (table->open_type != OT_BASE_ONLY && + find_temporary_table(thd, table)) + { + /* + A temporary table. + + Don't try to find a corresponding MDL lock or assign it + to table->mdl_request.ticket. There can't be metadata + locks for temporary tables: they are local to the session. + + Later in this function we release the MDL lock only if + table->mdl_requeset.ticket is not NULL. Thus here we + ensure that we won't release the metadata lock on the base + table locked with LOCK TABLES as a side effect of temporary + table drop. + */ + DBUG_ASSERT(table->mdl_request.ticket == NULL); + } + else + { + /* + Not a temporary table. + + Since 'tables' list can't contain duplicates (this is ensured + by parser) it is safe to cache pointer to the TABLE instances + in its elements. + */ + table->table= find_table_for_mdl_upgrade(thd->open_tables, table->db, + table->table_name, false); + if (!table->table) + DBUG_RETURN(true); + table->mdl_request.ticket= table->table->mdl_ticket; + } + } + } + /* mark for close and remove all cached entries */ thd->push_internal_handler(&err_handler); - error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0); + error= mysql_rm_table_no_locks(thd, tables, if_exists, drop_temporary, + false, false); thd->pop_internal_handler(); if (error) @@ -1863,39 +1928,38 @@ bool mysql_rm_table(THD *thd,TABLE_LIST DBUG_RETURN(FALSE); } -/* - Execute the drop of a normal or temporary table - SYNOPSIS - mysql_rm_table_part2() - thd Thread handler - tables Tables to drop - if_exists If set, don't give an error if table doesn't exists. - In this case we give an warning of level 'NOTE' - drop_temporary Only drop temporary tables - drop_view Allow to delete VIEW .frm - dont_log_query Don't write query to log files. This will also not - generate warnings if the handler files doesn't exists - - TODO: - When logging to the binary log, we should log - tmp_tables and transactional tables as separate statements if we - are in a transaction; This is needed to get these tables into the - cached binary log that is only written on COMMIT. - - The current code only writes DROP statements that only uses temporary - tables to the cache binary log. This should be ok on most cases, but - not all. - - RETURN - 0 ok - 1 Error - -1 Thread was killed +/** + Execute the drop of a normal or temporary table. + + @param thd Thread handler + @param tables Tables to drop + @param if_exists If set, don't give an error if table doesn't exists. + In this case we give an warning of level 'NOTE' + @param drop_temporary Only drop temporary tables + @param drop_view Allow to delete VIEW .frm + @param dont_log_query Don't write query to log files. This will also not + generate warnings if the handler files doesn't exists + + @retval 0 ok + @retval 1 Error + @retval -1 Thread was killed + + @note This function assumes that metadata locks have already been taken. + It is also assumed that the tables have been removed from TDC. + + @todo When logging to the binary log, we should log + tmp_tables and transactional tables as separate statements if we + are in a transaction; This is needed to get these tables into the + cached binary log that is only written on COMMIT. + The current code only writes DROP statements that only uses temporary + tables to the cache binary log. This should be ok on most cases, but + not all. */ -int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, - bool drop_temporary, bool drop_view, - bool dont_log_query) +int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, + bool drop_temporary, bool drop_view, + bool dont_log_query) { TABLE_LIST *table; char path[FN_REFLEN + 1], *alias= NULL; @@ -1909,7 +1973,7 @@ int mysql_rm_table_part2(THD *thd, TABLE bool non_tmp_table_deleted= 0; String built_query; String built_trans_tmp_query, built_non_trans_tmp_query; - DBUG_ENTER("mysql_rm_table_part2"); + DBUG_ENTER("mysql_rm_table_no_locks"); /* Prepares the drop statements that will be written into the binary @@ -1963,71 +2027,6 @@ int mysql_rm_table_part2(THD *thd, TABLE } } - mysql_ha_rm_tables(thd, tables); - - /* Disable drop of enabled log tables, must be done before name locking */ - for (table= tables; table; table= table->next_local) - { - if (check_if_log_table(table->db_length, table->db, - table->table_name_length, table->table_name, 1)) - { - my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP"); - DBUG_RETURN(1); - } - } - - if (!drop_temporary) - { - if (!thd->locked_tables_mode) - { - if (lock_table_names(thd, tables, NULL, thd->variables.lock_wait_timeout, - MYSQL_OPEN_SKIP_TEMPORARY)) - DBUG_RETURN(1); - for (table= tables; table; table= table->next_local) - { - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name, - FALSE); - } - } - else - { - for (table= tables; table; table= table->next_local) - if (table->open_type != OT_BASE_ONLY && - find_temporary_table(thd, table)) - { - /* - A temporary table. - - Don't try to find a corresponding MDL lock or assign it - to table->mdl_request.ticket. There can't be metadata - locks for temporary tables: they are local to the session. - - Later in this function we release the MDL lock only if - table->mdl_requeset.ticket is not NULL. Thus here we - ensure that we won't release the metadata lock on the base - table locked with LOCK TABLES as a side effect of temporary - table drop. - */ - DBUG_ASSERT(table->mdl_request.ticket == NULL); - } - else - { - /* - Not a temporary table. - - Since 'tables' list can't contain duplicates (this is ensured - by parser) it is safe to cache pointer to the TABLE instances - in its elements. - */ - table->table= find_table_for_mdl_upgrade(thd->open_tables, table->db, - table->table_name, FALSE); - if (!table->table) - DBUG_RETURN(1); - table->mdl_request.ticket= table->table->mdl_ticket; - } - } - } - for (table= tables; table; table= table->next_local) { bool is_trans; @@ -2040,6 +2039,16 @@ int mysql_rm_table_part2(THD *thd, TABLE table->table ? (long) table->table->s : (long) -1)); /* + If we are in locked tables mode and are dropping a temporary table, + the ticket should be NULL to ensure that we don't release a lock + on a base table later. + */ + DBUG_ASSERT(!(thd->locked_tables_mode && + table->open_type != OT_BASE_ONLY && + find_temporary_table(thd, table) && + table->mdl_request.ticket != NULL)); + + /* drop_temporary_table may return one of the following error codes: . 0 - a temporary table was successfully dropped. . 1 - a temporary table was not found. @@ -2114,6 +2123,10 @@ int mysql_rm_table_part2(THD *thd, TABLE table->table= 0; } + /* Check that we have an exclusive lock on the table to be dropped. */ + DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, table->db, + table->table_name, + MDL_EXCLUSIVE)); if (thd->killed) { error= -1; @@ -2156,8 +2169,8 @@ int mysql_rm_table_part2(THD *thd, TABLE built_query.append("`,"); } } - DEBUG_SYNC(thd, "rm_table_part2_before_delete_table"); - DBUG_EXECUTE_IF("sleep_before_part2_delete_table", + DEBUG_SYNC(thd, "rm_table_no_locks_before_delete_table"); + DBUG_EXECUTE_IF("sleep_before_no_locks_delete_table", my_sleep(100000);); error= 0; if (drop_temporary || @@ -2244,7 +2257,7 @@ int mysql_rm_table_part2(THD *thd, TABLE ER(ER_BAD_TABLE_ERROR), MYF(0), table->table_name);); } - DEBUG_SYNC(thd, "rm_table_part2_before_binlog"); + DEBUG_SYNC(thd, "rm_table_no_locks_before_binlog"); thd->thread_specific_used|= (trans_tmp_table_deleted || non_trans_tmp_table_deleted); error= 0; === modified file 'sql/sql_table.h' --- a/sql/sql_table.h 2010-08-20 17:15:48 +0000 +++ b/sql/sql_table.h 2010-11-16 10:00:12 +0000 @@ -174,8 +174,9 @@ bool mysql_checksum_table(THD* thd, TABL HA_CHECK_OPT* check_opt); bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, my_bool drop_temporary); -int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, - bool drop_temporary, bool drop_view, bool log_query); +int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, + bool drop_temporary, bool drop_view, + bool log_query); bool quick_rm_table(handlerton *base,const char *db, const char *table_name, uint flags); void close_cached_table(THD *thd, TABLE *table); No bundle (reason: useless for push emails).