From 6e9a48b67fceab17089ca4cd1406e302386a601b Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Fri, 11 Jan 2013 00:22:14 +0200 Subject: Fixed some race conditons and bugs related to killed queries KILL now breaks locks inside InnoDB Fixed possible deadlock when running INNODB STATUS Added ha_kill_query() and kill_query() to send kill signal to all storage engines Added reset_killed() to ensure we don't reset killed state while awake() is getting called include/mysql/plugin.h: Added thd_mark_as_hard_kill() include/mysql/plugin_audit.h.pp: Added thd_mark_as_hard_kill() include/mysql/plugin_auth.h.pp: Added thd_mark_as_hard_kill() include/mysql/plugin_ftparser.h.pp: Added thd_mark_as_hard_kill() sql/handler.cc: Added ha_kill_query() to send kill signal to all storage engines sql/handler.h: Added ha_kill_query() and kill_query() to send kill signal to all storage engines sql/log_event.cc: Use reset_killed() sql/mdl.cc: use thd->killed instead of thd_killed() to abort on soft kill sql/sp_rcontext.cc: Use reset_killed() sql/sql_class.cc: Fixed possible deadlock in INNODB STATUS by not getting thd->LOCK_thd_data if it's locked. Use reset_killed() Tell storge engines that KILL has been sent sql/sql_class.h: Added reset_killed() to ensure we don't reset killed state while awake() is getting called. Added mark_as_hard_kill() sql/sql_insert.cc: Use reset_killed() sql/sql_parse.cc: Simplify detection of killed queries. Use reset_killed() sql/sql_select.cc: Use reset_killed() sql/sql_union.cc: Use reset_killed() storage/innobase/handler/ha_innodb.cc: Added innobase_kill_query() Fixed error reporting for interrupted queries. storage/xtradb/handler/ha_innodb.cc: Added innobase_kill_query() Fixed error reporting for interrupted queries. --- sql/handler.cc | 21 ++++++++++++++++++++- sql/handler.h | 8 ++++++++ sql/log_event.cc | 2 +- sql/mdl.cc | 6 +++--- sql/sp_rcontext.cc | 2 +- sql/sql_class.cc | 44 ++++++++++++++++++++++++++++++-------------- sql/sql_class.h | 24 ++++++++++++++++++++++++ sql/sql_insert.cc | 5 ++++- sql/sql_parse.cc | 22 +++++++++++++--------- sql/sql_select.cc | 4 ++-- sql/sql_union.cc | 2 +- 11 files changed, 107 insertions(+), 33 deletions(-) (limited to 'sql') diff --git a/sql/handler.cc b/sql/handler.cc index 356ae330201..5108e3abc40 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -663,7 +663,6 @@ static my_bool closecon_handlerton(THD *thd, plugin_ref plugin, return FALSE; } - /** @note don't bother to rollback here, it's done already @@ -673,6 +672,26 @@ void ha_close_connection(THD* thd) plugin_foreach(thd, closecon_handlerton, MYSQL_STORAGE_ENGINE_PLUGIN, 0); } +static my_bool kill_handlerton(THD *thd, plugin_ref plugin, + void *hard_kill) +{ + handlerton *hton= plugin_data(plugin, handlerton *); + + if (hton->state == SHOW_OPTION_YES && hton->kill_query && + thd_get_ha_data(thd, hton)) + hton->kill_query(hton, thd, * (my_bool*) hard_kill); + return FALSE; +} + +void ha_kill_query(THD* thd, my_bool hard_kill) +{ + DBUG_ENTER("ha_kill_query"); + plugin_foreach(thd, kill_handlerton, MYSQL_STORAGE_ENGINE_PLUGIN, + &hard_kill); + DBUG_VOID_RETURN; +} + + /* ======================================================================== ======================= TRANSACTIONS ===================================*/ diff --git a/sql/handler.h b/sql/handler.h index 4a6fc2f1bd5..e5da92b0d40 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -864,6 +864,13 @@ struct handlerton this storage engine was accessed in this connection */ int (*close_connection)(handlerton *hton, THD *thd); + /* + Tell handler that query has been killed. + hard_kill is set in case of HARD KILL (abort query even if + it may corrupt table). + Return 1 if the handler wants to upgrade the kill to a hard kill + */ + void (*kill_query)(handlerton *hton, THD *thd, my_bool hard_kill); /* sv points to an uninitialized storage area of requested size (see savepoint_offset description) @@ -2975,6 +2982,7 @@ int ha_finalize_handlerton(st_plugin_int *plugin); TYPELIB *ha_known_exts(void); int ha_panic(enum ha_panic_function flag); void ha_close_connection(THD* thd); +void ha_kill_query(THD* thd, my_bool hard_kill); bool ha_flush_logs(handlerton *db_type); void ha_drop_database(char* path); void ha_checkpoint_state(bool disable); diff --git a/sql/log_event.cc b/sql/log_event.cc index 6fc116d2b92..a46da2304b2 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -3884,7 +3884,7 @@ Default database: '%s'. Query: '%s'", { DBUG_PRINT("info",("error ignored")); clear_all_errors(thd, const_cast(rli)); - thd->killed= NOT_KILLED; + thd->reset_killed(); } /* Other cases: mostly we expected no error and get one. diff --git a/sql/mdl.cc b/sql/mdl.cc index a2113909116..415d56887d5 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -14,7 +14,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#include "mdl.h" +#include "sql_class.h" #include "debug_sync.h" #include #include @@ -1185,7 +1185,7 @@ MDL_wait::timed_wait(THD *thd, struct timespec *abs_timeout, wait_state_name); thd_wait_begin(thd, THD_WAIT_META_DATA_LOCK); - while (!m_wait_status && !thd_killed(thd) && + while (!m_wait_status && !thd->killed && wait_result != ETIMEDOUT && wait_result != ETIME) { wait_result= mysql_cond_timedwait(&m_COND_wait_status, &m_LOCK_wait_status, @@ -1207,7 +1207,7 @@ MDL_wait::timed_wait(THD *thd, struct timespec *abs_timeout, false, which means that the caller intends to restart the wait. */ - if (thd_killed(thd)) + if (thd->killed) m_wait_status= KILLED; else if (set_status_on_timeout) m_wait_status= TIMEOUT; diff --git a/sql/sp_rcontext.cc b/sql/sp_rcontext.cc index 6fe4be989db..d78f2a162f5 100644 --- a/sql/sp_rcontext.cc +++ b/sql/sp_rcontext.cc @@ -419,7 +419,7 @@ sp_rcontext::activate_handler(THD *thd, /* Reset error state. */ thd->clear_error(); - thd->killed= NOT_KILLED; // Some errors set thd->killed + thd->reset_killed(); // Some errors set thd->killed // (e.g. "bad data"). /* Return IP of the activated SQL handler. */ diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 42952585e07..312de8c74e4 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -701,7 +701,7 @@ char *thd_security_context(THD *thd, char *buffer, unsigned int length, values doesn't have to very accurate and the memory it points to is static, but we need to attempt a snapshot on the pointer values to avoid using NULL values. The pointer to thd->query however, doesn't point to static memory - and has to be protected by LOCK_thread_count or risk pointing to + and has to be protected by thd->LOCK_thd_data or risk pointing to uninitialized memory. */ const char *proc_info= thd->proc_info; @@ -736,20 +736,21 @@ char *thd_security_context(THD *thd, char *buffer, unsigned int length, str.append(proc_info); } - mysql_mutex_lock(&thd->LOCK_thd_data); - - if (thd->query()) + /* Don't wait if LOCK_thd_data is used as this could cause a deadlock */ + if (!mysql_mutex_trylock(&thd->LOCK_thd_data)) { - if (max_query_len < 1) - len= thd->query_length(); - else - len= min(thd->query_length(), max_query_len); - str.append('\n'); - str.append(thd->query(), len); + if (thd->query()) + { + if (max_query_len < 1) + len= thd->query_length(); + else + len= min(thd->query_length(), max_query_len); + str.append('\n'); + str.append(thd->query(), len); + } + mysql_mutex_unlock(&thd->LOCK_thd_data); } - mysql_mutex_unlock(&thd->LOCK_thd_data); - if (str.c_ptr_safe() == buffer) return buffer; @@ -1355,7 +1356,7 @@ void THD::change_user(void) mysql_mutex_unlock(&LOCK_status); cleanup(); - killed= NOT_KILLED; + reset_killed(); cleanup_done= 0; init(); stmt_map.reset(); @@ -1610,6 +1611,10 @@ void THD::awake(killed_state state_to_set) MYSQL_CALLBACK(scheduler, post_kill_notification, (this)); } + /* Interrupt target waiting inside a storage engine. */ + if (state_to_set != NOT_KILLED) + ha_kill_query(this, test(state_to_set & KILL_HARD_BIT)); + /* Broadcast a condition to kick the target if it is waiting on it. */ if (mysys_var) { @@ -3848,6 +3853,18 @@ extern "C" int thd_killed(const MYSQL_THD thd) return thd->killed; } +/** + Change kill level to hard. + This ensures that thd_killed() will return true. + This is important for storage engines that uses thd_killed() to + verify if thread is killed. +*/ + +extern "C" void thd_mark_as_hard_kill(MYSQL_THD thd) +{ + thd->mark_as_hard_kill(); +} + /** Send an out-of-band progress report to the client @@ -5603,4 +5620,3 @@ bool Discrete_intervals_list::append(Discrete_interval *new_interval) } #endif /* !defined(MYSQL_CLIENT) */ - diff --git a/sql/sql_class.h b/sql/sql_class.h index f50d1442a3f..2561effb478 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -2703,6 +2703,19 @@ public: { return ::killed_errno(killed); } + inline void reset_killed() + { + /* + Resetting killed has to be done under a mutex to ensure + its not done during an awake() call. + */ + if (killed != NOT_KILLED) + { + mysql_mutex_lock(&LOCK_thd_data); + killed= NOT_KILLED; + mysql_mutex_unlock(&LOCK_thd_data); + } + } inline void send_kill_message() const { int err= killed_errno(); @@ -2716,6 +2729,17 @@ public: (!transaction.stmt.modified_non_trans_table || (variables.sql_mode & MODE_STRICT_ALL_TABLES))); } + /* + Increase level of kill ; Ensures that thd_killed() returns true. + + Needed if storage engine wants to abort things because of a 'soft' (ie, + safe) kill but still uses thd_killed() to check if it's killed. + */ + inline void mark_as_hard_kill() + { + DBUG_ASSERT(killed != NOT_KILLED); + killed= (killed_state) (killed | KILL_HARD_BIT); + } void set_status_var_init(); void reset_n_backup_open_tables_state(Open_tables_backup *backup); void restore_backup_open_tables_state(Open_tables_backup *backup); diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index f18a9548704..ef3f81f18c5 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2724,7 +2724,10 @@ pthread_handler_t handle_delayed_insert(void *arg) thd->thread_id= thd->variables.pseudo_thread_id= thread_id++; thd->set_current_time(); threads.append(thd); - thd->killed=abort_loop ? KILL_CONNECTION : NOT_KILLED; + if (abort_loop) + thd->killed= KILL_CONNECTION; + else + thd->reset_killed(); mysql_mutex_unlock(&LOCK_thread_count); mysql_thread_set_psi_id(thd->thread_id); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index de35d2f3d27..2ae4adb4e33 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -4495,16 +4495,20 @@ finish: if (! thd->in_sub_stmt) { - /* report error issued during command execution */ - if (thd->killed_errno()) + if (thd->killed != NOT_KILLED) { - if (! thd->stmt_da->is_set()) - thd->send_kill_message(); - } - if (thd->killed < KILL_CONNECTION) - { - thd->killed= NOT_KILLED; - thd->mysys_var->abort= 0; + /* report error issued during command execution */ + if (thd->killed_errno()) + { + /* If we already sent 'ok', we can ignore any kill query statements */ + if (! thd->stmt_da->is_set()) + thd->send_kill_message(); + } + if (thd->killed < KILL_CONNECTION) + { + thd->reset_killed(); + thd->mysys_var->abort= 0; + } } if (thd->is_error() || (thd->variables.option_bits & OPTION_MASTER_SQL_ERROR)) trans_rollback_stmt(thd); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 424d2d4a56b..9bbc139927e 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -331,7 +331,7 @@ bool handle_select(THD *thd, LEX *lex, select_result *result, ER(ER_QUERY_EXCEEDED_ROWS_EXAMINED_LIMIT), thd->accessed_rows_and_keys, thd->lex->limit_rows_examined->val_uint()); - thd->killed= NOT_KILLED; + thd->reset_killed(); } /* Disable LIMIT ROWS EXAMINED after query execution. */ thd->lex->limit_rows_examined_cnt= ULONGLONG_MAX; @@ -19213,7 +19213,7 @@ remove_duplicates(JOIN *join, TABLE *entry,List &fields, Item *having) */ thd->lex->limit_rows_examined_cnt= ULONGLONG_MAX; if (thd->killed == ABORT_QUERY) - thd->killed= NOT_KILLED; + thd->reset_killed(); free_io_cache(entry); // Safety entry->file->info(HA_STATUS_VARIABLE); if (entry->s->db_type() == heap_hton || diff --git a/sql/sql_union.cc b/sql/sql_union.cc index 8dcae907926..77d4df1b398 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -716,7 +716,7 @@ bool st_select_lex_unit::exec() ER(ER_QUERY_EXCEEDED_ROWS_EXAMINED_LIMIT), thd->accessed_rows_and_keys, thd->lex->limit_rows_examined->val_uint()); - thd->killed= NOT_KILLED; + thd->reset_killed(); break; } } -- cgit v1.2.1