From 06b245f76822d6c857877339a02a99aaded4940f Mon Sep 17 00:00:00 2001 From: Varun Gupta Date: Tue, 5 May 2020 15:35:58 +0530 Subject: MDEV-13266: Race condition in ANALYZE TABLE / statistics collection Fixing a race condition while collecting the engine independent statistics. Thread1> 1) start running "ANALYZE TABLE t PERISTENT FOR COLUMNS (..) INDEXES ($list)" 2) Walk through $list and save it in TABLE::keys_in_use_for_query 3) Close/re-open tables Thread2> 1) Make some use of table t. This involves taking table t from the table cache, and putting it back (with TABLE::keys_in_use_for_query reset to 0) Thread1> continue collecting EITS stats. Since TABLE::keys_in_use_for_query is set to 0 we will not collect statistics for indexes in $list. --- sql/sql_admin.cc | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) (limited to 'sql') diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc index ab95fdc340c..0613495f202 100644 --- a/sql/sql_admin.cc +++ b/sql/sql_admin.cc @@ -769,31 +769,6 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, (table->table->s->table_category == TABLE_CATEGORY_USER && (get_use_stat_tables_mode(thd) > NEVER || lex->with_persistent_for_clause)); - - - if (!lex->index_list) - { - tab->keys_in_use_for_query.init(tab->s->keys); - } - else - { - int pos; - LEX_STRING *index_name; - List_iterator_fast it(*lex->index_list); - - tab->keys_in_use_for_query.clear_all(); - while ((index_name= it++)) - { - if (tab->s->keynames.type_names == 0 || - (pos= find_type(&tab->s->keynames, index_name->str, - index_name->length, 1)) <= 0) - { - compl_result_code= result_code= HA_ADMIN_INVALID; - break; - } - tab->keys_in_use_for_query.set_bit(--pos); - } - } } if (result_code == HA_ADMIN_OK) @@ -878,6 +853,27 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, } tab->file->column_bitmaps_signal(); } + if (!lex->index_list) + tab->keys_in_use_for_query.init(tab->s->keys); + else + { + int pos; + LEX_STRING *index_name; + List_iterator_fast it(*lex->index_list); + + tab->keys_in_use_for_query.clear_all(); + while ((index_name= it++)) + { + if (tab->s->keynames.type_names == 0 || + (pos= find_type(&tab->s->keynames, index_name->str, + index_name->length, 1)) <= 0) + { + compl_result_code= result_code= HA_ADMIN_INVALID; + break; + } + tab->keys_in_use_for_query.set_bit(--pos); + } + } if (!(compl_result_code= alloc_statistics_for_table(thd, table->table)) && !(compl_result_code= -- cgit v1.2.1 From a2560b00770d36d3509c5ebdddad8c34dab0dddf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 12 May 2020 10:13:00 +0300 Subject: =?UTF-8?q?MDEV-22529=20thd=5Fquery=5Fsafe()=20isn=E2=80=99t,=20ca?= =?UTF-8?q?using=20InnoDB=20to=20hang?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function thd_query_safe() is used in the implementation of the following INFORMATION_SCHEMA views: information_schema.innodb_trx information_schema.innodb_locks information_schema.innodb_lock_waits information_schema.rocksdb_trx The implementation of the InnoDB views is in trx_i_s_common_fill_table(). This function invokes trx_i_s_possibly_fetch_data_into_cache(), which will acquire lock_sys->mutex and trx_sys->mutex in order to protect the set of active transactions and explicit locks. While holding those mutexes, it will traverse the collection of InnoDB transactions. For each transaction, thd_query_safe() will be invoked. When called via trx_i_s_common_fill_table(), thd_query_safe() is acquiring THD::LOCK_thd_data while holding the InnoDB locks. This will cause a deadlock with THD::awake() (such as executing KILL QUERY), because THD::awake() could invoke lock_trx_handle_wait(), which attempts to acquire lock_sys->mutex while already holding THD::lock_thd_data. thd_query_safe(): Invoke mysql_mutex_trylock() instead of mysql_mutex_lock(). Return the empty string if the mutex cannot be acquired without waiting. --- sql/sql_class.cc | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'sql') diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 38770a24dec..655824d93fe 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -4530,6 +4530,7 @@ extern "C" LEX_STRING * thd_query_string (MYSQL_THD thd) @param buflen Length of the buffer @return Length of the query + @retval 0 if LOCK_thd_data cannot be acquired without waiting @note This function is thread safe as the query string is accessed under mutex protection and the string is copied @@ -4538,10 +4539,19 @@ extern "C" LEX_STRING * thd_query_string (MYSQL_THD thd) extern "C" size_t thd_query_safe(MYSQL_THD thd, char *buf, size_t buflen) { - mysql_mutex_lock(&thd->LOCK_thd_data); - size_t len= MY_MIN(buflen - 1, thd->query_length()); - memcpy(buf, thd->query(), len); - mysql_mutex_unlock(&thd->LOCK_thd_data); + size_t len= 0; + /* InnoDB invokes this function while holding internal mutexes. + THD::awake() will hold LOCK_thd_data while invoking an InnoDB + function that would acquire the internal mutex. Because this + function is a non-essential part of information_schema view output, + we will break the deadlock by avoiding a mutex wait here + and returning the empty string if a wait would be needed. */ + if (!mysql_mutex_trylock(&thd->LOCK_thd_data)) + { + len= MY_MIN(buflen - 1, thd->query_length()); + memcpy(buf, thd->query(), len); + mysql_mutex_unlock(&thd->LOCK_thd_data); + } buf[len]= '\0'; return len; } -- cgit v1.2.1