diff options
author | Dmitry Lenev <dlenev@mysql.com> | 2010-02-08 23:19:55 +0300 |
---|---|---|
committer | Dmitry Lenev <dlenev@mysql.com> | 2010-02-08 23:19:55 +0300 |
commit | c7e7a7d20cae8a22e7730b2015e08233d680ed02 (patch) | |
tree | e483a661cd83b6e2635e718ef7413d8d09993bfb /sql | |
parent | f750b5f16029cdd06a01e056d0c68a82f7696310 (diff) | |
download | mariadb-git-c7e7a7d20cae8a22e7730b2015e08233d680ed02.tar.gz |
Fix for bug #50913 "Deadlock between open_and_lock_tables_derived
and MDL".
Concurrent execution of a multi-DELETE statement and ALTER
TABLE statement which affected one of the tables used in
the multi-DELETE sometimes led to deadlock.
Similar deadlocks might have occured when one performed
INSERT/UPDATE/DELETE on a view and concurrently executed
ALTER TABLE for the view's underlying table, or when one
concurrently executed TRUNCATE TABLE for InnoDB table and
ALTER TABLE for the same table.
These deadlocks were caused by a discrepancy between types of
metadata and thr_lock.cc locks acquired by those statements.
What happened was that multi-DELETE/TRUNCATE/DML-through-the-
view statement in the first connection acquired SR lock on a
table, then ALTER TABLE would come in in the second connection
and acquire SNW metadata lock and TL_WRITE_ALLOW_READ
thr_lock.c lock and then would start waiting for the first
connection during lock upgrade. After that the statement in
the first connection would try to acquire TL_WRITE lock on
table and would start waiting for the second connection,
creating a deadlock.
This patch solves this problem by ensuring that we acquire
SW metadata lock in all cases in which we acquiring write
thr_lock.c lock. This guarantees that deadlocks like the
one described above won't occur since all lock conflicts
in such situation are resolved within MDL subsystem.
This patch also adds assert which should guarantee that
such situations won't arise in future.
Diffstat (limited to 'sql')
-rw-r--r-- | sql/lock.cc | 19 | ||||
-rw-r--r-- | sql/mysql_priv.h | 5 | ||||
-rw-r--r-- | sql/sql_base.cc | 4 | ||||
-rw-r--r-- | sql/sql_delete.cc | 1 | ||||
-rw-r--r-- | sql/sql_handler.cc | 4 | ||||
-rw-r--r-- | sql/sql_parse.cc | 3 | ||||
-rw-r--r-- | sql/sql_show.cc | 2 | ||||
-rw-r--r-- | sql/sql_update.cc | 5 | ||||
-rw-r--r-- | sql/sql_view.cc | 4 |
9 files changed, 44 insertions, 3 deletions
diff --git a/sql/lock.cc b/sql/lock.cc index f34b6c80872..97756893e2b 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -153,6 +153,25 @@ int mysql_lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags) { system_count++; } + + /* + If we are going to lock a non-temporary table we must own metadata + lock of appropriate type on it (I.e. for table to be locked for + write we must own metadata lock of MDL_SHARED_WRITE or stronger + type. For table to be locked for read we must own metadata lock + of MDL_SHARED_READ or stronger type). + The only exception are HANDLER statements which are allowed to + lock table for read while having only MDL_SHARED lock on it. + */ + DBUG_ASSERT(t->s->tmp_table || + thd->mdl_context.is_lock_owner(MDL_key::TABLE, + t->s->db.str, t->s->table_name.str, + t->reginfo.lock_type >= TL_WRITE_ALLOW_WRITE ? + MDL_SHARED_WRITE : MDL_SHARED_READ) || + (t->open_by_handler && + thd->mdl_context.is_lock_owner(MDL_key::TABLE, + t->s->db.str, t->s->table_name.str, + MDL_SHARED))); } /* diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 5ca70302952..6f207ccb00e 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -2164,6 +2164,11 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **table, uint count, #define MYSQL_OPEN_FAIL_ON_MDL_CONFLICT 0x0200 /** Open tables using MDL_SHARED lock instead of one specified in parser. */ #define MYSQL_OPEN_FORCE_SHARED_MDL 0x0400 +/** + Open tables using MDL_SHARED_HIGH_PRIO lock instead of one specified + in parser. +*/ +#define MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL 0x0800 /** Please refer to the internals manual. */ #define MYSQL_OPEN_REOPEN (MYSQL_LOCK_IGNORE_FLUSH |\ diff --git a/sql/sql_base.cc b/sql/sql_base.cc index f68d9a29f05..1564838548f 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -2378,11 +2378,11 @@ open_table_get_mdl_lock(THD *thd, TABLE_LIST *table_list, used in the statement being prepared. */ DBUG_ASSERT(!(flags & (MYSQL_OPEN_TAKE_UPGRADABLE_MDL | - MYSQL_LOCK_IGNORE_FLUSH))); + MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL))); mdl_request->set_type(MDL_SHARED); } - else if (flags & MYSQL_LOCK_IGNORE_FLUSH) + else if (flags & MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL) { DBUG_ASSERT(!(flags & MYSQL_OPEN_TAKE_UPGRADABLE_MDL)); mdl_request->set_type(MDL_SHARED_HIGH_PRIO); diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index 0f84f5e9d73..0f5d51f924d 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -1052,6 +1052,7 @@ static bool mysql_truncate_by_delete(THD *thd, TABLE_LIST *table_list) bool error, save_binlog_row_based= thd->is_current_stmt_binlog_format_row(); DBUG_ENTER("mysql_truncate_by_delete"); table_list->lock_type= TL_WRITE; + table_list->mdl_request.set_type(MDL_SHARED_WRITE); mysql_init_select(thd->lex); thd->clear_current_stmt_binlog_format_row(); /* Delete all rows from table */ diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index d9c2a84d03e..9f365d0cf2f 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -124,6 +124,7 @@ static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables) { /* Non temporary table. */ tables->table->file->ha_index_or_rnd_end(); + tables->table->open_by_handler= 0; mysql_mutex_lock(&LOCK_open); if (close_thread_table(thd, &tables->table)) { @@ -332,7 +333,8 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) hash_tables->table->s->tmp_table); /* If it's a temp table, don't reset table->query_id as the table is - being used by this handler. Otherwise, no meaning at all. + being used by this handler. For non-temp tables we use this flag + in asserts. */ hash_tables->table->open_by_handler= 1; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index fe49962d77a..1906040d5c6 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -7061,6 +7061,9 @@ bool multi_delete_set_locks_and_link_aux_tables(LEX *lex) } walk->updating= target_tbl->updating; walk->lock_type= target_tbl->lock_type; + /* We can assume that tables to be deleted from are locked for write. */ + DBUG_ASSERT(walk->lock_type >= TL_WRITE_ALLOW_WRITE); + walk->mdl_request.set_type(MDL_SHARED_WRITE); target_tbl->correspondent_table= walk; // Remember corresponding table } DBUG_RETURN(FALSE); diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 2238c7bb1ef..2a05cbc561d 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -2928,6 +2928,7 @@ fill_schema_show_cols_or_idxs(THD *thd, TABLE_LIST *tables, lex->sql_command= SQLCOM_SHOW_FIELDS; res= open_normal_and_derived_tables(thd, show_table_list, (MYSQL_LOCK_IGNORE_FLUSH | + MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL | (can_deadlock ? MYSQL_OPEN_FAIL_ON_MDL_CONFLICT : 0))); lex->sql_command= save_sql_command; @@ -3507,6 +3508,7 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) schema_table->i_s_requested_object; res= open_normal_and_derived_tables(thd, show_table_list, (MYSQL_LOCK_IGNORE_FLUSH | + MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL | (can_deadlock ? MYSQL_OPEN_FAIL_ON_MDL_CONFLICT : 0))); lex->sql_command= save_sql_command; /* diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 736dceba837..a163dda2c69 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -1051,6 +1051,11 @@ reopen_tables: If we are using the binary log, we need TL_READ_NO_INSERT to get correct order of statements. Otherwise, we use a TL_READ lock to improve performance. + We don't downgrade metadata lock from SW to SR in this case as + there is no guarantee that the same ticket is not used by + another table instance used by this statement which is going to + be write-locked (for example, trigger to be invoked might try + to update this table). */ tl->lock_type= read_lock_type_for_table(thd, table); tl->updating= 0; diff --git a/sql/sql_view.cc b/sql/sql_view.cc index 931a7adb57f..b9eb6a63552 100644 --- a/sql/sql_view.cc +++ b/sql/sql_view.cc @@ -1347,7 +1347,11 @@ bool mysql_make_view(THD *thd, File_parser *parser, TABLE_LIST *table, anyway. */ for (tbl= view_main_select_tables; tbl; tbl= tbl->next_local) + { tbl->lock_type= table->lock_type; + tbl->mdl_request.set_type((tbl->lock_type >= TL_WRITE_ALLOW_WRITE) ? + MDL_SHARED_WRITE : MDL_SHARED_READ); + } /* If the view is mergeable, we might want to INSERT/UPDATE/DELETE into tables of this view. Preserve the |