From b3713993442461c92a51d642de79befe7d93401c Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Tue, 24 Aug 2010 11:18:48 +0200 Subject: [PATCH 01/24] Fixed tree name. --- .bzr-mysql/default.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bzr-mysql/default.conf b/.bzr-mysql/default.conf index 7d0379c08a0..c3fcbcf6acd 100644 --- a/.bzr-mysql/default.conf +++ b/.bzr-mysql/default.conf @@ -1,4 +1,4 @@ [MYSQL] post_commit_to = "commits@lists.mysql.com" post_push_to = "commits@lists.mysql.com" -tree_name = "mysql-5.5-bugfixing" +tree_name = "mysql-5.5-runtime" From bbd7277fbef57a3142281b4430776f5f8aede3e8 Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Thu, 26 Aug 2010 12:01:43 +0200 Subject: [PATCH 02/24] Bug #44171 KILL ALTER EVENT can crash the server This assert could be triggered if ALTER EVENT failed to load the event after altering it. Failing to load the event could for example happen because of KILL QUERY. The assert tested that the result of a failed load_named_event() was OP_LOAD_ERROR. However since load_named_event() returns bool, this assert did not make any sense. This patch therefore removes the assert, fixing the problem. The patch also removes enum_events_error_code since it was unused. No test case added. The bug fix is trivial and this bug was easily detected by RQG tests. Further, adding a MTR test case for this bug would require adding sync points to make the test case repeatable. --- sql/events.cc | 3 --- sql/events.h | 13 ------------- 2 files changed, 16 deletions(-) diff --git a/sql/events.cc b/sql/events.cc index 10a7535425f..e7e47801586 100644 --- a/sql/events.cc +++ b/sql/events.cc @@ -485,10 +485,7 @@ Events::update_event(THD *thd, Event_parse_data *parse_data, ret= TRUE; // OOM else if ((ret= db_repository->load_named_event(thd, dbname, name, new_element))) - { - DBUG_ASSERT(ret == OP_LOAD_ERROR); delete new_element; - } else { /* diff --git a/sql/events.h b/sql/events.h index 1482e736d29..f3ebc6da4ad 100644 --- a/sql/events.h +++ b/sql/events.h @@ -44,19 +44,6 @@ class THD; typedef class Item COND; typedef struct charset_info_st CHARSET_INFO; -/* Return codes */ -enum enum_events_error_code -{ - OP_OK= 0, - OP_NOT_RUNNING, - OP_CANT_KILL, - OP_CANT_INIT, - OP_DISABLED_EVENT, - OP_LOAD_ERROR, - OP_ALREADY_EXISTS -}; - - int sortcmp_lex_string(LEX_STRING s, LEX_STRING t, CHARSET_INFO *cs); From 0142c14a6b3fd551e5b8e8dc5fd61e215910c72f Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Fri, 27 Aug 2010 12:39:01 +0400 Subject: [PATCH 03/24] Bug#27480 (Extend CREATE TEMPORARY TABLES privilege to allow temp table operations) -- prerequisite patch #1. Move a piece of code that initialiazes TABLE instance after it was successfully opened into a separate function. This function will be reused in the following patches. --- sql/sql_base.cc | 36 +++---------------------------- sql/table.cc | 57 +++++++++++++++++++++++++++++++++++++++++++++++++ sql/table.h | 1 + 3 files changed, 61 insertions(+), 33 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 01ab0b6dec5..c14f2f41809 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -3023,41 +3023,11 @@ retry_share: table->reginfo.lock_type=TL_READ; /* Assume read */ reset: - DBUG_ASSERT(table->s->ref_count > 0 || table->s->tmp_table != NO_TMP_TABLE); - - if (thd->lex->need_correct_ident()) - table->alias_name_used= my_strcasecmp(table_alias_charset, - table->s->table_name.str, alias); - /* Fix alias if table name changes */ - if (strcmp(table->alias, alias)) - { - uint length=(uint) strlen(alias)+1; - table->alias= (char*) my_realloc((char*) table->alias, length, - MYF(MY_WME)); - memcpy((char*) table->alias, alias, length); - } - table->tablenr=thd->current_tablenr++; - table->used_fields=0; - table->const_table=0; - table->null_row= table->maybe_null= 0; - table->force_index= table->force_index_order= table->force_index_group= 0; - table->status=STATUS_NO_RECORD; - table->insert_values= 0; - table->fulltext_searched= 0; - table->file->ft_handler= 0; - table->reginfo.impossible_range= 0; - /* Catch wrong handling of the auto_increment_field_not_null. */ - DBUG_ASSERT(!table->auto_increment_field_not_null); - table->auto_increment_field_not_null= FALSE; - if (table->timestamp_field) - table->timestamp_field_type= table->timestamp_field->get_auto_set_type(); - table->pos_in_table_list= table_list; table_list->updatable= 1; // It is not derived table nor non-updatable VIEW - table->clear_column_bitmaps(); table_list->table= table; - DBUG_ASSERT(table->key_read == 0); - /* Tables may be reused in a sub statement. */ - DBUG_ASSERT(! table->file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN)); + + table->init(thd, table_list); + DBUG_RETURN(FALSE); err_lock: diff --git a/sql/table.cc b/sql/table.cc index f08a0aa84ca..5cdc0ce9042 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3223,6 +3223,63 @@ bool TABLE_SHARE::wait_for_old_version(THD *thd, struct timespec *abstime, } +/** + Initialize TABLE instance and prepare it to be used further. + Set the 'alias' attribute from the specified TABLE_LIST element. + Remember the TABLE_LIST element in the 'pos_in_table_list' member. + + @param thd Thread context. + @param tl TABLE_LIST element. +*/ + +void TABLE::init(THD *thd, TABLE_LIST *tl) +{ + DBUG_ASSERT(s->ref_count > 0 || s->tmp_table != NO_TMP_TABLE); + + if (thd->lex->need_correct_ident()) + alias_name_used= my_strcasecmp(table_alias_charset, + s->table_name.str, + tl->alias); + /* Fix alias if table name changes. */ + if (strcmp(alias, tl->alias)) + { + uint length= (uint) strlen(tl->alias)+1; + alias= (char*) my_realloc((char*) alias, length, MYF(MY_WME)); + memcpy((char*) alias, tl->alias, length); + } + + tablenr= thd->current_tablenr++; + used_fields= 0; + const_table= 0; + null_row= 0; + maybe_null= 0; + force_index= 0; + force_index_order= 0; + force_index_group= 0; + status= STATUS_NO_RECORD; + insert_values= 0; + fulltext_searched= 0; + file->ft_handler= 0; + reginfo.impossible_range= 0; + + /* Catch wrong handling of the auto_increment_field_not_null. */ + DBUG_ASSERT(!auto_increment_field_not_null); + auto_increment_field_not_null= FALSE; + + if (timestamp_field) + timestamp_field_type= timestamp_field->get_auto_set_type(); + + pos_in_table_list= tl; + + clear_column_bitmaps(); + + DBUG_ASSERT(key_read == 0); + + /* Tables may be reused in a sub statement. */ + DBUG_ASSERT(!file->extra(HA_EXTRA_IS_ATTACHED_CHILDREN)); +} + + /* Create Item_field for each column in the table. diff --git a/sql/table.h b/sql/table.h index bce1124fbb3..aa177c99856 100644 --- a/sql/table.h +++ b/sql/table.h @@ -1097,6 +1097,7 @@ public: #endif MDL_ticket *mdl_ticket; + void init(THD *thd, TABLE_LIST *tl); bool fill_item_list(List *item_list) const; void reset_item_list(List *item_list) const; void clear_column_bitmaps(void); From 26bc3b34d5c50599bd6f9856b19c9c4e4c58d819 Mon Sep 17 00:00:00 2001 From: Dmitry Lenev Date: Tue, 31 Aug 2010 13:04:19 +0400 Subject: [PATCH 04/24] Bug #56137 "Assertion `thd->lock == 0' failed on upgrading from 5.1.50 to 5.5.6". Debug builds of the server aborted due to an assertion failure when DROP DATABASE statement was run on an installation which had outdated or corrupt mysql.proc table. Particularly this affected the mysql_upgrade tool which is run as part of 5.1 to 5.5 upgrade. The problem was that sp_drop_db_routines(), which was invoked during dropping of the database, could have returned without closing and unlocking mysql.proc table in cases when this table was not up-to-date with the current server. As a result further attempt to open and lock the mysql.event table, which was necessary to complete dropping of the database, ended up with an assert. This patch solves this problem by ensuring that sp_drop_db_routines() always closes mysql.proc table and releases metadata locks on it. This is achieved by changing open_proc_table_for_update() function to close tables and release metadata locks acquired by it in case of failure. This step also makes behavior of the latter function consistent with behavior of open_proc_table_for_read()/ open_and_lock_tables(). Test case for this bug was added to sp-destruct.test. --- mysql-test/r/sp-destruct.result | 16 ++++++++++++++++ mysql-test/t/sp-destruct.test | 30 ++++++++++++++++++++++++++++++ sql/sp.cc | 4 ++++ 3 files changed, 50 insertions(+) diff --git a/mysql-test/r/sp-destruct.result b/mysql-test/r/sp-destruct.result index a4933ee9800..32d0e39c5af 100644 --- a/mysql-test/r/sp-destruct.result +++ b/mysql-test/r/sp-destruct.result @@ -134,3 +134,19 @@ Warning 1405 Failed to revoke all privileges to dropped routine # Restore the procs_priv table RENAME TABLE procs_priv_backup TO mysql.procs_priv; FLUSH TABLE mysql.procs_priv; +# +# Bug #56137 "Assertion `thd->lock == 0' failed on upgrading from +# 5.1.50 to 5.5.6". +# +drop database if exists mysqltest; +# Backup mysql.proc. +flush table mysql.proc; +create database mysqltest; +# Corrupt mysql.proc to make it unusable by current version of server. +alter table mysql.proc drop column type; +# The below statement should not cause assertion failure. +drop database mysqltest; +Warnings: +Error 1547 Column count of mysql.proc is wrong. Expected 20, found 19. The table is probably corrupted +# Restore mysql.proc. +drop table mysql.proc; diff --git a/mysql-test/t/sp-destruct.test b/mysql-test/t/sp-destruct.test index b7090c01f1e..a5c287e44a8 100644 --- a/mysql-test/t/sp-destruct.test +++ b/mysql-test/t/sp-destruct.test @@ -222,3 +222,33 @@ SHOW WARNINGS; --echo # Restore the procs_priv table RENAME TABLE procs_priv_backup TO mysql.procs_priv; FLUSH TABLE mysql.procs_priv; + + +--echo # +--echo # Bug #56137 "Assertion `thd->lock == 0' failed on upgrading from +--echo # 5.1.50 to 5.5.6". +--echo # +--disable_warnings +drop database if exists mysqltest; +--enable_warnings +--echo # Backup mysql.proc. +flush table mysql.proc; +let $MYSQLD_DATADIR= `select @@datadir`; +--copy_file $MYSQLD_DATADIR/mysql/proc.frm $MYSQLTEST_VARDIR/tmp/proc.frm +--copy_file $MYSQLD_DATADIR/mysql/proc.MYD $MYSQLTEST_VARDIR/tmp/proc.MYD +--copy_file $MYSQLD_DATADIR/mysql/proc.MYI $MYSQLTEST_VARDIR/tmp/proc.MYI + +create database mysqltest; +--echo # Corrupt mysql.proc to make it unusable by current version of server. +alter table mysql.proc drop column type; +--echo # The below statement should not cause assertion failure. +drop database mysqltest; + +--echo # Restore mysql.proc. +drop table mysql.proc; +--copy_file $MYSQLTEST_VARDIR/tmp/proc.frm $MYSQLD_DATADIR/mysql/proc.frm +--copy_file $MYSQLTEST_VARDIR/tmp/proc.MYD $MYSQLD_DATADIR/mysql/proc.MYD +--copy_file $MYSQLTEST_VARDIR/tmp/proc.MYI $MYSQLD_DATADIR/mysql/proc.MYI +--remove_file $MYSQLTEST_VARDIR/tmp/proc.frm +--remove_file $MYSQLTEST_VARDIR/tmp/proc.MYD +--remove_file $MYSQLTEST_VARDIR/tmp/proc.MYI diff --git a/sql/sp.cc b/sql/sp.cc index 0265ef45a2a..8821dc9365d 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -440,6 +440,7 @@ static TABLE *open_proc_table_for_update(THD *thd) { TABLE_LIST table_list; TABLE *table; + MDL_ticket *mdl_savepoint= thd->mdl_context.mdl_savepoint(); DBUG_ENTER("open_proc_table_for_update"); table_list.init_one_table("mysql", 5, "proc", 4, "proc", TL_WRITE); @@ -450,6 +451,9 @@ static TABLE *open_proc_table_for_update(THD *thd) if (!proc_table_intact.check(table, &proc_table_def)) DBUG_RETURN(table); + close_thread_tables(thd); + thd->mdl_context.rollback_to_savepoint(mdl_savepoint); + DBUG_RETURN(NULL); } From a92ca267e0c3d73dbe43f339721312044e3eec47 Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Tue, 31 Aug 2010 13:49:48 +0400 Subject: [PATCH 05/24] Fix TABLE::init() comment. --- sql/table.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sql/table.cc b/sql/table.cc index 5cdc0ce9042..e84991912a4 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3224,9 +3224,11 @@ bool TABLE_SHARE::wait_for_old_version(THD *thd, struct timespec *abstime, /** - Initialize TABLE instance and prepare it to be used further. - Set the 'alias' attribute from the specified TABLE_LIST element. - Remember the TABLE_LIST element in the 'pos_in_table_list' member. + Initialize TABLE instance (newly created, or coming either from table + cache or THD::temporary_tables list) and prepare it for further use + during statement execution. Set the 'alias' attribute from the specified + TABLE_LIST element. Remember the TABLE_LIST element in the + TABLE::pos_in_table_list member. @param thd Thread context. @param tl TABLE_LIST element. From 2e462c8f99c98bf525fc1105e9e6ca52a6b5c221 Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Tue, 31 Aug 2010 13:52:56 +0400 Subject: [PATCH 06/24] Bug#27480 (Extend CREATE TEMPORARY TABLES privilege to allow temp table operations) -- prerequisite patch #2. Introduce a new form of find_temporary_table() function: find_temporary_table() by a table key. It will be used in further patches. Replace find_temporary_table(table_list->db, table_list->name) by more appropiate find_temporary_table(table_list) across the codebase. --- sql/sql_base.cc | 68 ++++++++++++++++++++++++++++++---------------- sql/sql_base.h | 7 +++-- sql/sql_parse.cc | 3 +- sql/sql_table.cc | 2 +- sql/sql_trigger.cc | 2 +- 5 files changed, 53 insertions(+), 29 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index c14f2f41809..feb63355353 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -247,7 +247,8 @@ static void check_unused(void) Length of key */ -uint create_table_def_key(THD *thd, char *key, TABLE_LIST *table_list, +uint create_table_def_key(THD *thd, char *key, + const TABLE_LIST *table_list, bool tmp_table) { uint key_length= (uint) (strmov(strmov(key, table_list->db)+1, @@ -1987,39 +1988,60 @@ void update_non_unique_table_error(TABLE_LIST *update, } +/** + Find temporary table specified by database and table names in the + THD::temporary_tables list. + + @return TABLE instance if a temporary table has been found; NULL otherwise. +*/ + TABLE *find_temporary_table(THD *thd, const char *db, const char *table_name) { - TABLE_LIST table_list; + TABLE_LIST tl; - table_list.db= (char*) db; - table_list.table_name= (char*) table_name; - return find_temporary_table(thd, &table_list); + tl.db= (char*) db; + tl.table_name= (char*) table_name; + + return find_temporary_table(thd, &tl); } -TABLE *find_temporary_table(THD *thd, TABLE_LIST *table_list) -{ - char key[MAX_DBKEY_LENGTH]; - uint key_length; - TABLE *table; - DBUG_ENTER("find_temporary_table"); - DBUG_PRINT("enter", ("table: '%s'.'%s'", - table_list->db, table_list->table_name)); +/** + Find a temporary table specified by TABLE_LIST instance in the + THD::temporary_tables list. - key_length= create_table_def_key(thd, key, table_list, 1); - for (table=thd->temporary_tables ; table ; table= table->next) + @return TABLE instance if a temporary table has been found; NULL otherwise. +*/ + +TABLE *find_temporary_table(THD *thd, const TABLE_LIST *tl) +{ + char key[MAX_DBKEY_LENGTH]; + uint key_length= create_table_def_key(thd, key, tl, 1); + + return find_temporary_table(thd, key, key_length); +} + + +/** + Find a temporary table specified by a key in the THD::temporary_tables list. + + @return TABLE instance if a temporary table has been found; NULL otherwise. +*/ + +TABLE *find_temporary_table(THD *thd, + const char *table_key, + uint table_key_length) +{ + for (TABLE *table= thd->temporary_tables; table; table= table->next) { - if (table->s->table_cache_key.length == key_length && - !memcmp(table->s->table_cache_key.str, key, key_length)) + if (table->s->table_cache_key.length == table_key_length && + !memcmp(table->s->table_cache_key.str, table_key, table_key_length)) { - DBUG_PRINT("info", - ("Found table. server_id: %u pseudo_thread_id: %lu", - (uint) thd->server_id, - (ulong) thd->variables.pseudo_thread_id)); - DBUG_RETURN(table); + return table; } } - DBUG_RETURN(0); // Not a temporary table + + return NULL; } diff --git a/sql/sql_base.h b/sql/sql_base.h index ff935c3fc09..1a1f2d957f3 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -79,7 +79,8 @@ void table_def_start_shutdown(void); void assign_new_table_id(TABLE_SHARE *share); uint cached_open_tables(void); uint cached_table_definitions(void); -uint create_table_def_key(THD *thd, char *key, TABLE_LIST *table_list, +uint create_table_def_key(THD *thd, char *key, + const TABLE_LIST *table_list, bool tmp_table); TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list, char *key, uint key_length, uint db_flags, int *error, @@ -159,7 +160,9 @@ TABLE_LIST *find_table_in_list(TABLE_LIST *table, const char *db_name, const char *table_name); TABLE *find_temporary_table(THD *thd, const char *db, const char *table_name); -TABLE *find_temporary_table(THD *thd, TABLE_LIST *table_list); +TABLE *find_temporary_table(THD *thd, const TABLE_LIST *tl); +TABLE *find_temporary_table(THD *thd, const char *table_key, + uint table_key_length); void close_thread_tables(THD *thd); bool fill_record_n_invoke_before_triggers(THD *thd, List &fields, List &values, diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index b78815f0e52..a0b9959e626 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -177,8 +177,7 @@ static bool some_non_temp_table_to_be_updated(THD *thd, TABLE_LIST *tables) for (TABLE_LIST *table= tables; table; table= table->next_global) { DBUG_ASSERT(table->db && table->table_name); - if (table->updating && - !find_temporary_table(thd, table->db, table->table_name)) + if (table->updating && !find_temporary_table(thd, table)) return 1; } return 0; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 3f0a0326c84..06db813481d 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2008,7 +2008,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, { for (table= tables; table; table= table->next_local) if (table->open_type != OT_BASE_ONLY && - find_temporary_table(thd, table->db, table->table_name)) + find_temporary_table(thd, table)) { /* A temporary table. diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index b81461e8371..c1405015365 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -458,7 +458,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) DBUG_ASSERT(tables->next_global == 0); /* We do not allow creation of triggers on temporary tables. */ - if (create && find_temporary_table(thd, tables->db, tables->table_name)) + if (create && find_temporary_table(thd, tables)) { my_error(ER_TRG_ON_VIEW_OR_TEMP_TABLE, MYF(0), tables->alias); goto end; From 1f1eba3c68cc016c9cf832b7076dcb9f04b0dacf Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Tue, 31 Aug 2010 13:53:49 +0400 Subject: [PATCH 07/24] Remove unused enum (enum open_table_mode). It was added by mistake during backport from 6.0. --- sql/sql_base.h | 1 - sql/table.h | 12 ------------ 2 files changed, 13 deletions(-) diff --git a/sql/sql_base.h b/sql/sql_base.h index 1a1f2d957f3..56776bde1cc 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -17,7 +17,6 @@ #define SQL_BASE_INCLUDED #include "unireg.h" // REQUIRED: for other includes -#include "table.h" /* open_table_mode */ #include "sql_trigger.h" /* trg_event_type */ #include "sql_class.h" /* enum_mark_columns */ #include "mysqld.h" /* key_map */ diff --git a/sql/table.h b/sql/table.h index aa177c99856..ea93b321b5a 100644 --- a/sql/table.h +++ b/sql/table.h @@ -83,18 +83,6 @@ enum enum_table_ref_type }; -/** - Opening modes for open_temporary_table and open_table_from_share -*/ - -enum open_table_mode -{ - OTM_OPEN= 0, - OTM_CREATE= 1, - OTM_ALTER= 2 -}; - - /*************************************************************************/ /** From 6b9371bd458ba5ae204e85d590f07b1c681cb286 Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Tue, 31 Aug 2010 13:55:32 +0400 Subject: [PATCH 08/24] Polish check_grant(): name TABLE_LIST instance "tl", not "table". This allows to avoid mixing it up with pointer to TABLE object which will be introduced to this function in one of upcoming patches. --- sql/sql_acl.cc | 70 ++++++++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 19373507955..41e4ad7a522 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -4035,7 +4035,8 @@ end: bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables, bool any_combination_will_do, uint number, bool no_errors) { - TABLE_LIST *table, *first_not_own_table= thd->lex->first_not_own_table(); + TABLE_LIST *tl; + TABLE_LIST *first_not_own_table= thd->lex->first_not_own_table(); Security_context *sctx= thd->security_ctx; uint i; ulong orig_want_access= want_access; @@ -4052,34 +4053,32 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables, the given table list refers to the list for prelocking (contains tables of other queries). For simple queries first_not_own_table is 0. */ - for (i= 0, table= tables; - i < number && table != first_not_own_table; - table= table->next_global, i++) + for (i= 0, tl= tables; + i < number && tl != first_not_own_table; + tl= tl->next_global, i++) { /* Save a copy of the privileges without the SHOW_VIEW_ACL attribute. It will be checked during making view. */ - table->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL); + tl->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL); } mysql_rwlock_rdlock(&LOCK_grant); - for (table= tables; - table && number-- && table != first_not_own_table; - table= table->next_global) + for (tl= tables; + tl && number-- && tl != first_not_own_table; + tl= tl->next_global) { - GRANT_TABLE *grant_table; - sctx = test(table->security_ctx) ? - table->security_ctx : thd->security_ctx; + sctx = test(tl->security_ctx) ? tl->security_ctx : thd->security_ctx; - const ACL_internal_table_access *access; - access= get_cached_table_access(&table->grant.m_internal, - table->get_db_name(), - table->get_table_name()); + const ACL_internal_table_access *access= + get_cached_table_access(&tl->grant.m_internal, + tl->get_db_name(), + tl->get_table_name()); if (access) { - switch(access->check(orig_want_access, &table->grant.privilege)) + switch(access->check(orig_want_access, &tl->grant.privilege)) { case ACL_INTERNAL_ACCESS_GRANTED: /* @@ -4103,29 +4102,33 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables, if (!want_access) continue; // ok - if (!(~table->grant.privilege & want_access) || - table->is_anonymous_derived_table() || table->schema_table) + if (!(~tl->grant.privilege & want_access) || + tl->is_anonymous_derived_table() || tl->schema_table) { /* - It is subquery in the FROM clause. VIEW set table->derived after + It is subquery in the FROM clause. VIEW set tl->derived after table opening, but this function always called before table opening. */ - if (!table->referencing_view) + if (!tl->referencing_view) { /* If it's a temporary table created for a subquery in the FROM clause, or an INFORMATION_SCHEMA table, drop the request for a privilege. */ - table->grant.want_privilege= 0; + tl->grant.want_privilege= 0; } continue; } - if (!(grant_table= table_hash_search(sctx->host, sctx->ip, - table->get_db_name(), sctx->priv_user, - table->get_table_name(), FALSE))) + GRANT_TABLE *grant_table= table_hash_search(sctx->host, sctx->ip, + tl->get_db_name(), + sctx->priv_user, + tl->get_table_name(), + FALSE); + + if (!grant_table) { - want_access &= ~table->grant.privilege; + want_access &= ~tl->grant.privilege; goto err; // No grants } @@ -4136,18 +4139,17 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST *tables, if (any_combination_will_do) continue; - table->grant.grant_table=grant_table; // Remember for column test - table->grant.version=grant_version; - table->grant.privilege|= grant_table->privs; - table->grant.want_privilege= ((want_access & COL_ACLS) - & ~table->grant.privilege); + tl->grant.grant_table= grant_table; // Remember for column test + tl->grant.version= grant_version; + tl->grant.privilege|= grant_table->privs; + tl->grant.want_privilege= ((want_access & COL_ACLS) & ~tl->grant.privilege); - if (!(~table->grant.privilege & want_access)) + if (!(~tl->grant.privilege & want_access)) continue; - if (want_access & ~(grant_table->cols | table->grant.privilege)) + if (want_access & ~(grant_table->cols | tl->grant.privilege)) { - want_access &= ~(grant_table->cols | table->grant.privilege); + want_access &= ~(grant_table->cols | tl->grant.privilege); goto err; // impossible } } @@ -4164,7 +4166,7 @@ err: command, sctx->priv_user, sctx->host_or_ip, - table ? table->get_table_name() : "unknown"); + tl ? tl->get_table_name() : "unknown"); } DBUG_RETURN(TRUE); } From 53e566a42a0203bf6b1054252da4dfea2b9442b8 Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Tue, 31 Aug 2010 13:59:51 +0400 Subject: [PATCH 09/24] Remove check_merge_table_access(). check_merge_table_access() used to do two things: - set proper database for every merge table child; - check SELECT | UPDATE | DELETE for merge table children. Setting database is not needed anymore, since it's done on the parsing stage. Thus, check_merge_table_access() can be removed; needed privileges can be checked using check_table_access(). --- sql/sql_alter.cc | 13 +++++++----- sql/sql_parse.cc | 51 +++++++++--------------------------------------- sql/sql_parse.h | 3 --- 3 files changed, 17 insertions(+), 50 deletions(-) diff --git a/sql/sql_alter.cc b/sql/sql_alter.cc index 046e09b20a3..5af01523aa7 100644 --- a/sql/sql_alter.cc +++ b/sql/sql_alter.cc @@ -13,8 +13,7 @@ along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#include "sql_parse.h" // check_access, - // check_merge_table_access +#include "sql_parse.h" // check_access #include "sql_table.h" // mysql_alter_table, // mysql_exchange_partition #include "sql_alter.h" @@ -60,11 +59,15 @@ bool Alter_table_statement::execute(THD *thd) check_access(thd, INSERT_ACL | CREATE_ACL, select_lex->db, &priv, NULL, /* Don't use first_tab->grant with sel_lex->db */ - 0, 0) || - check_merge_table_access(thd, first_table->db, - create_info.merge_list.first)) + 0, 0)) DBUG_RETURN(TRUE); /* purecov: inspected */ + /* If it is a merge table, check privileges for merge children. */ + if (create_info.merge_list.first && + check_table_access(thd, SELECT_ACL | UPDATE_ACL | DELETE_ACL, + create_info.merge_list.first, FALSE, UINT_MAX, FALSE)) + DBUG_RETURN(TRUE); + if (check_grant(thd, priv_needed, first_table, FALSE, UINT_MAX, FALSE)) DBUG_RETURN(TRUE); /* purecov: inspected */ diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index a0b9959e626..aa9d29f3d07 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -642,45 +642,6 @@ end: } -/** - @brief Check access privs for a MERGE table and fix children lock types. - - @param[in] thd thread handle - @param[in] db database name - @param[in,out] table_list list of child tables (merge_list) - lock_type and optionally db set per table - - @return status - @retval 0 OK - @retval != 0 Error - - @detail - This function is used for write access to MERGE tables only - (CREATE TABLE, ALTER TABLE ... UNION=(...)). Set TL_WRITE for - every child. Set 'db' for every child if not present. -*/ -#ifndef NO_EMBEDDED_ACCESS_CHECKS -bool check_merge_table_access(THD *thd, char *db, TABLE_LIST *table_list) -{ - int error= 0; - - if (table_list) - { - /* Check that all tables use the current database */ - TABLE_LIST *tlist; - - for (tlist= table_list; tlist; tlist= tlist->next_local) - { - if (!tlist->db || !tlist->db[0]) - tlist->db= db; /* purecov: inspected */ - } - error= check_table_access(thd, SELECT_ACL | UPDATE_ACL | DELETE_ACL, - table_list, FALSE, UINT_MAX, FALSE); - } - return error; -} -#endif - /* This works because items are allocated with sql_alloc() */ void free_items(Item *item) @@ -6963,10 +6924,16 @@ bool create_table_precheck(THD *thd, TABLE_LIST *tables, if (check_access(thd, want_priv, create_table->db, &create_table->grant.privilege, &create_table->grant.m_internal, - 0, 0) || - check_merge_table_access(thd, create_table->db, - lex->create_info.merge_list.first)) + 0, 0)) goto err; + + /* If it is a merge table, check privileges for merge children. */ + if (lex->create_info.merge_list.first && + check_table_access(thd, SELECT_ACL | UPDATE_ACL | DELETE_ACL, + lex->create_info.merge_list.first, + FALSE, UINT_MAX, FALSE)) + goto err; + if (want_priv != CREATE_TMP_ACL && check_grant(thd, want_priv, create_table, FALSE, 1, FALSE)) goto err; diff --git a/sql/sql_parse.h b/sql/sql_parse.h index ad620544d29..c9b0f981d19 100644 --- a/sql/sql_parse.h +++ b/sql/sql_parse.h @@ -153,7 +153,6 @@ bool check_single_table_access(THD *thd, ulong privilege, bool check_routine_access(THD *thd,ulong want_access,char *db,char *name, bool is_proc, bool no_errors); bool check_some_access(THD *thd, ulong want_access, TABLE_LIST *table); -bool check_merge_table_access(THD *thd, char *db, TABLE_LIST *table_list); bool check_some_routine_access(THD *thd, const char *db, const char *name, bool is_proc); bool check_access(THD *thd, ulong want_access, const char *db, ulong *save_priv, GRANT_INTERNAL_INFO *grant_internal_info, @@ -176,8 +175,6 @@ inline bool check_some_access(THD *thd, ulong want_access, TABLE_LIST *table) table->grant.privilege= want_access; return false; } -inline bool check_merge_table_access(THD *thd, char *db, TABLE_LIST *table_list) -{ return false; } inline bool check_some_routine_access(THD *thd, const char *db, const char *name, bool is_proc) { return false; } From d2159e37f2931ac87baf956d21910a5edea1fcce Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Tue, 31 Aug 2010 14:03:36 +0400 Subject: [PATCH 10/24] Bug#27480 (Extend CREATE TEMPORARY TABLES privilege to allow temp table operations) -- prerequisite patch #3. Rename open_temporary_table() to open_table_uncached(). open_temporary_table() will be introduced in following patches to open temporary tables for a statement. --- sql/sql_base.cc | 40 +++++++++++++++++++++------------------- sql/sql_base.h | 5 +++-- sql/sql_table.cc | 19 ++++++++++++------- sql/sql_truncate.cc | 4 ++-- 4 files changed, 38 insertions(+), 30 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index feb63355353..8cd0ce4432a 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -5696,35 +5696,37 @@ void close_tables_for_reopen(THD *thd, TABLE_LIST **tables, } -/* - Open a single table without table caching and don't set it in open_list +/** + Open a single table without table caching and don't add it to + THD::open_tables. Depending on the 'add_to_temporary_tables_list' value, + the opened TABLE instance will be addded to THD::temporary_tables list. - SYNPOSIS - open_temporary_table() - thd Thread object - path Path (without .frm) - db database - table_name Table name - link_in_list 1 if table should be linked into thd->temporary_tables + @param thd Thread context. + @param path Path (without .frm) + @param db Database name. + @param table_name Table name. + @param add_to_temporary_tables_list Specifies if the opened TABLE + instance should be linked into + THD::temporary_tables list. - NOTES: - Used by alter_table to open a temporary table and when creating - a temporary table with CREATE TEMPORARY ... + @note This function is used: + - by alter_table() to open a temporary table; + - when creating a temporary table with CREATE TEMPORARY TABLE. - RETURN - 0 Error - # TABLE object + @return TABLE instance for opened table. + @retval NULL on error. */ -TABLE *open_temporary_table(THD *thd, const char *path, const char *db, - const char *table_name, bool link_in_list) +TABLE *open_table_uncached(THD *thd, const char *path, const char *db, + const char *table_name, + bool add_to_temporary_tables_list) { TABLE *tmp_table; TABLE_SHARE *share; char cache_key[MAX_DBKEY_LENGTH], *saved_cache_key, *tmp_path; uint key_length; TABLE_LIST table_list; - DBUG_ENTER("open_temporary_table"); + DBUG_ENTER("open_table_uncached"); DBUG_PRINT("enter", ("table: '%s'.'%s' path: '%s' server_id: %u " "pseudo_thread_id: %lu", @@ -5767,7 +5769,7 @@ TABLE *open_temporary_table(THD *thd, const char *path, const char *db, share->tmp_table= (tmp_table->file->has_transactions() ? TRANSACTIONAL_TMP_TABLE : NON_TRANSACTIONAL_TMP_TABLE); - if (link_in_list) + if (add_to_temporary_tables_list) { /* growing temp list at the head */ tmp_table->next= thd->temporary_tables; diff --git a/sql/sql_base.h b/sql/sql_base.h index 56776bde1cc..a21850355b4 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -141,8 +141,9 @@ bool open_new_frm(THD *thd, TABLE_SHARE *share, const char *alias, bool get_key_map_from_key_list(key_map *map, TABLE *table, List *index_list); -TABLE *open_temporary_table(THD *thd, const char *path, const char *db, - const char *table_name, bool link_in_list); +TABLE *open_table_uncached(THD *thd, const char *path, const char *db, + const char *table_name, + bool add_to_temporary_tables_list); TABLE *find_locked_table(TABLE *list, const char *db, const char *table_name); TABLE *find_write_locked_table(TABLE *list, const char *db, const char *table_name); diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 06db813481d..9d8f59189d0 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -22,7 +22,7 @@ #include "sql_rename.h" // do_rename #include "sql_parse.h" // test_if_data_home_dir #include "sql_cache.h" // query_cache_* -#include "sql_base.h" // open_temporary_table, lock_table_names +#include "sql_base.h" // open_table_uncached, lock_table_names #include "lock.h" // wait_if_global_read_lock // start_waiting_global_read_lock, // mysql_unlock_tables @@ -4224,9 +4224,14 @@ bool mysql_create_table_no_lock(THD *thd, if (create_info->options & HA_LEX_CREATE_TMP_TABLE) { - TABLE *table= NULL; - /* Open table and put in temporary table list */ - if (!(table= open_temporary_table(thd, path, db, table_name, 1))) + /* + Open a table (skipping table cache) and add it into + THD::temporary_tables list. + */ + + TABLE *table= open_table_uncached(thd, path, db, table_name, TRUE); + + if (!table) { (void) rm_temporary_table(create_info->db_type, path); goto err; @@ -6301,8 +6306,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, /* table is a normal table: Create temporary table in same directory */ build_table_filename(path, sizeof(path) - 1, new_db, tmp_name, "", FN_IS_TMP); - /* Open our intermediate table */ - new_table= open_temporary_table(thd, path, new_db, tmp_name, 1); + /* Open our intermediate table. */ + new_table= open_table_uncached(thd, path, new_db, tmp_name, TRUE); } if (!new_table) goto err_new_table_cleanup; @@ -6642,7 +6647,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, char path[FN_REFLEN]; TABLE *t_table; build_table_filename(path + 1, sizeof(path) - 1, new_db, table_name, "", 0); - t_table= open_temporary_table(thd, path, new_db, tmp_name, 0); + t_table= open_table_uncached(thd, path, new_db, tmp_name, FALSE); if (t_table) { intern_close_table(t_table); diff --git a/sql/sql_truncate.cc b/sql/sql_truncate.cc index a61abdafe3e..c0bc726a188 100644 --- a/sql/sql_truncate.cc +++ b/sql/sql_truncate.cc @@ -208,8 +208,8 @@ static bool recreate_temporary_table(THD *thd, TABLE *table) ha_create_table(thd, share->normalized_path.str, share->db.str, share->table_name.str, &create_info, 1); - if (open_temporary_table(thd, share->path.str, share->db.str, - share->table_name.str, 1)) + if (open_table_uncached(thd, share->path.str, share->db.str, + share->table_name.str, TRUE)) { error= FALSE; thd->thread_specific_used= TRUE; From 51a3375c98138cee6f51eec807c9a120a84da87a Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 8 Sep 2010 10:25:37 +0200 Subject: [PATCH 11/24] Bug #56292 Deadlock with ALTER TABLE and MERGE tables ALTER TABLE on a MERGE table could cause a deadlock with two other connections if we reached a situation where: 1) A connection doing ALTER TABLE can't upgrade to MDL_EXCLUSIVE on the parent table, but holds TL_READ_NO_INSERT on the child tables. 2) A connection doing DELETE on a child table can't get TL_WRITE on it since ALTER TABLE holds TL_READ_NO_INSERT. 3) A connection doing SELECT on the parent table can't get TL_READ on the child tables since TL_WRITE is ahead in the lock queue, but holds MDL_SHARED_READ on the parent table preventing ALTER TABLE from upgrading. For regular tables, this deadlock is avoided by having ALTER TABLE take a MDL_SHARED_NO_WRITE metadata lock on the table. This prevents DELETE from acquiring MDL_SHARED_WRITE on the table before ALTER TABLE tries to upgrade to MDL_EXCLUSIVE. In the example above, SELECT would therefore not be blocked by the pending DELETE as DELETE would not be able to enter TL_WRITE in the table lock queue. This patch fixes the problem for merge tables by using the same metadata lock type for child tables as for the parent table. The child tables will in this case therefore be locked with MDL_SHARED_NO_WRITE, preventing DELETE from acquiring a metadata lock and enter into the table lock queue. Change in behavior: By taking the same metadata lock for child tables as for the parent table, LOCK TABLE on the parent table will now also implicitly lock the child tables. Since LOCK TABLE on the parent table now takes more than one metadata lock, it is possible for LOCK TABLE ... WRITE on the parent table or child tables to give ER_LOCK_DEADLOCK error. Test case added to mdl_sync.test. Merge.test/.result has been updated to reflect the change to LOCK TABLE. --- mysql-test/r/mdl_sync.result | 38 +++++++++++++++++++ mysql-test/r/merge.result | 3 +- mysql-test/t/mdl_sync.test | 62 +++++++++++++++++++++++++++++++ mysql-test/t/merge.test | 3 +- storage/myisammrg/ha_myisammrg.cc | 2 + 5 files changed, 104 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/mdl_sync.result b/mysql-test/r/mdl_sync.result index d484ab77701..de57e3859b7 100644 --- a/mysql-test/r/mdl_sync.result +++ b/mysql-test/r/mdl_sync.result @@ -2913,3 +2913,41 @@ UNLOCK TABLES; # Connection default UNLOCK TABLES; DROP DATABASE db1; +# +# Bug#56292 Deadlock with ALTER TABLE and MERGE tables +# +DROP TABLE IF EXISTS t1, t2, m1; +CREATE TABLE t1(a INT) engine=MyISAM; +CREATE TABLE t2(a INT) engine=MyISAM; +CREATE TABLE m1(a INT) engine=MERGE UNION=(t1, t2); +INSERT INTO t1 VALUES (1), (2); +INSERT INTO t2 VALUES (3), (4); +# Connection con1 +SET DEBUG_SYNC= 'mdl_upgrade_shared_lock_to_exclusive SIGNAL upgrade WAIT_FOR continue'; +# Sending: +ALTER TABLE m1 engine=MERGE UNION=(t2, t1); +# Connection con2 +# Waiting for ALTER TABLE to try lock upgrade +SET DEBUG_SYNC= 'now WAIT_FOR upgrade'; +# Sending: +DELETE FROM t2 WHERE a = 3; +# Connection default +# Check that DELETE is waiting on a metadata lock and not a table lock. +# Now that DELETE blocks on a metadata lock, we should be able to do +# SELECT * FROM m1 here. SELECT used to be blocked by a DELETE table +# lock request. +SELECT * FROM m1; +a +1 +2 +3 +4 +# Resuming ALTER TABLE +SET DEBUG_SYNC= 'now SIGNAL continue'; +# Connection con1 +# Reaping: ALTER TABLE m1 engine=MERGE UNION=(t2, t1) +# Connection con2 +# Reaping: DELETE FROM t2 WHERE a = 3 +# Connection default +DROP TABLE m1, t1, t2; +SET DEBUG_SYNC= 'RESET'; diff --git a/mysql-test/r/merge.result b/mysql-test/r/merge.result index c7e54288f8b..5d96970ce74 100644 --- a/mysql-test/r/merge.result +++ b/mysql-test/r/merge.result @@ -3444,13 +3444,12 @@ ALTER TABLE m1 ADD INDEX (c1); UNLOCK TABLES; DROP TABLE m1, t1; # -# If children are to be altered, they need an explicit lock. +# Locking the merge table will implicitly lock children. # CREATE TABLE t1 (c1 INT); CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1); LOCK TABLE m1 WRITE; ALTER TABLE t1 ADD INDEX (c1); -ERROR HY000: Table 't1' was locked with a READ lock and can't be updated LOCK TABLE m1 WRITE, t1 WRITE; ALTER TABLE t1 ADD INDEX (c1); UNLOCK TABLES; diff --git a/mysql-test/t/mdl_sync.test b/mysql-test/t/mdl_sync.test index 55c04d7870b..f7780d2003a 100644 --- a/mysql-test/t/mdl_sync.test +++ b/mysql-test/t/mdl_sync.test @@ -4532,6 +4532,68 @@ disconnect con2; disconnect con3; +--echo # +--echo # Bug#56292 Deadlock with ALTER TABLE and MERGE tables +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1, t2, m1; +--enable_warnings + +CREATE TABLE t1(a INT) engine=MyISAM; +CREATE TABLE t2(a INT) engine=MyISAM; +CREATE TABLE m1(a INT) engine=MERGE UNION=(t1, t2); + +INSERT INTO t1 VALUES (1), (2); +INSERT INTO t2 VALUES (3), (4); + +connect(con1, localhost, root); +connect(con2, localhost, root); + +--echo # Connection con1 +connection con1; +SET DEBUG_SYNC= 'mdl_upgrade_shared_lock_to_exclusive SIGNAL upgrade WAIT_FOR continue'; +--echo # Sending: +--send ALTER TABLE m1 engine=MERGE UNION=(t2, t1) + +--echo # Connection con2 +connection con2; +--echo # Waiting for ALTER TABLE to try lock upgrade +SET DEBUG_SYNC= 'now WAIT_FOR upgrade'; +--echo # Sending: +--send DELETE FROM t2 WHERE a = 3 + +--echo # Connection default +connection default; +--echo # Check that DELETE is waiting on a metadata lock and not a table lock. +let $wait_condition= + SELECT COUNT(*) = 1 FROM information_schema.processlist + WHERE state = "Waiting for table metadata lock" AND + info = "DELETE FROM t2 WHERE a = 3"; +--source include/wait_condition.inc +--echo # Now that DELETE blocks on a metadata lock, we should be able to do +--echo # SELECT * FROM m1 here. SELECT used to be blocked by a DELETE table +--echo # lock request. +SELECT * FROM m1; +--echo # Resuming ALTER TABLE +SET DEBUG_SYNC= 'now SIGNAL continue'; + +--echo # Connection con1 +connection con1; +--echo # Reaping: ALTER TABLE m1 engine=MERGE UNION=(t2, t1) +--reap +--echo # Connection con2 +connection con2; +--echo # Reaping: DELETE FROM t2 WHERE a = 3 +--reap +--echo # Connection default +connection default; +DROP TABLE m1, t1, t2; +SET DEBUG_SYNC= 'RESET'; +disconnect con1; +disconnect con2; + + # 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/mysql-test/t/merge.test b/mysql-test/t/merge.test index b6ad3324d19..1553d7f29c5 100644 --- a/mysql-test/t/merge.test +++ b/mysql-test/t/merge.test @@ -2519,12 +2519,11 @@ UNLOCK TABLES; DROP TABLE m1, t1; --echo # ---echo # If children are to be altered, they need an explicit lock. +--echo # Locking the merge table will implicitly lock children. --echo # CREATE TABLE t1 (c1 INT); CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1); LOCK TABLE m1 WRITE; ---error ER_TABLE_NOT_LOCKED_FOR_WRITE ALTER TABLE t1 ADD INDEX (c1); LOCK TABLE m1 WRITE, t1 WRITE; ALTER TABLE t1 ADD INDEX (c1); diff --git a/storage/myisammrg/ha_myisammrg.cc b/storage/myisammrg/ha_myisammrg.cc index f62aff4e383..466c66ec769 100644 --- a/storage/myisammrg/ha_myisammrg.cc +++ b/storage/myisammrg/ha_myisammrg.cc @@ -478,6 +478,8 @@ int ha_myisammrg::add_children_list(void) /* Set the expected table version, to not cause spurious re-prepare. */ child_l->set_table_ref_id(mrg_child_def->get_child_table_ref_type(), mrg_child_def->get_child_def_version()); + /* Use the same metadata lock type for children. */ + child_l->mdl_request.set_type(parent_l->mdl_request.type); /* Link TABLE_LIST object into the children list. */ if (this->children_last_l) child_l->prev_global= this->children_last_l; From 3326614df1d1dd01b3956a8f9c87003d839c485d Mon Sep 17 00:00:00 2001 From: Dmitry Lenev Date: Thu, 9 Sep 2010 18:29:14 +0400 Subject: [PATCH 12/24] Fix for bug #55273 "FLUSH TABLE tm WITH READ LOCK for Merge table causes assert failure". Attempting to use FLUSH TABLE table_list WITH READ LOCK statement for a MERGE table led to an assertion failure if one of its children was not present in the list of tables to be flushed. The problem was not visible in non-debug builds. The assertion failure was caused by the fact that in such situations FLUSH TABLES table_list WITH READ LOCK implementation tried to use (e.g. lock) such child tables without acquiring metadata lock on them. This happened because when opening tables we assumed metadata locks on all tables were already acquired earlier during statement execution and a such assumption was false for MERGE children. This patch fixes the problem by ensuring at open_tables() time that we try to acquire metadata locks on all tables to be opened. For normal tables such requests are satisfied instantly since locks are already acquired for them. For MERGE children metadata locks are acquired in normal fashion. Note that FLUSH TABLES merge_table WITH READ LOCK will lock for read both the MERGE table and its children but will flush only the MERGE table. To flush children one has to mention them in table list explicitly. This is expected behavior and it is consistent with usage patterns for this statement (e.g. in mysqlhotcopy script). --- mysql-test/r/flush.result | 50 +++++++++++++++++++++++++++++++++++++++ mysql-test/t/flush.test | 31 ++++++++++++++++++++++++ sql/sql_base.cc | 6 +++-- sql/sql_base.h | 5 ++++ sql/sql_reload.cc | 42 +++++++++++++++++--------------- sql/sql_yacc.yy | 4 ++++ 6 files changed, 117 insertions(+), 21 deletions(-) diff --git a/mysql-test/r/flush.result b/mysql-test/r/flush.result index aee18d91edf..ced8306c3ab 100644 --- a/mysql-test/r/flush.result +++ b/mysql-test/r/flush.result @@ -373,3 +373,53 @@ commit; # --> connection con2 # --> connection default drop table t1; +# +# Test for bug #55273 "FLUSH TABLE tm WITH READ LOCK for Merge table +# causes assert failure". +# +drop table if exists t1, t2, tm; +create table t1 (i int); +create table t2 (i int); +create table tm (i int) engine=merge union=(t1, t2); +insert into t1 values (1), (2); +insert into t2 values (3), (4); +# The below statement should succeed and lock merge +# table for read. Only merge table gets flushed and +# not underlying tables. +flush tables tm with read lock; +select * from tm; +i +1 +2 +3 +4 +# Check that underlying tables are locked. +select * from t1; +i +1 +2 +select * from t2; +i +3 +4 +unlock tables; +# This statement should succeed as well and flush +# all tables in the list. +flush tables tm, t1, t2 with read lock; +select * from tm; +i +1 +2 +3 +4 +# Naturally, underlying tables should be locked in this case too. +select * from t1; +i +1 +2 +select * from t2; +i +3 +4 +unlock tables; +drop tables tm, t1, t2; diff --git a/mysql-test/t/flush.test b/mysql-test/t/flush.test index d4c3533847d..1cafbe3e6bd 100644 --- a/mysql-test/t/flush.test +++ b/mysql-test/t/flush.test @@ -546,3 +546,34 @@ disconnect con2; connection default; drop table t1; + +--echo # +--echo # Test for bug #55273 "FLUSH TABLE tm WITH READ LOCK for Merge table +--echo # causes assert failure". +--echo # +--disable_warnings +drop table if exists t1, t2, tm; +--enable_warnings +create table t1 (i int); +create table t2 (i int); +create table tm (i int) engine=merge union=(t1, t2); +insert into t1 values (1), (2); +insert into t2 values (3), (4); +--echo # The below statement should succeed and lock merge +--echo # table for read. Only merge table gets flushed and +--echo # not underlying tables. +flush tables tm with read lock; +select * from tm; +--echo # Check that underlying tables are locked. +select * from t1; +select * from t2; +unlock tables; +--echo # This statement should succeed as well and flush +--echo # all tables in the list. +flush tables tm, t1, t2 with read lock; +select * from tm; +--echo # Naturally, underlying tables should be locked in this case too. +select * from t1; +select * from t2; +unlock tables; +drop tables tm, t1, t2; diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 8cd0ce4432a..e2eb836a958 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -4522,13 +4522,15 @@ lock_table_names(THD *thd, ! (flags & MYSQL_OPEN_SKIP_TEMPORARY) && find_temporary_table(thd, table)))) { - if (schema_set.insert(table)) + if (! (flags & MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK) && + schema_set.insert(table)) return TRUE; mdl_requests.push_front(&table->mdl_request); } } - if (! mdl_requests.is_empty()) + if (! (flags & MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK) && + ! mdl_requests.is_empty()) { /* Scoped locks: Take intention exclusive locks on all involved diff --git a/sql/sql_base.h b/sql/sql_base.h index a21850355b4..b1a730ad277 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -122,6 +122,11 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update, (LONG_TIMEOUT = 1 year) rather than the user-supplied timeout value. */ #define MYSQL_LOCK_IGNORE_TIMEOUT 0x0800 +/** + When acquiring "strong" (SNW, SNRW, X) metadata locks on tables to + be open do not acquire global and schema-scope IX locks. +*/ +#define MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK 0x1000 /** Please refer to the internals manual. */ #define MYSQL_OPEN_REOPEN (MYSQL_OPEN_IGNORE_FLUSH |\ diff --git a/sql/sql_reload.cc b/sql/sql_reload.cc index bf38af78536..35f27408247 100644 --- a/sql/sql_reload.cc +++ b/sql/sql_reload.cc @@ -328,7 +328,6 @@ bool reload_acl_and_cache(THD *thd, unsigned long options, ------------------------------------- - you can't flush WITH READ LOCK a non-existent table - you can't flush WITH READ LOCK under LOCK TABLES - - currently incompatible with the GRL (@todo: fix) Effect on views and temporary tables. ------------------------------------ @@ -338,6 +337,13 @@ bool reload_acl_and_cache(THD *thd, unsigned long options, if there is a base table, it's used, otherwise ER_NO_SUCH_TABLE is returned. + Handling of MERGE tables + ------------------------ + For MERGE table this statement will open and lock child tables + for read (it is impossible to lock parent table without it). + Child tables won't be flushed unless they are explicitly present + in the statement's table list. + Implicit commit --------------- This statement causes an implicit commit before and @@ -354,7 +360,6 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) { Lock_tables_prelocking_strategy lock_tables_prelocking_strategy; TABLE_LIST *table_list; - MDL_request_list mdl_requests; /* This is called from SQLCOM_FLUSH, the transaction has @@ -368,17 +373,13 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) } /* - Acquire SNW locks on tables to be flushed. We can't use - lock_table_names() here as this call will also acquire global IX - and database-scope IX locks on the tables, and this will make + Acquire SNW locks on tables to be flushed. Don't acquire global + IX and database-scope IX locks on the tables as this will make this statement incompatible with FLUSH TABLES WITH READ LOCK. */ - for (table_list= all_tables; table_list; - table_list= table_list->next_global) - mdl_requests.push_front(&table_list->mdl_request); - - if (thd->mdl_context.acquire_locks(&mdl_requests, - thd->variables.lock_wait_timeout)) + if (lock_table_names(thd, all_tables, NULL, + thd->variables.lock_wait_timeout, + MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK)) goto error; DEBUG_SYNC(thd,"flush_tables_with_read_lock_after_acquire_locks"); @@ -390,21 +391,24 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED, table_list->db, table_list->table_name, FALSE); - - /* Skip views and temporary tables. */ - table_list->required_type= FRMTYPE_TABLE; /* Don't try to flush views. */ - table_list->open_type= OT_BASE_ONLY; /* Ignore temporary tables. */ + /* Reset ticket to satisfy asserts in open_tables(). */ + table_list->mdl_request.ticket= NULL; } /* Before opening and locking tables the below call also waits for old shares to go away, so the fact that we don't pass MYSQL_LOCK_IGNORE_FLUSH flag to it is important. + Also we don't pass MYSQL_OPEN_HAS_MDL_LOCK flag as we want + to open underlying tables if merge table is flushed. + For underlying tables of the merge the below call has to + acquire SNW locks to ensure that they can be locked for + read without further waiting. */ - if (open_and_lock_tables(thd, all_tables, FALSE, - MYSQL_OPEN_HAS_MDL_LOCK, - &lock_tables_prelocking_strategy) || - thd->locked_tables_list.init_locked_tables(thd)) + if (open_and_lock_tables(thd, all_tables, FALSE, + MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK, + &lock_tables_prelocking_strategy) || + thd->locked_tables_list.init_locked_tables(thd)) { goto error; } diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 83e9078f5cb..2ee2d83f9d2 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -11278,7 +11278,11 @@ opt_with_read_lock: TABLE_LIST *tables= Lex->query_tables; Lex->type|= REFRESH_READ_LOCK; for (; tables; tables= tables->next_global) + { tables->mdl_request.set_type(MDL_SHARED_NO_WRITE); + tables->required_type= FRMTYPE_TABLE; /* Don't try to flush views. */ + tables->open_type= OT_BASE_ONLY; /* Ignore temporary tables. */ + } } ; From 20b7be9263683db32623d07aa53b52fda867c45b Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Mon, 13 Sep 2010 13:31:22 +0200 Subject: [PATCH 13/24] Bug #56448 Assertion failed: ! is_set() with second xa end The problem was that issuing XA END when the XA transaction was already ended, caused an assertion. This assertion tests that the server does not try to send OK to the client if there has already been an error reported. The bug was only noticeable on debug versions of the server. The reason for the problem was that the trans_xa_end() function reported success if the transaction was at XA_IDLE state at the end regardless of any errors occured during processing of trans_xa_end(). So if the transaction state was XA_IDLE already, reported errors would be ignored. This patch fixes the problem by having trans_xa_end() take into consideration any reported errors. The patch also fixes a similar bug with XA PREPARE. Test case added to xa.test. --- mysql-test/r/xa.result | 11 +++++++++++ mysql-test/t/xa.test | 17 +++++++++++++++++ sql/transaction.cc | 6 ++++-- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/xa.result b/mysql-test/r/xa.result index fedbb43ea2a..6ef3bf392d9 100644 --- a/mysql-test/r/xa.result +++ b/mysql-test/r/xa.result @@ -131,3 +131,14 @@ XA START 'xid1'; XA END 'xid1'; XA ROLLBACK 'xid1'; DROP TABLE t1; +# +# Bug#56448 Assertion failed: ! is_set() with second xa end +# +XA START 'x'; +XA END 'x'; +XA END 'x'; +ERROR XAE07: XAER_RMFAIL: The command cannot be executed when global transaction is in the IDLE state +XA PREPARE 'x'; +XA PREPARE 'x'; +ERROR XAE07: XAER_RMFAIL: The command cannot be executed when global transaction is in the PREPARED state +XA ROLLBACK 'x'; diff --git a/mysql-test/t/xa.test b/mysql-test/t/xa.test index fe36af60c27..3fd243398a9 100644 --- a/mysql-test/t/xa.test +++ b/mysql-test/t/xa.test @@ -228,6 +228,23 @@ XA ROLLBACK 'xid1'; disconnect con1; DROP TABLE t1; + +--echo # +--echo # Bug#56448 Assertion failed: ! is_set() with second xa end +--echo # + +XA START 'x'; +XA END 'x'; +# Second XA END caused an assertion. +--error ER_XAER_RMFAIL +XA END 'x'; +XA PREPARE 'x'; +# Second XA PREPARE also caused an assertion. +--error ER_XAER_RMFAIL +XA PREPARE 'x'; +XA ROLLBACK 'x'; + + # Wait till all disconnects are completed --source include/wait_until_count_sessions.inc diff --git a/sql/transaction.cc b/sql/transaction.cc index a28fba8805d..d3e3ba142b9 100644 --- a/sql/transaction.cc +++ b/sql/transaction.cc @@ -565,7 +565,8 @@ bool trans_xa_end(THD *thd) else if (!xa_trans_rolled_back(&thd->transaction.xid_state)) thd->transaction.xid_state.xa_state= XA_IDLE; - DBUG_RETURN(thd->transaction.xid_state.xa_state != XA_IDLE); + DBUG_RETURN(thd->is_error() || + thd->transaction.xid_state.xa_state != XA_IDLE); } @@ -596,7 +597,8 @@ bool trans_xa_prepare(THD *thd) else thd->transaction.xid_state.xa_state= XA_PREPARED; - DBUG_RETURN(thd->transaction.xid_state.xa_state != XA_PREPARED); + DBUG_RETURN(thd->is_error() || + thd->transaction.xid_state.xa_state != XA_PREPARED); } From 6d5065a9f4b6accf51906f5c5e38325dfb4707e8 Mon Sep 17 00:00:00 2001 From: Dmitry Lenev Date: Wed, 15 Sep 2010 18:15:31 +0400 Subject: [PATCH 14/24] Fix for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables". Attempting to issue an INSERT DELAYED statement for a MERGE table might have caused a deadlock if it happened as part of a transaction or under LOCK TABLES, and there was a concurrent DDL or LOCK TABLES ... WRITE statement which tried to lock one of its underlying tables. The problem occurred when a delayed insert handler thread tried to open a MERGE table and discovered that to do this it had also to open all underlying tables and hence acquire metadata locks on them. Since metadata locks on the underlying tables were not pre-acquired by the connection thread executing INSERT DELAYED, attempts to do so might lead to waiting. In this case the connection thread had to wait for the delayed insert thread. If the thread which was preventing the lock on the underlying table from being acquired had to wait for the connection thread (due to this or other metadata locks), a deadlock occurred. This deadlock was not detected by the MDL deadlock detector since waiting for the handler thread by the connection thread is not represented in the wait-for graph. This patch solves the problem by ensuring that the delayed insert handler thread never tries to open underlying tables of a MERGE table. Instead open_tables() is aborted right after the parent table is opened and a ER_DELAYED_NOT_SUPPORTED error is emitted (which is passed to the connection thread and ultimately to the user). --- mysql-test/r/merge.result | 28 ++++++++++++++ mysql-test/t/merge.test | 56 ++++++++++++++++++++++++++++ sql/sql_base.cc | 8 +++- sql/sql_base.h | 13 ++++++- sql/sql_insert.cc | 77 ++++++++++++++++++++++++++++++++++----- 5 files changed, 170 insertions(+), 12 deletions(-) diff --git a/mysql-test/r/merge.result b/mysql-test/r/merge.result index 5d96970ce74..0481ee8f49a 100644 --- a/mysql-test/r/merge.result +++ b/mysql-test/r/merge.result @@ -3575,4 +3575,32 @@ ERROR HY000: The definition of table 'v1' prevents operation DELETE on table 'm1 drop view v1; drop temporary table tmp; drop table t1, t2, t3, m1, m2; +# +# Test for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables". +# +drop table if exists t1, t2, tm; +create table t1(a int); +create table t2(a int); +create table tm(a int) engine=merge union=(t1, t2); +begin; +select * from t1; +a +# Connection 'con1'. +# Sending: +alter table t1 comment 'test'; +# Connection 'default'. +# Wait until ALTER TABLE blocks and starts waiting +# for connection 'default'. It should wait with a +# pending SNW lock on 't1'. +# Attempt to perform delayed insert into 'tm' should not lead +# to a deadlock. Instead error ER_DELAYED_NOT_SUPPORTED should +# be emitted. +insert delayed into tm values (1); +ERROR HY000: DELAYED option not supported for table 'tm' +# Unblock ALTER TABLE. +commit; +# Connection 'con1'. +# Reaping ALTER TABLE: +# Connection 'default'. +drop tables tm, t1, t2; End of 6.0 tests diff --git a/mysql-test/t/merge.test b/mysql-test/t/merge.test index 1553d7f29c5..3633a8b30ad 100644 --- a/mysql-test/t/merge.test +++ b/mysql-test/t/merge.test @@ -2,6 +2,9 @@ # Test of MERGE TABLES # +# Save the initial number of concurrent sessions. +--source include/count_sessions.inc + # MERGE tables require MyISAM tables let $default=`select @@global.storage_engine`; set global storage_engine=myisam; @@ -2664,6 +2667,55 @@ drop view v1; drop temporary table tmp; drop table t1, t2, t3, m1, m2; + +--echo # +--echo # Test for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables". +--echo # +connect (con1,localhost,root,,); +connection default; +--disable_warnings +drop table if exists t1, t2, tm; +--enable_warnings +create table t1(a int); +create table t2(a int); +create table tm(a int) engine=merge union=(t1, t2); +begin; +select * from t1; + +--echo # Connection 'con1'. +connection con1; +--echo # Sending: +--send alter table t1 comment 'test' + +--echo # Connection 'default'. +connection default; +--echo # Wait until ALTER TABLE blocks and starts waiting +--echo # for connection 'default'. It should wait with a +--echo # pending SNW lock on 't1'. +let $wait_condition= + select count(*) = 1 from information_schema.processlist + where state = "Waiting for table metadata lock" and + info = "alter table t1 comment 'test'"; +--source include/wait_condition.inc +--echo # Attempt to perform delayed insert into 'tm' should not lead +--echo # to a deadlock. Instead error ER_DELAYED_NOT_SUPPORTED should +--echo # be emitted. +--error ER_DELAYED_NOT_SUPPORTED +insert delayed into tm values (1); +--echo # Unblock ALTER TABLE. +commit; + +--echo # Connection 'con1'. +connection con1; +--echo # Reaping ALTER TABLE: +--reap + +--echo # Connection 'default'. +connection default; +disconnect con1; +drop tables tm, t1, t2; + + --echo End of 6.0 tests --disable_result_log @@ -2671,3 +2723,7 @@ drop table t1, t2, t3, m1, m2; eval set global storage_engine=$default; --enable_result_log --enable_query_log + +# 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 e2eb836a958..8305283cd17 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -5165,6 +5165,8 @@ static bool check_lock_and_start_stmt(THD *thd, @param[in] lock_type lock to use for table @param[in] flags options to be used while opening and locking table (see open_table(), mysql_lock_tables()) + @param[in] prelocking_strategy Strategy which specifies how prelocking + algorithm should work for this statement. @return table @retval != NULL OK, opened table returned @@ -5190,7 +5192,8 @@ static bool check_lock_and_start_stmt(THD *thd, */ TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l, - thr_lock_type lock_type, uint flags) + thr_lock_type lock_type, uint flags, + Prelocking_strategy *prelocking_strategy) { TABLE_LIST *save_next_global; DBUG_ENTER("open_n_lock_single_table"); @@ -5206,7 +5209,8 @@ TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l, table_l->required_type= FRMTYPE_TABLE; /* Open the table. */ - if (open_and_lock_tables(thd, table_l, FALSE, flags)) + if (open_and_lock_tables(thd, table_l, FALSE, flags, + prelocking_strategy)) table_l->table= NULL; /* Just to be sure. */ /* Restore list. */ diff --git a/sql/sql_base.h b/sql/sql_base.h index b1a730ad277..7ae3971942b 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -246,7 +246,8 @@ bool open_and_lock_tables(THD *thd, TABLE_LIST *tables, int open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables, bool derived); /* simple open_and_lock_tables without derived handling for single table */ TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l, - thr_lock_type lock_type, uint flags); + thr_lock_type lock_type, uint flags, + Prelocking_strategy *prelocking_strategy); bool open_normal_and_derived_tables(THD *thd, TABLE_LIST *tables, uint flags); bool lock_tables(THD *thd, TABLE_LIST *tables, uint counter, uint flags); int decide_logging_format(THD *thd, TABLE_LIST *tables); @@ -455,6 +456,16 @@ open_tables(THD *thd, TABLE_LIST **tables, uint *counter, uint flags) } +inline TABLE *open_n_lock_single_table(THD *thd, TABLE_LIST *table_l, + thr_lock_type lock_type, uint flags) +{ + DML_prelocking_strategy prelocking_strategy; + + return open_n_lock_single_table(thd, table_l, lock_type, flags, + &prelocking_strategy); +} + + /* open_and_lock_tables with derived handling */ inline bool open_and_lock_tables(THD *thd, TABLE_LIST *tables, bool derived, uint flags) diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index adbdc1ffbc8..ad324fed4fe 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2495,6 +2495,65 @@ void kill_delayed_threads(void) } +/** + A strategy for the prelocking algorithm which prevents the + delayed insert thread from opening tables with engines which + do not support delayed inserts. + + Particularly it allows to abort open_tables() as soon as we + discover that we have opened a MERGE table, without acquiring + metadata locks on underlying tables. +*/ + +class Delayed_prelocking_strategy : public Prelocking_strategy +{ +public: + virtual bool handle_routine(THD *thd, Query_tables_list *prelocking_ctx, + Sroutine_hash_entry *rt, sp_head *sp, + bool *need_prelocking); + virtual bool handle_table(THD *thd, Query_tables_list *prelocking_ctx, + TABLE_LIST *table_list, bool *need_prelocking); + virtual bool handle_view(THD *thd, Query_tables_list *prelocking_ctx, + TABLE_LIST *table_list, bool *need_prelocking); +}; + + +bool Delayed_prelocking_strategy:: +handle_table(THD *thd, Query_tables_list *prelocking_ctx, + TABLE_LIST *table_list, bool *need_prelocking) +{ + DBUG_ASSERT(table_list->lock_type == TL_WRITE_DELAYED); + + if (!(table_list->table->file->ha_table_flags() & HA_CAN_INSERT_DELAYED)) + { + my_error(ER_DELAYED_NOT_SUPPORTED, MYF(0), table_list->table_name); + return TRUE; + } + return FALSE; +} + + +bool Delayed_prelocking_strategy:: +handle_routine(THD *thd, Query_tables_list *prelocking_ctx, + Sroutine_hash_entry *rt, sp_head *sp, + bool *need_prelocking) +{ + /* LEX used by the delayed insert thread has no routines. */ + DBUG_ASSERT(0); + return FALSE; +} + + +bool Delayed_prelocking_strategy:: +handle_view(THD *thd, Query_tables_list *prelocking_ctx, + TABLE_LIST *table_list, bool *need_prelocking) +{ + /* We don't open views in the delayed insert thread. */ + DBUG_ASSERT(0); + return FALSE; +} + + /** Open and lock table for use by delayed thread and check that this table is suitable for delayed inserts. @@ -2505,21 +2564,21 @@ void kill_delayed_threads(void) bool Delayed_insert::open_and_lock_table() { + Delayed_prelocking_strategy prelocking_strategy; + + /* + Use special prelocking strategy to get ER_DELAYED_NOT_SUPPORTED + error for tables with engines which don't support delayed inserts. + */ if (!(table= open_n_lock_single_table(&thd, &table_list, TL_WRITE_DELAYED, - MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK))) + MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK, + &prelocking_strategy))) { thd.fatal_error(); // Abort waiting inserts return TRUE; } - if (!(table->file->ha_table_flags() & HA_CAN_INSERT_DELAYED)) - { - /* To rollback InnoDB statement transaction. */ - trans_rollback_stmt(&thd); - my_error(ER_DELAYED_NOT_SUPPORTED, MYF(ME_FATALERROR), - table_list.table_name); - return TRUE; - } + if (table->triggers) { /* From f14d947c9898569367fda8028a8287ff676777c8 Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Thu, 16 Sep 2010 11:11:13 +0200 Subject: [PATCH 15/24] Bug #56595 RENAME TABLE causes assert on OS X The problem was that RENAME TABLE caused an assert if the system variable lower_case_table_names was 2 (default on Mac OS X) and the old table name was given in upper case. This caused lowercase_table2.test to fail. The assert checks that an exclusive metadata lock is held by the connection trying to do RENAME TABLE - specificially during updates of table triggers. The assert was triggered since the check is case sensitive and the lock was held on the normalized (lower case) version of the table name. This patch fixes the problem by making sure a normalized version of the table name is used for the metadata lock check, while using a non-normalized version of the table name for the rename of trigger files. The same is done for ALTER TABLE ... RENAME. Regression testing for the bug itself is already covered by lowercase_table2.test. Additional coverage added to lowercase_fs_off.test. --- mysql-test/r/lowercase_fs_off.result | 8 ++++++++ mysql-test/t/lowercase_fs_off.test | 11 +++++++++++ sql/sql_rename.cc | 1 + sql/sql_table.cc | 5 +++-- sql/sql_trigger.cc | 6 ++++-- sql/sql_trigger.h | 1 + 6 files changed, 28 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/lowercase_fs_off.result b/mysql-test/r/lowercase_fs_off.result index 30f835a8ea3..c3284b225dd 100644 --- a/mysql-test/r/lowercase_fs_off.result +++ b/mysql-test/r/lowercase_fs_off.result @@ -55,3 +55,11 @@ DROP USER user_1@localhost; DROP USER USER_1@localhost; DROP DATABASE db1; use test; +# +# Extra test coverage for Bug#56595 RENAME TABLE causes assert on OS X +# +CREATE TABLE t1(a INT); +CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW SET new.a= 1; +RENAME TABLE t1 TO T1; +ALTER TABLE T1 RENAME t1; +DROP TABLE t1; diff --git a/mysql-test/t/lowercase_fs_off.test b/mysql-test/t/lowercase_fs_off.test index 86d1e084c29..1be0351f9bc 100644 --- a/mysql-test/t/lowercase_fs_off.test +++ b/mysql-test/t/lowercase_fs_off.test @@ -91,3 +91,14 @@ DROP DATABASE db1; use test; # End of 5.0 tests + + +--echo # +--echo # Extra test coverage for Bug#56595 RENAME TABLE causes assert on OS X +--echo # + +CREATE TABLE t1(a INT); +CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW SET new.a= 1; +RENAME TABLE t1 TO T1; +ALTER TABLE T1 RENAME t1; +DROP TABLE t1; diff --git a/sql/sql_rename.cc b/sql/sql_rename.cc index 97f8e46d052..ac15239b040 100644 --- a/sql/sql_rename.cc +++ b/sql/sql_rename.cc @@ -285,6 +285,7 @@ do_rename(THD *thd, TABLE_LIST *ren_table, char *new_db, char *new_table_name, { if ((rc= Table_triggers_list::change_table_name(thd, ren_table->db, old_alias, + ren_table->table_name, new_db, new_alias))) { diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 9d8f59189d0..d8ef6d7030a 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -5918,7 +5918,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, *fn_ext(new_name)=0; if (mysql_rename_table(old_db_type,db,table_name,new_db,new_alias, 0)) error= -1; - else if (Table_triggers_list::change_table_name(thd, db, table_name, + else if (Table_triggers_list::change_table_name(thd, db, + alias, table_name, new_db, new_alias)) { (void) mysql_rename_table(old_db_type, new_db, new_alias, db, @@ -6555,7 +6556,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, (need_copy_table != ALTER_TABLE_METADATA_ONLY || mysql_rename_table(save_old_db_type, db, table_name, new_db, new_alias, NO_FRM_RENAME)) && - Table_triggers_list::change_table_name(thd, db, table_name, + Table_triggers_list::change_table_name(thd, db, alias, table_name, new_db, new_alias))) { /* Try to get everything back. */ diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index c1405015365..392343eb9a3 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -1874,6 +1874,7 @@ Table_triggers_list::change_table_name_in_trignames(const char *old_db_name, @param[in,out] thd Thread context @param[in] db Old database of subject table + @param[in] old_alias Old alias of subject table @param[in] old_table Old name of subject table @param[in] new_db New database for subject table @param[in] new_table New name of subject table @@ -1890,6 +1891,7 @@ Table_triggers_list::change_table_name_in_trignames(const char *old_db_name, */ bool Table_triggers_list::change_table_name(THD *thd, const char *db, + const char *old_alias, const char *old_table, const char *new_db, const char *new_table) @@ -1911,7 +1913,7 @@ bool Table_triggers_list::change_table_name(THD *thd, const char *db, MDL_EXCLUSIVE)); DBUG_ASSERT(my_strcasecmp(table_alias_charset, db, new_db) || - my_strcasecmp(table_alias_charset, old_table, new_table)); + my_strcasecmp(table_alias_charset, old_alias, new_table)); if (Table_triggers_list::check_n_load(thd, db, old_table, &table, TRUE)) { @@ -1920,7 +1922,7 @@ bool Table_triggers_list::change_table_name(THD *thd, const char *db, } if (table.triggers) { - LEX_STRING old_table_name= { (char *) old_table, strlen(old_table) }; + LEX_STRING old_table_name= { (char *) old_alias, strlen(old_alias) }; LEX_STRING new_table_name= { (char *) new_table, strlen(new_table) }; /* Since triggers should be in the same schema as their subject tables diff --git a/sql/sql_trigger.h b/sql/sql_trigger.h index 28bf0a60877..dd954957f07 100644 --- a/sql/sql_trigger.h +++ b/sql/sql_trigger.h @@ -157,6 +157,7 @@ public: TABLE *table, bool names_only); static bool drop_all_triggers(THD *thd, char *db, char *table_name); static bool change_table_name(THD *thd, const char *db, + const char *old_alias, const char *old_table, const char *new_db, const char *new_table); From 355b74d2a622722aa028c806d44f8b9c4495094d Mon Sep 17 00:00:00 2001 From: Dmitry Lenev Date: Thu, 16 Sep 2010 18:06:46 +0400 Subject: [PATCH 16/24] Fix for merge.test failures in mysql-5.5-runtime tree for embedded server Test case for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables" can't be run against embedded server. Embedded server converts all DELAYED INSERTs into ordinary INSERTs and this test can't work properly if such conversion happens. Moved this test from merge.test to delayed.test which is skipped if test suite is run with --embedded-server option. --- mysql-test/r/delayed.result | 28 +++++++++++++++++++++ mysql-test/r/merge.result | 28 --------------------- mysql-test/t/delayed.test | 49 +++++++++++++++++++++++++++++++++++++ mysql-test/t/merge.test | 48 ------------------------------------ 4 files changed, 77 insertions(+), 76 deletions(-) diff --git a/mysql-test/r/delayed.result b/mysql-test/r/delayed.result index 4cb97719ff2..77aa0d49407 100644 --- a/mysql-test/r/delayed.result +++ b/mysql-test/r/delayed.result @@ -422,3 +422,31 @@ UNLOCK TABLES; # Connection con1 # Connection default DROP TABLE t1, t2, t3; +# +# Test for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables". +# +drop table if exists t1, t2, tm; +create table t1(a int); +create table t2(a int); +create table tm(a int) engine=merge union=(t1, t2); +begin; +select * from t1; +a +# Connection 'con1'. +# Sending: +alter table t1 comment 'test'; +# Connection 'default'. +# Wait until ALTER TABLE blocks and starts waiting +# for connection 'default'. It should wait with a +# pending SNW lock on 't1'. +# Attempt to perform delayed insert into 'tm' should not lead +# to a deadlock. Instead error ER_DELAYED_NOT_SUPPORTED should +# be emitted. +insert delayed into tm values (1); +ERROR HY000: DELAYED option not supported for table 'tm' +# Unblock ALTER TABLE. +commit; +# Connection 'con1'. +# Reaping ALTER TABLE: +# Connection 'default'. +drop tables tm, t1, t2; diff --git a/mysql-test/r/merge.result b/mysql-test/r/merge.result index 0481ee8f49a..5d96970ce74 100644 --- a/mysql-test/r/merge.result +++ b/mysql-test/r/merge.result @@ -3575,32 +3575,4 @@ ERROR HY000: The definition of table 'v1' prevents operation DELETE on table 'm1 drop view v1; drop temporary table tmp; drop table t1, t2, t3, m1, m2; -# -# Test for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables". -# -drop table if exists t1, t2, tm; -create table t1(a int); -create table t2(a int); -create table tm(a int) engine=merge union=(t1, t2); -begin; -select * from t1; -a -# Connection 'con1'. -# Sending: -alter table t1 comment 'test'; -# Connection 'default'. -# Wait until ALTER TABLE blocks and starts waiting -# for connection 'default'. It should wait with a -# pending SNW lock on 't1'. -# Attempt to perform delayed insert into 'tm' should not lead -# to a deadlock. Instead error ER_DELAYED_NOT_SUPPORTED should -# be emitted. -insert delayed into tm values (1); -ERROR HY000: DELAYED option not supported for table 'tm' -# Unblock ALTER TABLE. -commit; -# Connection 'con1'. -# Reaping ALTER TABLE: -# Connection 'default'. -drop tables tm, t1, t2; End of 6.0 tests diff --git a/mysql-test/t/delayed.test b/mysql-test/t/delayed.test index 5f56cdf54e1..3a2bc982ad3 100644 --- a/mysql-test/t/delayed.test +++ b/mysql-test/t/delayed.test @@ -552,3 +552,52 @@ disconnect con1; connection default; DROP TABLE t1, t2, t3; --enable_ps_protocol + + +--echo # +--echo # Test for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables". +--echo # +connect (con1,localhost,root,,); +connection default; +--disable_warnings +drop table if exists t1, t2, tm; +--enable_warnings +create table t1(a int); +create table t2(a int); +create table tm(a int) engine=merge union=(t1, t2); +begin; +select * from t1; + +--echo # Connection 'con1'. +connection con1; +--echo # Sending: +--send alter table t1 comment 'test' + +--echo # Connection 'default'. +connection default; +--echo # Wait until ALTER TABLE blocks and starts waiting +--echo # for connection 'default'. It should wait with a +--echo # pending SNW lock on 't1'. +let $wait_condition= + select count(*) = 1 from information_schema.processlist + where state = "Waiting for table metadata lock" and + info = "alter table t1 comment 'test'"; +--source include/wait_condition.inc +--echo # Attempt to perform delayed insert into 'tm' should not lead +--echo # to a deadlock. Instead error ER_DELAYED_NOT_SUPPORTED should +--echo # be emitted. +--error ER_DELAYED_NOT_SUPPORTED +insert delayed into tm values (1); +--echo # Unblock ALTER TABLE. +commit; + +--echo # Connection 'con1'. +connection con1; +--echo # Reaping ALTER TABLE: +--reap + +disconnect con1; +--source include/wait_until_disconnected.inc +--echo # Connection 'default'. +connection default; +drop tables tm, t1, t2; diff --git a/mysql-test/t/merge.test b/mysql-test/t/merge.test index 3633a8b30ad..51c4ca51a29 100644 --- a/mysql-test/t/merge.test +++ b/mysql-test/t/merge.test @@ -2668,54 +2668,6 @@ drop temporary table tmp; drop table t1, t2, t3, m1, m2; ---echo # ---echo # Test for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables". ---echo # -connect (con1,localhost,root,,); -connection default; ---disable_warnings -drop table if exists t1, t2, tm; ---enable_warnings -create table t1(a int); -create table t2(a int); -create table tm(a int) engine=merge union=(t1, t2); -begin; -select * from t1; - ---echo # Connection 'con1'. -connection con1; ---echo # Sending: ---send alter table t1 comment 'test' - ---echo # Connection 'default'. -connection default; ---echo # Wait until ALTER TABLE blocks and starts waiting ---echo # for connection 'default'. It should wait with a ---echo # pending SNW lock on 't1'. -let $wait_condition= - select count(*) = 1 from information_schema.processlist - where state = "Waiting for table metadata lock" and - info = "alter table t1 comment 'test'"; ---source include/wait_condition.inc ---echo # Attempt to perform delayed insert into 'tm' should not lead ---echo # to a deadlock. Instead error ER_DELAYED_NOT_SUPPORTED should ---echo # be emitted. ---error ER_DELAYED_NOT_SUPPORTED -insert delayed into tm values (1); ---echo # Unblock ALTER TABLE. -commit; - ---echo # Connection 'con1'. -connection con1; ---echo # Reaping ALTER TABLE: ---reap - ---echo # Connection 'default'. -connection default; -disconnect con1; -drop tables tm, t1, t2; - - --echo End of 6.0 tests --disable_result_log From 5d06dddff34475c1a2b76be2b16d804e63a2f32b Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 22 Sep 2010 10:15:41 +0200 Subject: [PATCH 17/24] Bug #56494 Segfault in upgrade_shared_lock_to_exclusive() for REPAIR of merge table Bug #56422 CHECK TABLE run when the table is locked reports corruption along with timeout The crash happened if a table maintenance statement (ANALYZE TABLE, REPAIR TABLE, etc.) was executed on a MERGE table and opening and locking a child table failed. This could for example happen if a child table did not exist or if a lock timeout happened while waiting for a conflicting metadata lock to disappear. Since opening and locking the MERGE table and its children failed, the tables would be closed and the metadata locks released. However, TABLE_LIST::table for the MERGE table would still be set, with its value invalid since the tables had been closed. This caused the table maintenance statement to try to continue and upgrade the metadata lock on the MERGE table. But since the lock already had been released, this caused a segfault. This patch fixes the problem by setting TABLE_LIST::table to NULL if open_and_lock_tables() fails. This prevents maintenance statements from continuing and trying to upgrade the metadata lock. The patch includes a 5.5 version of the fix for Bug #46339 crash on REPAIR TABLE merge table USE_FRM. This bug caused REPAIR TABLE ... USE_FRM to give an assert when used on merge tables. The patch also enables the CHECK TABLE statement for log tables. Before, CHECK TABLE for log tables gave ER_CANT_LOCK_LOG_TABLE, yet still counted the statement as successfully executed. With the changes to table maintenance statement error handling in this patch, CHECK TABLE would no longer be considered as successful in this case. This would have caused upgrade scripts to mistakenly think that the general and slow logs are corrupted and have to be repaired. Enabling CHECK TABLES for log tables prevents this from happening. Finally, the patch changes the error message from "Corrupt" to "Operation failed" for a number of issues not related to table corruption. For example "Lock wait timeout exceeded" and "Deadlock found trying to get lock". Test cases added to merge.test and check.test. --- mysql-test/r/check.result | 16 ++++ mysql-test/r/log_tables_upgrade.result | 8 +- mysql-test/r/merge.result | 88 +++++++++++++++++++- mysql-test/r/myisampack.result | 10 +-- mysql-test/r/mysql_upgrade.result | 48 +++-------- mysql-test/r/mysql_upgrade_ssl.result | 8 +- mysql-test/t/check.test | 24 ++++++ mysql-test/t/merge.test | 108 +++++++++++++++++++++++++ sql/sql_admin.cc | 46 +++++++++-- sql/sql_parse.cc | 2 +- 10 files changed, 295 insertions(+), 63 deletions(-) diff --git a/mysql-test/r/check.result b/mysql-test/r/check.result index 0bff34dba20..ac9c7bca9d7 100644 --- a/mysql-test/r/check.result +++ b/mysql-test/r/check.result @@ -23,3 +23,19 @@ REPAIR TABLE t1; Table Op Msg_type Msg_text test.t1 repair status OK DROP TABLE t1; +# +# Bug#56422 CHECK TABLE run when the table is locked reports corruption +# along with timeout +# +DROP TABLE IF EXISTS t1; +CREATE TABLE t1(a INT); +LOCK TABLE t1 WRITE; +# Connection con1 +SET lock_wait_timeout= 1; +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check Error Lock wait timeout exceeded; try restarting transaction +test.t1 check status Operation failed +# Connection default +UNLOCK TABLES; +DROP TABLE t1; diff --git a/mysql-test/r/log_tables_upgrade.result b/mysql-test/r/log_tables_upgrade.result index 5d9be85a48a..814ebd7d5dc 100644 --- a/mysql-test/r/log_tables_upgrade.result +++ b/mysql-test/r/log_tables_upgrade.result @@ -17,9 +17,7 @@ mysql.columns_priv OK mysql.db OK mysql.event OK mysql.func OK -mysql.general_log -Error : You can't use locks with log tables. -status : OK +mysql.general_log OK mysql.help_category OK mysql.help_keyword OK mysql.help_relation OK @@ -31,9 +29,7 @@ mysql.proc OK mysql.procs_priv OK mysql.renamed_general_log OK mysql.servers OK -mysql.slow_log -Error : You can't use locks with log tables. -status : OK +mysql.slow_log OK mysql.tables_priv OK mysql.time_zone OK mysql.time_zone_leap_second OK diff --git a/mysql-test/r/merge.result b/mysql-test/r/merge.result index 5d96970ce74..bb6af084f38 100644 --- a/mysql-test/r/merge.result +++ b/mysql-test/r/merge.result @@ -2358,6 +2358,48 @@ t2 WHERE b SOUNDS LIKE e AND d = 1; id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE NULL NULL NULL NULL NULL NULL NULL Impossible WHERE noticed after reading const tables DROP TABLE t2, t1; +# +# Bug#46339 - crash on REPAIR TABLE merge table USE_FRM +# +DROP TABLE IF EXISTS m1, t1; +CREATE TABLE t1 (c1 INT) ENGINE=MYISAM; +CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1) INSERT_METHOD=LAST; +LOCK TABLE m1 READ; +REPAIR TABLE m1 USE_FRM; +Table Op Msg_type Msg_text +test.m1 repair Error Table 'm1' was locked with a READ lock and can't be updated +test.m1 repair status Operation failed +UNLOCK TABLES; +REPAIR TABLE m1 USE_FRM; +Table Op Msg_type Msg_text +test.m1 repair note The storage engine for the table doesn't support repair +DROP TABLE m1,t1; +CREATE TABLE m1 (f1 BIGINT) ENGINE=MRG_MyISAM UNION(t1); +REPAIR TABLE m1 USE_FRM; +Table Op Msg_type Msg_text +test.m1 repair Warning Can't open table +test.m1 repair error Corrupt +CREATE TABLE t1 (f1 BIGINT) ENGINE = MyISAM; +REPAIR TABLE m1 USE_FRM; +Table Op Msg_type Msg_text +test.m1 repair note The storage engine for the table doesn't support repair +REPAIR TABLE m1; +Table Op Msg_type Msg_text +test.m1 repair note The storage engine for the table doesn't support repair +DROP TABLE m1, t1; +CREATE TEMPORARY TABLE m1 (f1 BIGINT) ENGINE=MRG_MyISAM UNION(t1); +REPAIR TABLE m1 USE_FRM; +Table Op Msg_type Msg_text +test.m1 repair Error Table 'test.m1' doesn't exist +test.m1 repair error Corrupt +CREATE TEMPORARY TABLE t1 (f1 BIGINT) ENGINE=MyISAM; +REPAIR TABLE m1 USE_FRM; +Table Op Msg_type Msg_text +m1 repair error Cannot repair temporary table from .frm file +REPAIR TABLE m1; +Table Op Msg_type Msg_text +test.m1 repair note The storage engine for the table doesn't support repair +DROP TABLE m1, t1; End of 5.1 tests # # An additional test case for Bug#27430 Crash in subquery code @@ -2677,7 +2719,7 @@ OPTIMIZE TABLE t1; Table Op Msg_type Msg_text test.t1 optimize Error Table 'test.t_not_exists' doesn't exist test.t1 optimize Error Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist -test.t1 optimize note The storage engine for the table doesn't support optimize +test.t1 optimize error Corrupt DROP TABLE t1; # # Bug#36171 - CREATE TEMPORARY TABLE and MERGE engine @@ -3575,4 +3617,48 @@ ERROR HY000: The definition of table 'v1' prevents operation DELETE on table 'm1 drop view v1; drop temporary table tmp; drop table t1, t2, t3, m1, m2; +# +# Bug#56494 Segfault in upgrade_shared_lock_to_exclusive() for +# REPAIR of merge table +# +DROP TABLE IF EXISTS t1, t2, t_not_exists; +CREATE TABLE t1(a INT); +ALTER TABLE t1 engine= MERGE UNION (t_not_exists); +ANALYZE TABLE t1; +Table Op Msg_type Msg_text +test.t1 analyze Error Table 'test.t_not_exists' doesn't exist +test.t1 analyze Error Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist +test.t1 analyze error Corrupt +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check Error Table 'test.t_not_exists' doesn't exist +test.t1 check Error Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist +test.t1 check error Corrupt +CHECKSUM TABLE t1; +Table Checksum +test.t1 NULL +Warnings: +Error 1146 Table 'test.t_not_exists' doesn't exist +Error 1168 Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist +OPTIMIZE TABLE t1; +Table Op Msg_type Msg_text +test.t1 optimize Error Table 'test.t_not_exists' doesn't exist +test.t1 optimize Error Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist +test.t1 optimize error Corrupt +REPAIR TABLE t1; +Table Op Msg_type Msg_text +test.t1 repair Error Table 'test.t_not_exists' doesn't exist +test.t1 repair Error Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist +test.t1 repair error Corrupt +REPAIR TABLE t1 USE_FRM; +Table Op Msg_type Msg_text +test.t1 repair Warning Can't open table +test.t1 repair error Corrupt +DROP TABLE t1; +CREATE TABLE t1(a INT); +CREATE TABLE t2(a INT) engine= MERGE UNION (t1); +REPAIR TABLE t2 USE_FRM; +Table Op Msg_type Msg_text +test.t2 repair note The storage engine for the table doesn't support repair +DROP TABLE t1, t2; End of 6.0 tests diff --git a/mysql-test/r/myisampack.result b/mysql-test/r/myisampack.result index dd14c31f32e..91700701139 100644 --- a/mysql-test/r/myisampack.result +++ b/mysql-test/r/myisampack.result @@ -46,14 +46,12 @@ insert into t1 select * from t1; flush tables; optimize table t1; Table Op Msg_type Msg_text -test.t1 optimize error Table 'test.t1' is read only -Warnings: -Error 1036 Table 't1' is read only +test.t1 optimize Error Table 't1' is read only +test.t1 optimize status Operation failed repair table t1; Table Op Msg_type Msg_text -test.t1 repair error Table 'test.t1' is read only -Warnings: -Error 1036 Table 't1' is read only +test.t1 repair Error Table 't1' is read only +test.t1 repair status Operation failed drop table t1; # # BUG#41541 - Valgrind warnings on packed MyISAM table diff --git a/mysql-test/r/mysql_upgrade.result b/mysql-test/r/mysql_upgrade.result index 58f6ffd4040..5252d5f0c17 100644 --- a/mysql-test/r/mysql_upgrade.result +++ b/mysql-test/r/mysql_upgrade.result @@ -5,9 +5,7 @@ mysql.columns_priv OK mysql.db OK mysql.event OK mysql.func OK -mysql.general_log -Error : You can't use locks with log tables. -status : OK +mysql.general_log OK mysql.help_category OK mysql.help_keyword OK mysql.help_relation OK @@ -18,9 +16,7 @@ mysql.plugin OK mysql.proc OK mysql.procs_priv OK mysql.servers OK -mysql.slow_log -Error : You can't use locks with log tables. -status : OK +mysql.slow_log OK mysql.tables_priv OK mysql.time_zone OK mysql.time_zone_leap_second OK @@ -37,9 +33,7 @@ mysql.columns_priv OK mysql.db OK mysql.event OK mysql.func OK -mysql.general_log -Error : You can't use locks with log tables. -status : OK +mysql.general_log OK mysql.help_category OK mysql.help_keyword OK mysql.help_relation OK @@ -50,9 +44,7 @@ mysql.plugin OK mysql.proc OK mysql.procs_priv OK mysql.servers OK -mysql.slow_log -Error : You can't use locks with log tables. -status : OK +mysql.slow_log OK mysql.tables_priv OK mysql.time_zone OK mysql.time_zone_leap_second OK @@ -69,9 +61,7 @@ mysql.columns_priv OK mysql.db OK mysql.event OK mysql.func OK -mysql.general_log -Error : You can't use locks with log tables. -status : OK +mysql.general_log OK mysql.help_category OK mysql.help_keyword OK mysql.help_relation OK @@ -82,9 +72,7 @@ mysql.plugin OK mysql.proc OK mysql.procs_priv OK mysql.servers OK -mysql.slow_log -Error : You can't use locks with log tables. -status : OK +mysql.slow_log OK mysql.tables_priv OK mysql.time_zone OK mysql.time_zone_leap_second OK @@ -103,9 +91,7 @@ mysql.columns_priv OK mysql.db OK mysql.event OK mysql.func OK -mysql.general_log -Error : You can't use locks with log tables. -status : OK +mysql.general_log OK mysql.help_category OK mysql.help_keyword OK mysql.help_relation OK @@ -116,9 +102,7 @@ mysql.plugin OK mysql.proc OK mysql.procs_priv OK mysql.servers OK -mysql.slow_log -Error : You can't use locks with log tables. -status : OK +mysql.slow_log OK mysql.tables_priv OK mysql.time_zone OK mysql.time_zone_leap_second OK @@ -141,9 +125,7 @@ mysql.columns_priv OK mysql.db OK mysql.event OK mysql.func OK -mysql.general_log -Error : You can't use locks with log tables. -status : OK +mysql.general_log OK mysql.help_category OK mysql.help_keyword OK mysql.help_relation OK @@ -154,9 +136,7 @@ mysql.plugin OK mysql.proc OK mysql.procs_priv OK mysql.servers OK -mysql.slow_log -Error : You can't use locks with log tables. -status : OK +mysql.slow_log OK mysql.tables_priv OK mysql.time_zone OK mysql.time_zone_leap_second OK @@ -182,9 +162,7 @@ mysql.columns_priv OK mysql.db OK mysql.event OK mysql.func OK -mysql.general_log -Error : You can't use locks with log tables. -status : OK +mysql.general_log OK mysql.help_category OK mysql.help_keyword OK mysql.help_relation OK @@ -195,9 +173,7 @@ mysql.plugin OK mysql.proc OK mysql.procs_priv OK mysql.servers OK -mysql.slow_log -Error : You can't use locks with log tables. -status : OK +mysql.slow_log OK mysql.tables_priv OK mysql.time_zone OK mysql.time_zone_leap_second OK diff --git a/mysql-test/r/mysql_upgrade_ssl.result b/mysql-test/r/mysql_upgrade_ssl.result index f1229c4a405..b89862966b5 100644 --- a/mysql-test/r/mysql_upgrade_ssl.result +++ b/mysql-test/r/mysql_upgrade_ssl.result @@ -7,9 +7,7 @@ mysql.columns_priv OK mysql.db OK mysql.event OK mysql.func OK -mysql.general_log -Error : You can't use locks with log tables. -status : OK +mysql.general_log OK mysql.help_category OK mysql.help_keyword OK mysql.help_relation OK @@ -20,9 +18,7 @@ mysql.plugin OK mysql.proc OK mysql.procs_priv OK mysql.servers OK -mysql.slow_log -Error : You can't use locks with log tables. -status : OK +mysql.slow_log OK mysql.tables_priv OK mysql.time_zone OK mysql.time_zone_leap_second OK diff --git a/mysql-test/t/check.test b/mysql-test/t/check.test index ff23b352b5a..2234d2c680f 100644 --- a/mysql-test/t/check.test +++ b/mysql-test/t/check.test @@ -53,5 +53,29 @@ REPAIR TABLE t1; DROP TABLE t1; +--echo # +--echo # Bug#56422 CHECK TABLE run when the table is locked reports corruption +--echo # along with timeout +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1(a INT); +LOCK TABLE t1 WRITE; + +--echo # Connection con1 +connect(con1, localhost, root); +SET lock_wait_timeout= 1; +CHECK TABLE t1; + +--echo # Connection default +connection default; +UNLOCK TABLES; +DROP TABLE t1; +disconnect con1; + + # Wait till we reached the initial number of concurrent sessions --source include/wait_until_count_sessions.inc diff --git a/mysql-test/t/merge.test b/mysql-test/t/merge.test index 51c4ca51a29..34d0b5bf237 100644 --- a/mysql-test/t/merge.test +++ b/mysql-test/t/merge.test @@ -1734,6 +1734,84 @@ t2 WHERE b SOUNDS LIKE e AND d = 1; DROP TABLE t2, t1; +--echo # +--echo # Bug#46339 - crash on REPAIR TABLE merge table USE_FRM +--echo # +--disable_warnings +DROP TABLE IF EXISTS m1, t1; +--enable_warnings +# +# Test derived from a proposal of Shane Bester. +# +CREATE TABLE t1 (c1 INT) ENGINE=MYISAM; +CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1) INSERT_METHOD=LAST; +# +# REPAIR ... USE_FRM with LOCK TABLES. +# +LOCK TABLE m1 READ; +REPAIR TABLE m1 USE_FRM; +UNLOCK TABLES; +# +# REPAIR ... USE_FRM without LOCK TABLES. +# +# This statement crashed the server (Bug#46339). +# +REPAIR TABLE m1 USE_FRM; +# +DROP TABLE m1,t1; +# +# Test derived from a proposal of Matthias Leich. +# +# Base table is missing. +# +CREATE TABLE m1 (f1 BIGINT) ENGINE=MRG_MyISAM UNION(t1); +# +# This statement crashed the server (Bug#46339). +# +REPAIR TABLE m1 USE_FRM; +# +# Create base table. +# +CREATE TABLE t1 (f1 BIGINT) ENGINE = MyISAM; +# +# This statement crashed the server (Bug#46339). +# +REPAIR TABLE m1 USE_FRM; +# +# Normal repair as reference. +# +REPAIR TABLE m1; +# +# Cleanup. +# +DROP TABLE m1, t1; +# +# Same with temporary tables. +# +# Base table is missing. +# +CREATE TEMPORARY TABLE m1 (f1 BIGINT) ENGINE=MRG_MyISAM UNION(t1); +# +# This statement crashed the server (Bug#46339). +# +REPAIR TABLE m1 USE_FRM; +# +# Create base table. +# +CREATE TEMPORARY TABLE t1 (f1 BIGINT) ENGINE=MyISAM; +# +# This statement crashed the server (Bug#46339). +# +REPAIR TABLE m1 USE_FRM; +# +# Normal repair as reference. +# +REPAIR TABLE m1; +# +# Cleanup. +# +DROP TABLE m1, t1; + --echo End of 5.1 tests --echo # @@ -2668,6 +2746,36 @@ drop temporary table tmp; drop table t1, t2, t3, m1, m2; +--echo # +--echo # Bug#56494 Segfault in upgrade_shared_lock_to_exclusive() for +--echo # REPAIR of merge table +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1, t2, t_not_exists; +--enable_warnings + +CREATE TABLE t1(a INT); +ALTER TABLE t1 engine= MERGE UNION (t_not_exists); +# This caused the segfault +ANALYZE TABLE t1; +CHECK TABLE t1; +CHECKSUM TABLE t1; +OPTIMIZE TABLE t1; +REPAIR TABLE t1; + +# This caused an assert +REPAIR TABLE t1 USE_FRM; + +DROP TABLE t1; +CREATE TABLE t1(a INT); +CREATE TABLE t2(a INT) engine= MERGE UNION (t1); +# This caused an assert +REPAIR TABLE t2 USE_FRM; + +DROP TABLE t1, t2; + + --echo End of 6.0 tests --disable_result_log diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc index 1f96f1cf0e4..21a05a5baca 100644 --- a/sql/sql_admin.cc +++ b/sql/sql_admin.cc @@ -111,9 +111,6 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, table= &tmp_table; } - /* A MERGE table must not come here. */ - DBUG_ASSERT(table->file->ht->db_type != DB_TYPE_MRG_MYISAM); - /* REPAIR TABLE ... USE_FRM for temporary tables makes little sense. */ @@ -151,6 +148,9 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, if (!ext[0] || !ext[1]) goto end; // No data file + /* A MERGE table must not come here. */ + DBUG_ASSERT(table->file->ht->db_type != DB_TYPE_MRG_MYISAM); + // Name of data file strxmov(from, table->s->normalized_path.str, ext[1], NullS); if (!mysql_file_stat(key_file_misc, from, &stat_info, MYF(0))) @@ -231,6 +231,26 @@ end: } +/** + Check if a given error is something that could occur during + open_and_lock_tables() that does not indicate table corruption. + + @param sql_errno Error number to check. + + @retval TRUE Error does not indicate table corruption. + @retval FALSE Error could indicate table corruption. +*/ + +static inline bool table_not_corrupt_error(uint sql_errno) +{ + return (sql_errno == ER_NO_SUCH_TABLE || + sql_errno == ER_FILE_NOT_FOUND || + sql_errno == ER_LOCK_WAIT_TIMEOUT || + sql_errno == ER_LOCK_DEADLOCK || + sql_errno == ER_CANT_LOCK_LOG_TABLE || + sql_errno == ER_OPEN_AS_READONLY); +} + /* RETURN VALUES @@ -311,7 +331,13 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, lex->query_tables= table; lex->query_tables_last= &table->next_global; lex->query_tables_own_last= 0; - thd->no_warnings_for_error= no_warnings_for_error; + /* + Under locked tables, we know that the table can be opened, + so any errors opening the table are logical errors. + In these cases it makes sense to report them. + */ + if (!thd->locked_tables_mode) + thd->no_warnings_for_error= no_warnings_for_error; if (view_operator_func == NULL) table->required_type=FRMTYPE_TABLE; @@ -320,6 +346,14 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, table->next_global= save_next_global; table->next_local= save_next_local; thd->open_options&= ~extra_open_options; + + /* + If open_and_lock_tables() failed, close_thread_tables() will close + the table and table->table can therefore be invalid. + */ + if (open_error) + table->table= NULL; + /* Under locked tables, we know that the table can be opened, so any errors opening the table are logical errors. @@ -418,9 +452,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, ER_VIEW_CHECKSUM, ER(ER_VIEW_CHECKSUM)); if (thd->stmt_da->is_error() && - (thd->stmt_da->sql_errno() == ER_NO_SUCH_TABLE || - thd->stmt_da->sql_errno() == ER_FILE_NOT_FOUND)) - /* A missing table is just issued as a failed command */ + table_not_corrupt_error(thd->stmt_da->sql_errno())) result_code= HA_ADMIN_FAILED; else /* Default failure code is corrupt table */ diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index aa9d29f3d07..23a5f35d967 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -401,6 +401,7 @@ void init_update_queries(void) sql_command_flags[SQLCOM_REPAIR]= CF_WRITE_LOGS_COMMAND | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_OPTIMIZE]|= CF_WRITE_LOGS_COMMAND | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_ANALYZE]= CF_WRITE_LOGS_COMMAND | CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_CHECK]= CF_WRITE_LOGS_COMMAND | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_CREATE_USER]|= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_DROP_USER]|= CF_AUTO_COMMIT_TRANS; @@ -414,7 +415,6 @@ void init_update_queries(void) sql_command_flags[SQLCOM_FLUSH]= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_RESET]= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_CHECK]= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_CREATE_SERVER]= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_ALTER_SERVER]= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_DROP_SERVER]= CF_AUTO_COMMIT_TRANS; From 9b9299f8d53b8d67b2f39e2526343fd250cd9d3e Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Thu, 23 Sep 2010 11:13:21 +0200 Subject: [PATCH 18/24] Followup to Bug #56422 CHECK TABLE run when the table is locked reports corruption along with timeout This patch updates the result file for the parts.partition_special_innodb test case which was, by mistake, not updated in the original patch. --- mysql-test/suite/parts/r/partition_special_innodb.result | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql-test/suite/parts/r/partition_special_innodb.result b/mysql-test/suite/parts/r/partition_special_innodb.result index b8dffd61fbe..0738f45b012 100644 --- a/mysql-test/suite/parts/r/partition_special_innodb.result +++ b/mysql-test/suite/parts/r/partition_special_innodb.result @@ -242,7 +242,7 @@ ERROR HY000: Lock wait timeout exceeded; try restarting transaction CHECK TABLE t1; Table Op Msg_type Msg_text test.t1 check Error Lock wait timeout exceeded; try restarting transaction -test.t1 check error Corrupt +test.t1 check status Operation failed UNLOCK TABLES; DROP TABLE t1; CREATE TABLE t2 ( i INT NOT NULL AUTO_INCREMENT PRIMARY KEY, f INT ) From 11c4d8ba1f66a3173c51b6be678b9f2607adb4cc Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Fri, 24 Sep 2010 09:18:16 +0200 Subject: [PATCH 19/24] Bug #54920 Stored functions are allowed in HANDLER statements, but broken. Before this patch, it was allowed to use stored functions in HANDLER ... READ statements. The problem was that this functionality was not really supported by the code. Proper locking would for example not be performed, and it was also possible to break replication by having stored functions that performed updates. This patch disallows the use of stored functions in HANDLER ... READ. Any such statement will now give an ER_NOT_SUPPORTED_YET error. This is an incompatible change and should be reflected in the documentation. Test case added to handler_myisam/handler_innodb.test. --- mysql-test/include/handler.inc | 25 ++++++++++++++++++++++++- mysql-test/r/handler_innodb.result | 17 ++++++++++++++++- mysql-test/r/handler_myisam.result | 17 ++++++++++++++++- sql/sql_yacc.yy | 7 +++++++ 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/mysql-test/include/handler.inc b/mysql-test/include/handler.inc index 65e9e61d077..b86d5d9287f 100644 --- a/mysql-test/include/handler.inc +++ b/mysql-test/include/handler.inc @@ -1809,9 +1809,32 @@ CREATE TABLE t1(a INT); INSERT INTO t1 VALUES (1); HANDLER t1 OPEN; # This used to cause the assert ---error ER_NO_SUCH_TABLE +--error ER_NOT_SUPPORTED_YET HANDLER t1 READ FIRST WHERE f1() = 1; HANDLER t1 CLOSE; DROP FUNCTION f1; DROP TABLE t1; + + +--echo # +--echo # Bug#54920 Stored functions are allowed in HANDLER statements, +--echo # but broken. +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1; +DROP FUNCTION IF EXISTS f1; +--enable_warnings + +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1), (2); +CREATE FUNCTION f1() RETURNS INT RETURN 1; +HANDLER t1 OPEN; + +--error ER_NOT_SUPPORTED_YET +HANDLER t1 READ FIRST WHERE f1() = 1; + +HANDLER t1 CLOSE; +DROP FUNCTION f1; +DROP TABLE t1; diff --git a/mysql-test/r/handler_innodb.result b/mysql-test/r/handler_innodb.result index 121cfa89f1c..66914285733 100644 --- a/mysql-test/r/handler_innodb.result +++ b/mysql-test/r/handler_innodb.result @@ -1726,7 +1726,22 @@ CREATE TABLE t1(a INT); INSERT INTO t1 VALUES (1); HANDLER t1 OPEN; HANDLER t1 READ FIRST WHERE f1() = 1; -ERROR 42S02: Table 'test.t2' doesn't exist +ERROR 42000: This version of MySQL doesn't yet support 'stored functions in HANDLER ... READ' +HANDLER t1 CLOSE; +DROP FUNCTION f1; +DROP TABLE t1; +# +# Bug#54920 Stored functions are allowed in HANDLER statements, +# but broken. +# +DROP TABLE IF EXISTS t1; +DROP FUNCTION IF EXISTS f1; +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1), (2); +CREATE FUNCTION f1() RETURNS INT RETURN 1; +HANDLER t1 OPEN; +HANDLER t1 READ FIRST WHERE f1() = 1; +ERROR 42000: This version of MySQL doesn't yet support 'stored functions in HANDLER ... READ' HANDLER t1 CLOSE; DROP FUNCTION f1; DROP TABLE t1; diff --git a/mysql-test/r/handler_myisam.result b/mysql-test/r/handler_myisam.result index fd08fd12f15..d5d43ca0717 100644 --- a/mysql-test/r/handler_myisam.result +++ b/mysql-test/r/handler_myisam.result @@ -1722,7 +1722,22 @@ CREATE TABLE t1(a INT); INSERT INTO t1 VALUES (1); HANDLER t1 OPEN; HANDLER t1 READ FIRST WHERE f1() = 1; -ERROR 42S02: Table 'test.t2' doesn't exist +ERROR 42000: This version of MySQL doesn't yet support 'stored functions in HANDLER ... READ' +HANDLER t1 CLOSE; +DROP FUNCTION f1; +DROP TABLE t1; +# +# Bug#54920 Stored functions are allowed in HANDLER statements, +# but broken. +# +DROP TABLE IF EXISTS t1; +DROP FUNCTION IF EXISTS f1; +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1), (2); +CREATE FUNCTION f1() RETURNS INT RETURN 1; +HANDLER t1 OPEN; +HANDLER t1 READ FIRST WHERE f1() = 1; +ERROR 42000: This version of MySQL doesn't yet support 'stored functions in HANDLER ... READ' HANDLER t1 CLOSE; DROP FUNCTION f1; DROP TABLE t1; diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 2ee2d83f9d2..006a416d9e4 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -13245,6 +13245,13 @@ handler: handler_read_or_scan where_clause opt_limit_clause { Lex->expr_allows_subselect= TRUE; + /* Stored functions are not supported for HANDLER READ. */ + if (Lex->uses_stored_routines()) + { + my_error(ER_NOT_SUPPORTED_YET, MYF(0), + "stored functions in HANDLER ... READ"); + MYSQL_YYABORT; + } } ; From 71affc142d0b77dcfaa54c7d1d71666adf131b4e Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Fri, 24 Sep 2010 10:44:09 +0200 Subject: [PATCH 20/24] Bug #56678 Valgrind warnings from binlog.binlog_unsafe After the patch for Bug#54579, multi inserts done with INSERT DELAYED are binlogged as normal INSERT. During processing of the statement, a new query string without the DELAYED keyword is made. The problem was that this new string was incorrectly made when the INSERT DELAYED was part of a prepared statement - data was read outside the allocated buffer. The reason for this bug was that a pointer to the position of the DELAYED keyword inside the query string was stored when parsing the statement. This pointer was then later (at runtime) used (via pointer subtraction) to find the number of characters to skip when making a new query string without DELAYED. But when the statement was re-executed as part of a prepared statement, the original pointer would be invalid and the pointer subtraction would give a wrong/random result. This patch fixes the problem by instead storing the offsets from the beginning of the query string to the start and end of the DELAYED keyword. These values will not depend on the memory position of the query string at runtime and therefore not give wrong results when the statement is executed in a prepared statement. This bug was a regression introduced by the patch for Bug#54579. No test case added as this bug is already covered by the existing binlog.binlog_unsafe test case when running with valgrind. --- sql/sql_insert.cc | 12 +++++------- sql/sql_lex.h | 12 ++++++++---- sql/sql_yacc.yy | 10 ++++++++-- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index ad324fed4fe..cccb715bd5e 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -634,14 +634,12 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list) static int create_insert_stmt_from_insert_delayed(THD *thd, String *buf) { - /* Append the part of thd->query before "DELAYED" keyword */ - if (buf->append(thd->query(), - thd->lex->keyword_delayed_begin - thd->query())) + /* Make a copy of thd->query() and then remove the "DELAYED" keyword */ + if (buf->append(thd->query()) || + buf->replace(thd->lex->keyword_delayed_begin_offset, + thd->lex->keyword_delayed_end_offset - + thd->lex->keyword_delayed_begin_offset, 0)) return 1; - /* Append the part of thd->query after "DELAYED" keyword */ - if (buf->append(thd->lex->keyword_delayed_begin + 7)) - return 1; - return 0; } diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 1f4c9420102..2055b609ad5 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -2355,15 +2355,19 @@ struct LEX: public Query_tables_list This pointer is required to add possibly omitted DEFINER-clause to the DDL-statement before dumping it to the binlog. - keyword_delayed_begin points to the begin of the DELAYED keyword in - INSERT DELAYED statement. + keyword_delayed_begin_offset is the offset to the beginning of the DELAYED + keyword in INSERT DELAYED statement. keyword_delayed_end_offset is the + offset to the character right after the DELAYED keyword. */ union { const char *stmt_definition_begin; - const char *keyword_delayed_begin; + uint keyword_delayed_begin_offset; }; - const char *stmt_definition_end; + union { + const char *stmt_definition_end; + uint keyword_delayed_end_offset; + }; /** During name resolution search only in the table list given by diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 006a416d9e4..4bf211bf5c1 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -10447,7 +10447,10 @@ insert_lock_option: | LOW_PRIORITY { $$= TL_WRITE_LOW_PRIORITY; } | DELAYED_SYM { - Lex->keyword_delayed_begin= YYLIP->get_tok_start(); + Lex->keyword_delayed_begin_offset= (uint)(YYLIP->get_tok_start() - + YYTHD->query()); + Lex->keyword_delayed_end_offset= Lex->keyword_delayed_begin_offset + + YYLIP->yyLength() + 1; $$= TL_WRITE_DELAYED; } | HIGH_PRIORITY { $$= TL_WRITE; } @@ -10457,7 +10460,10 @@ replace_lock_option: opt_low_priority { $$= $1; } | DELAYED_SYM { - Lex->keyword_delayed_begin= YYLIP->get_tok_start(); + Lex->keyword_delayed_begin_offset= (uint)(YYLIP->get_tok_start() - + YYTHD->query()); + Lex->keyword_delayed_end_offset= Lex->keyword_delayed_begin_offset + + YYLIP->yyLength() + 1; $$= TL_WRITE_DELAYED; } ; From 92b247814a988e24b61a553a175af4ba6398e687 Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Tue, 28 Sep 2010 11:07:58 +0200 Subject: [PATCH 21/24] Bug #46165 server crash in dbug This crash occured if the same debug trace file was closed twice, leading to the same memory being free'd twice. This could occur if the "debug" server system variable refered to the same trace file in both global and session scope. Example of an order of events that would lead to a crash: 1) Enable debug tracing to a trace file (global scope) 2) Enable debug tracing to the same trace file (session scope) 3) Reset debug settings (global scope) 4) Reset debug settings (session scope) This caused a crash because the trace file was, by mistake, closed in 3), leading to the same memory being free'd twice when the file was closed again in 4). Internally, the debug settings are stored in a stack, with session settings (if any) on top and the global settings below. Each connection has its own stack. When a set of settings is changed, it must be determined if its debug trace file is to be closed. Before, this was done by only checking below on the settings stack. So if the global settings were changed, an existing debug trace file reference in session settings would be missed. This caused the file to be closed even if it was in use, leading to a crash later when it was closed again. This patch fixes the problem by preventing the trace file from being shared between global and session settings. If session debug settings are set without specifying a new trace file, stderr is used for output. This is a change in behaviour and should be reflected in the documentation. Test case added to variables.test. --- dbug/dbug.c | 10 ++++- mysql-test/r/variables.result | 44 ++++++++++++++++++++ mysql-test/t/variables.test | 75 +++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 2 deletions(-) diff --git a/dbug/dbug.c b/dbug/dbug.c index 4968d9e0568..5029bbb8d61 100644 --- a/dbug/dbug.c +++ b/dbug/dbug.c @@ -515,11 +515,16 @@ int DbugParse(CODE_STATE *cs, const char *control) stack->maxdepth= stack->next->maxdepth; stack->sub_level= stack->next->sub_level; strcpy(stack->name, stack->next->name); - stack->out_file= stack->next->out_file; stack->prof_file= stack->next->prof_file; if (stack->next == &init_settings) { - /* never share with the global parent - it can change under your feet */ + /* + Never share with the global parent - it can change under your feet. + + Reset out_file to stderr to prevent sharing of trace files between + global and session settings. + */ + stack->out_file= stderr; stack->functions= ListCopy(init_settings.functions); stack->p_functions= ListCopy(init_settings.p_functions); stack->keywords= ListCopy(init_settings.keywords); @@ -527,6 +532,7 @@ int DbugParse(CODE_STATE *cs, const char *control) } else { + stack->out_file= stack->next->out_file; stack->functions= stack->next->functions; stack->p_functions= stack->next->p_functions; stack->keywords= stack->next->keywords; diff --git a/mysql-test/r/variables.result b/mysql-test/r/variables.result index be81afe1a43..971e4caf516 100644 --- a/mysql-test/r/variables.result +++ b/mysql-test/r/variables.result @@ -1699,3 +1699,47 @@ set @@session.autocommit=t1_min(), @@session.autocommit=t1_max(), drop table t1; drop function t1_min; drop function t1_max; +# +# Bug#46165 server crash in dbug +# +SET @old_globaldebug = @@global.debug; +SET @old_sessiondebug= @@session.debug; +# Test 1 - Bug test case, single connection +SET GLOBAL debug= '+O,../../log/bug46165.1.trace'; +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +SET SESSION debug= ''; +# Test 2 - Bug test case, two connections +# Connection default +SET GLOBAL debug= '+O,../../log/bug46165.2.trace'; +SET SESSION debug= '-d:-t:-i'; +# Connection con1 +SET GLOBAL debug= ''; +# Connection default +SET SESSION debug= ''; +# Connection con1 +# Connection default +SET GLOBAL debug= ''; +# Test 3 - Active session trace file on disconnect +# Connection con1 +SET GLOBAL debug= '+O,../../log/bug46165.3.trace'; +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +# Test 4 - Active session trace file on two connections +# Connection default +SET GLOBAL debug= '+O,../../log/bug46165.4.trace'; +SET SESSION debug= '-d:-t:-i'; +# Connection con1 +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +SET SESSION debug= ''; +# Connection default +SET SESSION debug= ''; +# Connection con1 +# Connection default +# Test 5 - Different trace files +SET SESSION debug= '+O,../../log/bug46165.5.trace'; +SET SESSION debug= '+O,../../log/bug46165.6.trace'; +SET SESSION debug= '-O'; +SET GLOBAL debug= @old_globaldebug; +SET SESSION debug= @old_sessiondebug; diff --git a/mysql-test/t/variables.test b/mysql-test/t/variables.test index 75099523062..0209fb00a77 100644 --- a/mysql-test/t/variables.test +++ b/mysql-test/t/variables.test @@ -1432,3 +1432,78 @@ drop function t1_max; ########################################################################### + + +--echo # +--echo # Bug#46165 server crash in dbug +--echo # + +SET @old_globaldebug = @@global.debug; +SET @old_sessiondebug= @@session.debug; + +--echo # Test 1 - Bug test case, single connection +SET GLOBAL debug= '+O,../../log/bug46165.1.trace'; +SET SESSION debug= '-d:-t:-i'; + +SET GLOBAL debug= ''; +SET SESSION debug= ''; + +--echo # Test 2 - Bug test case, two connections +--echo # Connection default +connection default; +SET GLOBAL debug= '+O,../../log/bug46165.2.trace'; +SET SESSION debug= '-d:-t:-i'; + +--echo # Connection con1 +connect (con1, localhost, root); +SET GLOBAL debug= ''; + +--echo # Connection default +connection default; +SET SESSION debug= ''; +--echo # Connection con1 +connection con1; +disconnect con1; +--source include/wait_until_disconnected.inc +--echo # Connection default +connection default; +SET GLOBAL debug= ''; + +--echo # Test 3 - Active session trace file on disconnect +--echo # Connection con1 +connect (con1, localhost, root); +SET GLOBAL debug= '+O,../../log/bug46165.3.trace'; +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +disconnect con1; +--source include/wait_until_disconnected.inc + +--echo # Test 4 - Active session trace file on two connections +--echo # Connection default +connection default; +SET GLOBAL debug= '+O,../../log/bug46165.4.trace'; +SET SESSION debug= '-d:-t:-i'; + +--echo # Connection con1 +connect (con1, localhost, root); +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +SET SESSION debug= ''; + +--echo # Connection default +connection default; +SET SESSION debug= ''; +--echo # Connection con1 +connection con1; +disconnect con1; +--source include/wait_until_disconnected.inc +--echo # Connection default +connection default; + +--echo # Test 5 - Different trace files +SET SESSION debug= '+O,../../log/bug46165.5.trace'; +SET SESSION debug= '+O,../../log/bug46165.6.trace'; +SET SESSION debug= '-O'; + +SET GLOBAL debug= @old_globaldebug; +SET SESSION debug= @old_sessiondebug; From 4228b8bbdd93cc905b75432c1e9d23316f2430e6 Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 29 Sep 2010 10:07:56 +0200 Subject: [PATCH 22/24] Followup to Bug#46165 server crash in dbug This patch moves the regression test from variables.test to variables_debug.test as the debug system variable is not available on release builds. --- mysql-test/r/variables.result | 44 ----------------- mysql-test/r/variables_debug.result | 44 +++++++++++++++++ mysql-test/t/variables.test | 75 ----------------------------- mysql-test/t/variables_debug.test | 75 +++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 119 deletions(-) diff --git a/mysql-test/r/variables.result b/mysql-test/r/variables.result index 971e4caf516..be81afe1a43 100644 --- a/mysql-test/r/variables.result +++ b/mysql-test/r/variables.result @@ -1699,47 +1699,3 @@ set @@session.autocommit=t1_min(), @@session.autocommit=t1_max(), drop table t1; drop function t1_min; drop function t1_max; -# -# Bug#46165 server crash in dbug -# -SET @old_globaldebug = @@global.debug; -SET @old_sessiondebug= @@session.debug; -# Test 1 - Bug test case, single connection -SET GLOBAL debug= '+O,../../log/bug46165.1.trace'; -SET SESSION debug= '-d:-t:-i'; -SET GLOBAL debug= ''; -SET SESSION debug= ''; -# Test 2 - Bug test case, two connections -# Connection default -SET GLOBAL debug= '+O,../../log/bug46165.2.trace'; -SET SESSION debug= '-d:-t:-i'; -# Connection con1 -SET GLOBAL debug= ''; -# Connection default -SET SESSION debug= ''; -# Connection con1 -# Connection default -SET GLOBAL debug= ''; -# Test 3 - Active session trace file on disconnect -# Connection con1 -SET GLOBAL debug= '+O,../../log/bug46165.3.trace'; -SET SESSION debug= '-d:-t:-i'; -SET GLOBAL debug= ''; -# Test 4 - Active session trace file on two connections -# Connection default -SET GLOBAL debug= '+O,../../log/bug46165.4.trace'; -SET SESSION debug= '-d:-t:-i'; -# Connection con1 -SET SESSION debug= '-d:-t:-i'; -SET GLOBAL debug= ''; -SET SESSION debug= ''; -# Connection default -SET SESSION debug= ''; -# Connection con1 -# Connection default -# Test 5 - Different trace files -SET SESSION debug= '+O,../../log/bug46165.5.trace'; -SET SESSION debug= '+O,../../log/bug46165.6.trace'; -SET SESSION debug= '-O'; -SET GLOBAL debug= @old_globaldebug; -SET SESSION debug= @old_sessiondebug; diff --git a/mysql-test/r/variables_debug.result b/mysql-test/r/variables_debug.result index a02d285b15b..8abdbdaed12 100644 --- a/mysql-test/r/variables_debug.result +++ b/mysql-test/r/variables_debug.result @@ -32,3 +32,47 @@ SELECT @@global.debug; SET GLOBAL debug=@old_debug; End of 5.1 tests +# +# Bug#46165 server crash in dbug +# +SET @old_globaldebug = @@global.debug; +SET @old_sessiondebug= @@session.debug; +# Test 1 - Bug test case, single connection +SET GLOBAL debug= '+O,../../log/bug46165.1.trace'; +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +SET SESSION debug= ''; +# Test 2 - Bug test case, two connections +# Connection default +SET GLOBAL debug= '+O,../../log/bug46165.2.trace'; +SET SESSION debug= '-d:-t:-i'; +# Connection con1 +SET GLOBAL debug= ''; +# Connection default +SET SESSION debug= ''; +# Connection con1 +# Connection default +SET GLOBAL debug= ''; +# Test 3 - Active session trace file on disconnect +# Connection con1 +SET GLOBAL debug= '+O,../../log/bug46165.3.trace'; +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +# Test 4 - Active session trace file on two connections +# Connection default +SET GLOBAL debug= '+O,../../log/bug46165.4.trace'; +SET SESSION debug= '-d:-t:-i'; +# Connection con1 +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +SET SESSION debug= ''; +# Connection default +SET SESSION debug= ''; +# Connection con1 +# Connection default +# Test 5 - Different trace files +SET SESSION debug= '+O,../../log/bug46165.5.trace'; +SET SESSION debug= '+O,../../log/bug46165.6.trace'; +SET SESSION debug= '-O'; +SET GLOBAL debug= @old_globaldebug; +SET SESSION debug= @old_sessiondebug; diff --git a/mysql-test/t/variables.test b/mysql-test/t/variables.test index 0209fb00a77..75099523062 100644 --- a/mysql-test/t/variables.test +++ b/mysql-test/t/variables.test @@ -1432,78 +1432,3 @@ drop function t1_max; ########################################################################### - - ---echo # ---echo # Bug#46165 server crash in dbug ---echo # - -SET @old_globaldebug = @@global.debug; -SET @old_sessiondebug= @@session.debug; - ---echo # Test 1 - Bug test case, single connection -SET GLOBAL debug= '+O,../../log/bug46165.1.trace'; -SET SESSION debug= '-d:-t:-i'; - -SET GLOBAL debug= ''; -SET SESSION debug= ''; - ---echo # Test 2 - Bug test case, two connections ---echo # Connection default -connection default; -SET GLOBAL debug= '+O,../../log/bug46165.2.trace'; -SET SESSION debug= '-d:-t:-i'; - ---echo # Connection con1 -connect (con1, localhost, root); -SET GLOBAL debug= ''; - ---echo # Connection default -connection default; -SET SESSION debug= ''; ---echo # Connection con1 -connection con1; -disconnect con1; ---source include/wait_until_disconnected.inc ---echo # Connection default -connection default; -SET GLOBAL debug= ''; - ---echo # Test 3 - Active session trace file on disconnect ---echo # Connection con1 -connect (con1, localhost, root); -SET GLOBAL debug= '+O,../../log/bug46165.3.trace'; -SET SESSION debug= '-d:-t:-i'; -SET GLOBAL debug= ''; -disconnect con1; ---source include/wait_until_disconnected.inc - ---echo # Test 4 - Active session trace file on two connections ---echo # Connection default -connection default; -SET GLOBAL debug= '+O,../../log/bug46165.4.trace'; -SET SESSION debug= '-d:-t:-i'; - ---echo # Connection con1 -connect (con1, localhost, root); -SET SESSION debug= '-d:-t:-i'; -SET GLOBAL debug= ''; -SET SESSION debug= ''; - ---echo # Connection default -connection default; -SET SESSION debug= ''; ---echo # Connection con1 -connection con1; -disconnect con1; ---source include/wait_until_disconnected.inc ---echo # Connection default -connection default; - ---echo # Test 5 - Different trace files -SET SESSION debug= '+O,../../log/bug46165.5.trace'; -SET SESSION debug= '+O,../../log/bug46165.6.trace'; -SET SESSION debug= '-O'; - -SET GLOBAL debug= @old_globaldebug; -SET SESSION debug= @old_sessiondebug; diff --git a/mysql-test/t/variables_debug.test b/mysql-test/t/variables_debug.test index 8fa2124137a..63c34acc876 100644 --- a/mysql-test/t/variables_debug.test +++ b/mysql-test/t/variables_debug.test @@ -36,3 +36,78 @@ SELECT @@global.debug; SET GLOBAL debug=@old_debug; --echo End of 5.1 tests + + +--echo # +--echo # Bug#46165 server crash in dbug +--echo # + +SET @old_globaldebug = @@global.debug; +SET @old_sessiondebug= @@session.debug; + +--echo # Test 1 - Bug test case, single connection +SET GLOBAL debug= '+O,../../log/bug46165.1.trace'; +SET SESSION debug= '-d:-t:-i'; + +SET GLOBAL debug= ''; +SET SESSION debug= ''; + +--echo # Test 2 - Bug test case, two connections +--echo # Connection default +connection default; +SET GLOBAL debug= '+O,../../log/bug46165.2.trace'; +SET SESSION debug= '-d:-t:-i'; + +--echo # Connection con1 +connect (con1, localhost, root); +SET GLOBAL debug= ''; + +--echo # Connection default +connection default; +SET SESSION debug= ''; +--echo # Connection con1 +connection con1; +disconnect con1; +--source include/wait_until_disconnected.inc +--echo # Connection default +connection default; +SET GLOBAL debug= ''; + +--echo # Test 3 - Active session trace file on disconnect +--echo # Connection con1 +connect (con1, localhost, root); +SET GLOBAL debug= '+O,../../log/bug46165.3.trace'; +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +disconnect con1; +--source include/wait_until_disconnected.inc + +--echo # Test 4 - Active session trace file on two connections +--echo # Connection default +connection default; +SET GLOBAL debug= '+O,../../log/bug46165.4.trace'; +SET SESSION debug= '-d:-t:-i'; + +--echo # Connection con1 +connect (con1, localhost, root); +SET SESSION debug= '-d:-t:-i'; +SET GLOBAL debug= ''; +SET SESSION debug= ''; + +--echo # Connection default +connection default; +SET SESSION debug= ''; +--echo # Connection con1 +connection con1; +disconnect con1; +--source include/wait_until_disconnected.inc +--echo # Connection default +connection default; + +--echo # Test 5 - Different trace files +SET SESSION debug= '+O,../../log/bug46165.5.trace'; +SET SESSION debug= '+O,../../log/bug46165.6.trace'; +SET SESSION debug= '-O'; + +SET GLOBAL debug= @old_globaldebug; +SET SESSION debug= @old_sessiondebug; From 49406b97dfe1d54709eae4aa1234d0071b6667ad Mon Sep 17 00:00:00 2001 From: Dmitry Lenev Date: Wed, 29 Sep 2010 16:09:07 +0400 Subject: [PATCH 23/24] A better fix for bug #56405 "Deadlock in the MDL deadlock detector" that doesn't introduce bug #56715 "Concurrent transactions + FLUSH result in sporadical unwarranted deadlock errors". Deadlock could have occurred when workload containing a mix of DML, DDL and FLUSH TABLES statements affecting the same set of tables was executed in a heavily concurrent environment. This deadlock occurred when several connections tried to perform deadlock detection in the metadata locking subsystem. The first connection started traversing wait-for graph, encountered a sub-graph representing a wait for flush, acquired LOCK_open and dived into sub-graph inspection. Then it encountered sub-graph corresponding to wait for metadata lock and blocked while trying to acquire a rd-lock on MDL_lock::m_rwlock, since some,other thread had a wr-lock on it. When this wr-lock was released it could have happened (if there was another pending wr-lock against this rwlock) that the rd-lock from the first connection was left unsatisfied but at the same time the new rd-lock request from the second connection sneaked in and was satisfied (for this to be possible the second rd-request should come exactly after the wr-lock is released but before pending the wr-lock manages to grab rwlock, which is possible both on Linux and in our own rwlock implementation). If this second connection continued traversing the wait-for graph and encountered a sub-graph representing a wait for flush it tried to acquire LOCK_open and thus the deadlock was created. The previous patch tried to workaround this problem by not allowing the deadlock detector to lock LOCK_open mutex if some other thread doing deadlock detection already owns it and current search depth is greater than 0. Instead deadlock was reported. As a result it has introduced bug #56715. This patch solves this problem in a different way. It introduces a new rw_pr_lock_t implementation to be used by MDL subsystem instead of one based on Linux rwlocks or our own rwlock implementation. This new implementation never allows situation in which an rwlock is rd-locked and there is a blocked pending rd-lock. Thus the situation which has caused this bug becomes impossible with this implementation. Due to fact that this implementation is optimized for wr-lock/unlock scenario which is most common in the MDL subsystem it doesn't introduce noticeable performance regressions in sysbench tests. Moreover it significantly improves situation for POINT_SELECT test when many connections are used. No test case is provided as this bug is very hard to repeat in MTR environment but is repeatable with the help of RQG tests. This patch also doesn't include a test for bug #56715 "Concurrent transactions + FLUSH result in sporadical unwarranted deadlock errors" as it takes too much time to be run as part of normal test-suite runs. --- config.h.cmake | 1 - configure.cmake | 1 - configure.in | 2 +- include/my_pthread.h | 104 ++++++++++++++-------- include/mysql/psi/mysql_thread.h | 88 ------------------- mysys/thr_rwlock.c | 145 +++++++++++++++++++++++++------ 6 files changed, 188 insertions(+), 153 deletions(-) diff --git a/config.h.cmake b/config.h.cmake index c484edb65a5..7047cc6bbf5 100644 --- a/config.h.cmake +++ b/config.h.cmake @@ -220,7 +220,6 @@ #cmakedefine HAVE_PTHREAD_KEY_DELETE 1 #cmakedefine HAVE_PTHREAD_KILL 1 #cmakedefine HAVE_PTHREAD_RWLOCK_RDLOCK 1 -#cmakedefine HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP 1 #cmakedefine HAVE_PTHREAD_SETPRIO_NP 1 #cmakedefine HAVE_PTHREAD_SETSCHEDPARAM 1 #cmakedefine HAVE_PTHREAD_SIGMASK 1 diff --git a/configure.cmake b/configure.cmake index c2e1dc4647b..91c39fc5b09 100644 --- a/configure.cmake +++ b/configure.cmake @@ -346,7 +346,6 @@ CHECK_FUNCTION_EXISTS (pthread_condattr_setclock HAVE_PTHREAD_CONDATTR_SETCLOCK) CHECK_FUNCTION_EXISTS (pthread_init HAVE_PTHREAD_INIT) CHECK_FUNCTION_EXISTS (pthread_key_delete HAVE_PTHREAD_KEY_DELETE) CHECK_FUNCTION_EXISTS (pthread_rwlock_rdlock HAVE_PTHREAD_RWLOCK_RDLOCK) -CHECK_FUNCTION_EXISTS (pthread_rwlockattr_setkind_np HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP) CHECK_FUNCTION_EXISTS (pthread_sigmask HAVE_PTHREAD_SIGMASK) CHECK_FUNCTION_EXISTS (pthread_threadmask HAVE_PTHREAD_THREADMASK) CHECK_FUNCTION_EXISTS (pthread_yield_np HAVE_PTHREAD_YIELD_NP) diff --git a/configure.in b/configure.in index 1f56d958a7f..48385ba2491 100644 --- a/configure.in +++ b/configure.in @@ -2169,7 +2169,7 @@ AC_CHECK_FUNCS(alarm bfill bmove bsearch bzero \ mkstemp mlockall perror poll pread pthread_attr_create mmap mmap64 getpagesize \ pthread_attr_getstacksize pthread_attr_setstacksize pthread_condattr_create \ pthread_getsequence_np pthread_key_delete pthread_rwlock_rdlock \ - pthread_rwlockattr_setkind_np pthread_sigmask \ + pthread_sigmask \ readlink realpath rename rint rwlock_init setupterm \ shmget shmat shmdt shmctl sigaction sigemptyset sigaddset \ sighold sigset sigthreadmask port_create sleep thr_yield \ diff --git a/include/my_pthread.h b/include/my_pthread.h index 5cf181596ad..27ab5ba23fe 100644 --- a/include/my_pthread.h +++ b/include/my_pthread.h @@ -594,7 +594,7 @@ int my_pthread_fastmutex_lock(my_pthread_fastmutex_t *mp); /* Use our own version of read/write locks */ #define NEED_MY_RW_LOCK 1 #define rw_lock_t my_rw_lock_t -#define my_rwlock_init(A,B) my_rw_init((A), 0) +#define my_rwlock_init(A,B) my_rw_init((A)) #define rw_rdlock(A) my_rw_rdlock((A)) #define rw_wrlock(A) my_rw_wrlock((A)) #define rw_tryrdlock(A) my_rw_tryrdlock((A)) @@ -606,49 +606,82 @@ int my_pthread_fastmutex_lock(my_pthread_fastmutex_t *mp); #endif /* USE_MUTEX_INSTEAD_OF_RW_LOCKS */ -/* - Portable read-write locks which prefer readers. +/** + Portable implementation of special type of read-write locks. - Required by some algorithms in order to provide correctness. + These locks have two properties which are unusual for rwlocks: + 1) They "prefer readers" in the sense that they do not allow + situations in which rwlock is rd-locked and there is a + pending rd-lock which is blocked (e.g. due to pending + request for wr-lock). + This is a stronger guarantee than one which is provided for + PTHREAD_RWLOCK_PREFER_READER_NP rwlocks in Linux. + MDL subsystem deadlock detector relies on this property for + its correctness. + 2) They are optimized for uncontended wr-lock/unlock case. + This is scenario in which they are most oftenly used + within MDL subsystem. Optimizing for it gives significant + performance improvements in some of tests involving many + connections. + + Another important requirement imposed on this type of rwlock + by the MDL subsystem is that it should be OK to destroy rwlock + object which is in unlocked state even though some threads might + have not yet fully left unlock operation for it (of course there + is an external guarantee that no thread will try to lock rwlock + which is destroyed). + Putting it another way the unlock operation should not access + rwlock data after changing its state to unlocked. + + TODO/FIXME: We should consider alleviating this requirement as + it blocks us from doing certain performance optimizations. */ -#if defined(HAVE_PTHREAD_RWLOCK_RDLOCK) && defined(HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP) -/* - On systems which have a way to specify that readers should - be preferred through attribute mechanism (e.g. Linux) we use - system implementation of read/write locks. -*/ -#define rw_pr_lock_t pthread_rwlock_t +typedef struct st_rw_pr_lock_t { + /** + Lock which protects the structure. + Also held for the duration of wr-lock. + */ + pthread_mutex_t lock; + /** + Condition variable which is used to wake-up + writers waiting for readers to go away. + */ + pthread_cond_t no_active_readers; + /** Number of active readers. */ + uint active_readers; + /** Number of writers waiting for readers to go away. */ + uint writers_waiting_readers; + /** Indicates whether there is an active writer. */ + my_bool active_writer; +#ifdef SAFE_MUTEX + /** Thread holding wr-lock (for debug purposes only). */ + pthread_t writer_thread; +#endif +} rw_pr_lock_t; + extern int rw_pr_init(rw_pr_lock_t *); -#define rw_pr_rdlock(A) pthread_rwlock_rdlock(A) -#define rw_pr_wrlock(A) pthread_rwlock_wrlock(A) -#define rw_pr_tryrdlock(A) pthread_rwlock_tryrdlock(A) -#define rw_pr_trywrlock(A) pthread_rwlock_trywrlock(A) -#define rw_pr_unlock(A) pthread_rwlock_unlock(A) -#define rw_pr_destroy(A) pthread_rwlock_destroy(A) +extern int rw_pr_rdlock(rw_pr_lock_t *); +extern int rw_pr_wrlock(rw_pr_lock_t *); +extern int rw_pr_unlock(rw_pr_lock_t *); +extern int rw_pr_destroy(rw_pr_lock_t *); +#ifdef SAFE_MUTEX +#define rw_pr_lock_assert_write_owner(A) \ + DBUG_ASSERT((A)->active_writer && pthread_equal(pthread_self(), \ + (A)->writer_thread)) +#define rw_pr_lock_assert_not_write_owner(A) \ + DBUG_ASSERT(! (A)->active_writer || ! pthread_equal(pthread_self(), \ + (A)->writer_thread)) +#else #define rw_pr_lock_assert_write_owner(A) #define rw_pr_lock_assert_not_write_owner(A) -#else -/* Otherwise we have to use our own implementation of read/write locks. */ -#define NEED_MY_RW_LOCK 1 -struct st_my_rw_lock_t; -#define rw_pr_lock_t my_rw_lock_t -extern int rw_pr_init(struct st_my_rw_lock_t *); -#define rw_pr_rdlock(A) my_rw_rdlock((A)) -#define rw_pr_wrlock(A) my_rw_wrlock((A)) -#define rw_pr_tryrdlock(A) my_rw_tryrdlock((A)) -#define rw_pr_trywrlock(A) my_rw_trywrlock((A)) -#define rw_pr_unlock(A) my_rw_unlock((A)) -#define rw_pr_destroy(A) my_rw_destroy((A)) -#define rw_pr_lock_assert_write_owner(A) my_rw_lock_assert_write_owner((A)) -#define rw_pr_lock_assert_not_write_owner(A) my_rw_lock_assert_not_write_owner((A)) -#endif /* defined(HAVE_PTHREAD_RWLOCK_RDLOCK) && defined(HAVE_PTHREAD_RWLOCKATTR_SETKIND_NP) */ +#endif /* SAFE_MUTEX */ #ifdef NEED_MY_RW_LOCK /* - On systems which don't support native read/write locks, or don't support - read/write locks which prefer readers we have to use own implementation. + On systems which don't support native read/write locks we have + to use own implementation. */ typedef struct st_my_rw_lock_t { pthread_mutex_t lock; /* lock for structure */ @@ -656,13 +689,12 @@ typedef struct st_my_rw_lock_t { pthread_cond_t writers; /* waiting writers */ int state; /* -1:writer,0:free,>0:readers */ int waiters; /* number of waiting writers */ - my_bool prefer_readers; #ifdef SAFE_MUTEX pthread_t write_thread; #endif } my_rw_lock_t; -extern int my_rw_init(my_rw_lock_t *, my_bool *); +extern int my_rw_init(my_rw_lock_t *); extern int my_rw_destroy(my_rw_lock_t *); extern int my_rw_rdlock(my_rw_lock_t *); extern int my_rw_wrlock(my_rw_lock_t *); diff --git a/include/mysql/psi/mysql_thread.h b/include/mysql/psi/mysql_thread.h index 60b4f5d6ef4..5b8ea3dc5dc 100644 --- a/include/mysql/psi/mysql_thread.h +++ b/include/mysql/psi/mysql_thread.h @@ -141,9 +141,7 @@ typedef struct st_mysql_rwlock mysql_rwlock_t; @c mysql_prlock_t is a drop-in replacement for @c rw_pr_lock_t. @sa mysql_prlock_init @sa mysql_prlock_rdlock - @sa mysql_prlock_tryrdlock @sa mysql_prlock_wrlock - @sa mysql_prlock_trywrlock @sa mysql_prlock_unlock @sa mysql_prlock_destroy */ @@ -420,20 +418,6 @@ typedef struct st_mysql_cond mysql_cond_t; inline_mysql_rwlock_tryrdlock(RW) #endif -/** - @def mysql_prlock_tryrdlock(RW) - Instrumented rw_pr_tryrdlock. - @c mysql_prlock_tryrdlock is a drop-in replacement - for @c rw_pr_tryrdlock. -*/ -#ifdef HAVE_PSI_INTERFACE - #define mysql_prlock_tryrdlock(RW) \ - inline_mysql_prlock_tryrdlock(RW, __FILE__, __LINE__) -#else - #define mysql_prlock_tryrdlock(RW) \ - inline_mysql_prlock_tryrdlock(RW) -#endif - /** @def mysql_rwlock_trywrlock(RW) Instrumented rwlock_trywrlock. @@ -448,20 +432,6 @@ typedef struct st_mysql_cond mysql_cond_t; inline_mysql_rwlock_trywrlock(RW) #endif -/** - @def mysql_prlock_trywrlock(RW) - Instrumented rw_pr_trywrlock. - @c mysql_prlock_trywrlock is a drop-in replacement - for @c rw_pr_trywrlock. -*/ -#ifdef HAVE_PSI_INTERFACE - #define mysql_prlock_trywrlock(RW) \ - inline_mysql_prlock_trywrlock(RW, __FILE__, __LINE__) -#else - #define mysql_prlock_trywrlock(RW) \ - inline_mysql_prlock_trywrlock(RW) -#endif - /** @def mysql_rwlock_unlock(RW) Instrumented rwlock_unlock. @@ -905,35 +875,6 @@ static inline int inline_mysql_rwlock_tryrdlock( return result; } -#ifndef DISABLE_MYSQL_PRLOCK_H -static inline int inline_mysql_prlock_tryrdlock( - mysql_prlock_t *that -#ifdef HAVE_PSI_INTERFACE - , const char *src_file, uint src_line -#endif - ) -{ - int result; -#ifdef HAVE_PSI_INTERFACE - struct PSI_rwlock_locker *locker= NULL; - PSI_rwlock_locker_state state; - if (likely(PSI_server && that->m_psi)) - { - locker= PSI_server->get_thread_rwlock_locker(&state, that->m_psi, - PSI_RWLOCK_TRYREADLOCK); - if (likely(locker != NULL)) - PSI_server->start_rwlock_rdwait(locker, src_file, src_line); - } -#endif - result= rw_pr_tryrdlock(&that->m_prlock); -#ifdef HAVE_PSI_INTERFACE - if (likely(locker != NULL)) - PSI_server->end_rwlock_rdwait(locker, result); -#endif - return result; -} -#endif - static inline int inline_mysql_rwlock_trywrlock( mysql_rwlock_t *that #ifdef HAVE_PSI_INTERFACE @@ -961,35 +902,6 @@ static inline int inline_mysql_rwlock_trywrlock( return result; } -#ifndef DISABLE_MYSQL_PRLOCK_H -static inline int inline_mysql_prlock_trywrlock( - mysql_prlock_t *that -#ifdef HAVE_PSI_INTERFACE - , const char *src_file, uint src_line -#endif - ) -{ - int result; -#ifdef HAVE_PSI_INTERFACE - struct PSI_rwlock_locker *locker= NULL; - PSI_rwlock_locker_state state; - if (likely(PSI_server && that->m_psi)) - { - locker= PSI_server->get_thread_rwlock_locker(&state, that->m_psi, - PSI_RWLOCK_TRYWRITELOCK); - if (likely(locker != NULL)) - PSI_server->start_rwlock_wrwait(locker, src_file, src_line); - } -#endif - result= rw_pr_trywrlock(&that->m_prlock); -#ifdef HAVE_PSI_INTERFACE - if (likely(locker != NULL)) - PSI_server->end_rwlock_wrwait(locker, result); -#endif - return result; -} -#endif - static inline int inline_mysql_rwlock_unlock( mysql_rwlock_t *that) { diff --git a/mysys/thr_rwlock.c b/mysys/thr_rwlock.c index ecd12849822..218cfb251c8 100644 --- a/mysys/thr_rwlock.c +++ b/mysys/thr_rwlock.c @@ -59,7 +59,7 @@ * Mountain View, California 94043 */ -int my_rw_init(my_rw_lock_t *rwp, my_bool *prefer_readers_attr) +int my_rw_init(my_rw_lock_t *rwp) { pthread_condattr_t cond_attr; @@ -74,8 +74,6 @@ int my_rw_init(my_rw_lock_t *rwp, my_bool *prefer_readers_attr) #ifdef SAFE_MUTEX rwp->write_thread = 0; #endif - /* If attribute argument is NULL use default value - prefer writers. */ - rwp->prefer_readers= prefer_readers_attr ? *prefer_readers_attr : FALSE; return(0); } @@ -96,8 +94,7 @@ int my_rw_rdlock(my_rw_lock_t *rwp) pthread_mutex_lock(&rwp->lock); /* active or queued writers */ - while (( rwp->state < 0 ) || - (rwp->waiters && ! rwp->prefer_readers)) + while (( rwp->state < 0 ) || rwp->waiters) pthread_cond_wait( &rwp->readers, &rwp->lock); rwp->state++; @@ -109,8 +106,7 @@ int my_rw_tryrdlock(my_rw_lock_t *rwp) { int res; pthread_mutex_lock(&rwp->lock); - if ((rwp->state < 0 ) || - (rwp->waiters && ! rwp->prefer_readers)) + if ((rwp->state < 0 ) || rwp->waiters) res= EBUSY; /* Can't get lock */ else { @@ -192,30 +188,127 @@ int my_rw_unlock(my_rw_lock_t *rwp) return(0); } +#endif /* defined(NEED_MY_RW_LOCK) */ -int rw_pr_init(struct st_my_rw_lock_t *rwlock) -{ - my_bool prefer_readers_attr= TRUE; - return my_rw_init(rwlock, &prefer_readers_attr); -} - -#else - -/* - We are on system which has native read/write locks which support - preferring of readers. -*/ int rw_pr_init(rw_pr_lock_t *rwlock) { - pthread_rwlockattr_t rwlock_attr; - - pthread_rwlockattr_init(&rwlock_attr); - pthread_rwlockattr_setkind_np(&rwlock_attr, PTHREAD_RWLOCK_PREFER_READER_NP); - pthread_rwlock_init(rwlock, NULL); - pthread_rwlockattr_destroy(&rwlock_attr); + pthread_mutex_init(&rwlock->lock, NULL); + pthread_cond_init(&rwlock->no_active_readers, NULL); + rwlock->active_readers= 0; + rwlock->writers_waiting_readers= 0; + rwlock->active_writer= FALSE; +#ifdef SAFE_MUTEX + rwlock->writer_thread= 0; +#endif return 0; } -#endif /* defined(NEED_MY_RW_LOCK) */ + +int rw_pr_destroy(rw_pr_lock_t *rwlock) +{ + pthread_cond_destroy(&rwlock->no_active_readers); + pthread_mutex_destroy(&rwlock->lock); + return 0; +} + + +int rw_pr_rdlock(rw_pr_lock_t *rwlock) +{ + pthread_mutex_lock(&rwlock->lock); + /* + The fact that we were able to acquire 'lock' mutex means + that there are no active writers and we can acquire rd-lock. + Increment active readers counter to prevent requests for + wr-lock from succeeding and unlock mutex. + */ + rwlock->active_readers++; + pthread_mutex_unlock(&rwlock->lock); + return 0; +} + + +int rw_pr_wrlock(rw_pr_lock_t *rwlock) +{ + pthread_mutex_lock(&rwlock->lock); + + if (rwlock->active_readers != 0) + { + /* There are active readers. We have to wait until they are gone. */ + rwlock->writers_waiting_readers++; + + while (rwlock->active_readers != 0) + pthread_cond_wait(&rwlock->no_active_readers, &rwlock->lock); + + rwlock->writers_waiting_readers--; + } + + /* + We own 'lock' mutex so there is no active writers. + Also there are no active readers. + This means that we can grant wr-lock. + Not releasing 'lock' mutex until unlock will block + both requests for rd and wr-locks. + Set 'active_writer' flag to simplify unlock. + + Thanks to the fact wr-lock/unlock in the absence of + contention from readers is essentially mutex lock/unlock + with a few simple checks make this rwlock implementation + wr-lock optimized. + */ + rwlock->active_writer= TRUE; +#ifdef SAFE_MUTEX + rwlock->writer_thread= pthread_self(); +#endif + return 0; +} + + +int rw_pr_unlock(rw_pr_lock_t *rwlock) +{ + if (rwlock->active_writer) + { + /* We are unlocking wr-lock. */ +#ifdef SAFE_MUTEX + rwlock->writer_thread= 0; +#endif + rwlock->active_writer= FALSE; + if (rwlock->writers_waiting_readers) + { + /* + Avoid expensive cond signal in case when there is no contention + or it is wr-only. + + Note that from view point of performance it would be better to + signal on the condition variable after unlocking mutex (as it + reduces number of contex switches). + + Unfortunately this would mean that such rwlock can't be safely + used by MDL subsystem, which relies on the fact that it is OK + to destroy rwlock once it is in unlocked state. + */ + pthread_cond_signal(&rwlock->no_active_readers); + } + pthread_mutex_unlock(&rwlock->lock); + } + else + { + /* We are unlocking rd-lock. */ + pthread_mutex_lock(&rwlock->lock); + rwlock->active_readers--; + if (rwlock->active_readers == 0 && + rwlock->writers_waiting_readers) + { + /* + If we are last reader and there are waiting + writers wake them up. + */ + pthread_cond_signal(&rwlock->no_active_readers); + } + pthread_mutex_unlock(&rwlock->lock); + } + return 0; +} + + #endif /* defined(THREAD) */ From 8322cad0150bf1389bb4a3344772ef1bd30c0b36 Mon Sep 17 00:00:00 2001 From: Dmitry Lenev Date: Thu, 30 Sep 2010 17:29:12 +0400 Subject: [PATCH 24/24] Reverted a temporary workaround for bug #56405 "Deadlock in the MDL deadlock detector". It is no longer needed as a better fix for this bug has been pushed. --- .../perfschema/r/dml_setup_instruments.result | 2 +- sql/mdl.cc | 34 ++++++------------- sql/mdl.h | 16 +-------- sql/sql_base.cc | 10 ++---- sql/sql_base.h | 2 -- sql/table.cc | 33 ------------------ 6 files changed, 14 insertions(+), 83 deletions(-) diff --git a/mysql-test/suite/perfschema/r/dml_setup_instruments.result b/mysql-test/suite/perfschema/r/dml_setup_instruments.result index 7f203118346..2338252976c 100644 --- a/mysql-test/suite/perfschema/r/dml_setup_instruments.result +++ b/mysql-test/suite/perfschema/r/dml_setup_instruments.result @@ -13,7 +13,7 @@ wait/synch/mutex/sql/LOCK_active_mi YES YES wait/synch/mutex/sql/LOCK_audit_mask YES YES wait/synch/mutex/sql/LOCK_connection_count YES YES wait/synch/mutex/sql/LOCK_crypt YES YES -wait/synch/mutex/sql/LOCK_dd_owns_lock_open YES YES +wait/synch/mutex/sql/LOCK_delayed_create YES YES select * from performance_schema.SETUP_INSTRUMENTS where name like 'Wait/Synch/Rwlock/sql/%' and name not in ('wait/synch/rwlock/sql/CRYPTO_dynlock_value::lock') diff --git a/sql/mdl.cc b/sql/mdl.cc index aa7c2a4b7f2..d53ddcee0c8 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -124,6 +124,7 @@ public: Deadlock_detection_visitor(MDL_context *start_node_arg) : m_start_node(start_node_arg), m_victim(NULL), + m_current_search_depth(0), m_found_deadlock(FALSE) {} virtual bool enter_node(MDL_context *node); @@ -132,8 +133,6 @@ public: virtual bool inspect_edge(MDL_context *dest); MDL_context *get_victim() const { return m_victim; } - - void abort_traversal(MDL_context *node); private: /** Change the deadlock victim to a new one if it has lower deadlock @@ -148,6 +147,13 @@ private: MDL_context *m_start_node; /** If a deadlock is found, the context that identifies the victim. */ MDL_context *m_victim; + /** Set to the 0 at start. Increased whenever + we descend into another MDL context (aka traverse to the next + wait-for graph node). When MAX_SEARCH_DEPTH is reached, we + assume that a deadlock is found, even if we have not found a + loop. + */ + uint m_current_search_depth; /** TRUE if we found a deadlock. */ bool m_found_deadlock; /** @@ -181,7 +187,7 @@ private: bool Deadlock_detection_visitor::enter_node(MDL_context *node) { - m_found_deadlock= m_current_search_depth >= MAX_SEARCH_DEPTH; + m_found_deadlock= ++m_current_search_depth >= MAX_SEARCH_DEPTH; if (m_found_deadlock) { DBUG_ASSERT(! m_victim); @@ -201,6 +207,7 @@ bool Deadlock_detection_visitor::enter_node(MDL_context *node) void Deadlock_detection_visitor::leave_node(MDL_context *node) { + --m_current_search_depth; if (m_found_deadlock) opt_change_victim_to(node); } @@ -244,21 +251,6 @@ Deadlock_detection_visitor::opt_change_victim_to(MDL_context *new_victim) } -/** - Abort traversal of a wait-for graph and report a deadlock. - - @param node Node which we were about to visit when abort - was initiated. -*/ - -void Deadlock_detection_visitor::abort_traversal(MDL_context *node) -{ - DBUG_ASSERT(! m_victim); - m_found_deadlock= TRUE; - opt_change_victim_to(node); -} - - /** Get a bit corresponding to enum_mdl_type value in a granted/waiting bitmaps and compatibility matrices. @@ -2064,13 +2056,8 @@ bool MDL_lock::visit_subgraph(MDL_ticket *waiting_ticket, are visiting it but this is OK: in the worst case we might do some extra work and one more context might be chosen as a victim. */ - ++gvisitor->m_current_search_depth; - if (gvisitor->enter_node(src_ctx)) - { - --gvisitor->m_current_search_depth; goto end; - } /* We do a breadth-first search first -- that is, inspect all @@ -2127,7 +2114,6 @@ bool MDL_lock::visit_subgraph(MDL_ticket *waiting_ticket, end_leave_node: gvisitor->leave_node(src_ctx); - --gvisitor->m_current_search_depth; end: mysql_prlock_unlock(&m_rwlock); diff --git a/sql/mdl.h b/sql/mdl.h index e1d4cf74dd6..7938d833eac 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -385,10 +385,7 @@ public: virtual bool inspect_edge(MDL_context *dest) = 0; virtual ~MDL_wait_for_graph_visitor(); - MDL_wait_for_graph_visitor() :m_lock_open_count(0), - m_current_search_depth(0) - { } - virtual void abort_traversal(MDL_context *node) = 0; + MDL_wait_for_graph_visitor() :m_lock_open_count(0) {} public: /** XXX, hack: During deadlock search, we may need to @@ -399,17 +396,6 @@ public: LOCK_open since it has significant performance impacts. */ uint m_lock_open_count; - /** - Set to the 0 at start. Increased whenever - we descend into another MDL context (aka traverse to the next - wait-for graph node). When MAX_SEARCH_DEPTH is reached, we - assume that a deadlock is found, even if we have not found a - loop. - - XXX: This member belongs to this class only temporarily until - bug #56405 is fixed. - */ - uint m_current_search_depth; }; /** diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 634d950cab7..8305283cd17 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -100,14 +100,11 @@ bool No_such_table_error_handler::safely_trapped_errors() TABLE_SHAREs, refresh_version and the table id counter. */ mysql_mutex_t LOCK_open; -mysql_mutex_t LOCK_dd_owns_lock_open; -uint dd_owns_lock_open= 0; #ifdef HAVE_PSI_INTERFACE -static PSI_mutex_key key_LOCK_open, key_LOCK_dd_owns_lock_open; +static PSI_mutex_key key_LOCK_open; static PSI_mutex_info all_tdc_mutexes[]= { - { &key_LOCK_open, "LOCK_open", PSI_FLAG_GLOBAL }, - { &key_LOCK_dd_owns_lock_open, "LOCK_dd_owns_lock_open", PSI_FLAG_GLOBAL } + { &key_LOCK_open, "LOCK_open", PSI_FLAG_GLOBAL } }; /** @@ -302,8 +299,6 @@ bool table_def_init(void) init_tdc_psi_keys(); #endif mysql_mutex_init(key_LOCK_open, &LOCK_open, MY_MUTEX_INIT_FAST); - mysql_mutex_init(key_LOCK_dd_owns_lock_open, &LOCK_dd_owns_lock_open, - MY_MUTEX_INIT_FAST); oldest_unused_share= &end_of_unused_share; end_of_unused_share.prev= &oldest_unused_share; @@ -347,7 +342,6 @@ void table_def_free(void) table_def_inited= 0; /* Free table definitions. */ my_hash_free(&table_def_cache); - mysql_mutex_destroy(&LOCK_dd_owns_lock_open); mysql_mutex_destroy(&LOCK_open); } DBUG_VOID_RETURN; diff --git a/sql/sql_base.h b/sql/sql_base.h index 509760909da..7ae3971942b 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -70,8 +70,6 @@ enum enum_tdc_remove_table_type {TDC_RT_REMOVE_ALL, TDC_RT_REMOVE_NOT_OWN, bool check_dup(const char *db, const char *name, TABLE_LIST *tables); extern mysql_mutex_t LOCK_open; -extern mysql_mutex_t LOCK_dd_owns_lock_open; -extern uint dd_owns_lock_open; bool table_cache_init(void); void table_cache_free(void); bool table_def_init(void); diff --git a/sql/table.cc b/sql/table.cc index 21545f9ec75..e84991912a4 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -3085,30 +3085,7 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush, holding a write-lock on MDL_lock::m_rwlock. */ if (gvisitor->m_lock_open_count++ == 0) - { - /* - To circumvent bug #56405 "Deadlock in the MDL deadlock detector" - we don't try to lock LOCK_open mutex if some thread doing - deadlock detection already owns it and current search depth is - greater than 0. Instead we report a deadlock. - - TODO/FIXME: The proper fix for this bug is to use rwlocks for - protection of table shares/instead of LOCK_open. - Unfortunately it requires more effort/has significant - performance effect. - */ - mysql_mutex_lock(&LOCK_dd_owns_lock_open); - if (gvisitor->m_current_search_depth > 0 && dd_owns_lock_open > 0) - { - mysql_mutex_unlock(&LOCK_dd_owns_lock_open); - --gvisitor->m_lock_open_count; - gvisitor->abort_traversal(src_ctx); - return TRUE; - } - ++dd_owns_lock_open; - mysql_mutex_unlock(&LOCK_dd_owns_lock_open); mysql_mutex_lock(&LOCK_open); - } I_P_List_iterator tables_it(used_tables); @@ -3123,12 +3100,8 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush, goto end; } - ++gvisitor->m_current_search_depth; if (gvisitor->enter_node(src_ctx)) - { - --gvisitor->m_current_search_depth; goto end; - } while ((table= tables_it++)) { @@ -3151,16 +3124,10 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush, end_leave_node: gvisitor->leave_node(src_ctx); - --gvisitor->m_current_search_depth; end: if (gvisitor->m_lock_open_count-- == 1) - { mysql_mutex_unlock(&LOCK_open); - mysql_mutex_lock(&LOCK_dd_owns_lock_open); - --dd_owns_lock_open; - mysql_mutex_unlock(&LOCK_dd_owns_lock_open); - } return result; }