From 58721c3e38f1d5cd9261c6da205126cf0ae2ab24 Mon Sep 17 00:00:00 2001 From: Monty Date: Sat, 26 May 2018 16:57:18 +0300 Subject: [PATCH] MDEV-16286 Killed CREATE SEQUENCE leaves sequence in unusable state Fixed by deleting the sequence if we where not able to initialize it I also noticed that we didn't always set the error message when check_killed(), which could lead to aborted queries without error beeing properly set. Fixed by default setting error message if check_error() noticed that killed had been called. This allowed me to remove a lot of calls to thd->send_kill_message(). --- sql/group_by_handler.cc | 1 - sql/handler.cc | 2 +- sql/log.cc | 2 +- sql/rpl_gtid.cc | 5 ++--- sql/rpl_parallel.cc | 12 ++++-------- sql/sql_class.cc | 2 +- sql/sql_class.h | 6 +++++- sql/sql_join_cache.cc | 2 -- sql/sql_select.cc | 13 +------------ sql/sql_table.cc | 37 ++++++++++++++++++++++++++++++++----- sql/table.cc | 3 --- 11 files changed, 47 insertions(+), 38 deletions(-) diff --git a/sql/group_by_handler.cc b/sql/group_by_handler.cc index c92df8922ce10..f18758a2d9441 100644 --- a/sql/group_by_handler.cc +++ b/sql/group_by_handler.cc @@ -65,7 +65,6 @@ int Pushdown_query::execute(JOIN *join) { if (unlikely(thd->check_killed())) { - thd->send_kill_message(); handler->end_scan(); DBUG_RETURN(-1); } diff --git a/sql/handler.cc b/sql/handler.cc index 77a3faf0b6f9c..07459d4cd8cf0 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -2766,7 +2766,7 @@ int handler::ha_rnd_next(uchar *buf) if (result != HA_ERR_RECORD_DELETED) break; status_var_increment(table->in_use->status_var.ha_read_rnd_deleted_count); - } while (!table->in_use->check_killed()); + } while (!table->in_use->check_killed(1)); if (result == HA_ERR_RECORD_DELETED) result= HA_ERR_ABORTED_BY_USER; diff --git a/sql/log.cc b/sql/log.cc index bf3838e5545b2..ff4b0366b43d0 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -7441,7 +7441,7 @@ MYSQL_BIN_LOG::queue_for_group_commit(group_commit_entry *orig_entry) &wfc->LOCK_wait_commit, &stage_waiting_for_prior_transaction_to_commit, &old_stage); - while ((loc_waitee= wfc->waitee) && !orig_entry->thd->check_killed()) + while ((loc_waitee= wfc->waitee) && !orig_entry->thd->check_killed(1)) mysql_cond_wait(&wfc->COND_wait_commit, &wfc->LOCK_wait_commit); wfc->opaque_pointer= NULL; DBUG_PRINT("info", ("After waiting for prior commit, queued_by_other=%d", diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc index 6cd02f6ee9fd3..ad885c925d949 100644 --- a/sql/rpl_gtid.cc +++ b/sql/rpl_gtid.cc @@ -169,7 +169,6 @@ rpl_slave_state::check_duplicate_gtid(rpl_gtid *gtid, rpl_group_info *rgi) thd= rgi->thd; if (unlikely(thd->check_killed())) { - thd->send_kill_message(); res= -1; break; } @@ -2602,7 +2601,7 @@ gtid_waiting::wait_for_gtid(THD *thd, rpl_gtid *wait_gtid, &stage_master_gtid_wait_primary, &old_stage); do { - if (unlikely(thd->check_killed())) + if (unlikely(thd->check_killed(1))) break; else if (wait_until) { @@ -2654,7 +2653,7 @@ gtid_waiting::wait_for_gtid(THD *thd, rpl_gtid *wait_gtid, &stage_master_gtid_wait, &old_stage); did_enter_cond= true; } - while (!elem.done && likely(!thd->check_killed())) + while (!elem.done && likely(!thd->check_killed(1))) { thd_wait_begin(thd, THD_WAIT_BINLOG); if (wait_until) diff --git a/sql/rpl_parallel.cc b/sql/rpl_parallel.cc index ad38e9f356880..a12bbba5a1fa8 100644 --- a/sql/rpl_parallel.cc +++ b/sql/rpl_parallel.cc @@ -337,7 +337,7 @@ do_gco_wait(rpl_group_info *rgi, group_commit_orderer *gco, thd->set_time_for_next_stage(); do { - if (unlikely(thd->check_killed()) && !rgi->worker_error) + if (!rgi->worker_error && unlikely(thd->check_killed(1))) { DEBUG_SYNC(thd, "rpl_parallel_start_waiting_for_prior_killed"); thd->clear_error(); @@ -404,7 +404,6 @@ do_ftwrl_wait(rpl_group_info *rgi, break; if (unlikely(thd->check_killed())) { - thd->send_kill_message(); slave_output_error_info(rgi, thd); signal_error_to_sql_driver_thread(thd, rgi, 1); break; @@ -455,7 +454,6 @@ pool_mark_busy(rpl_parallel_thread_pool *pool, THD *thd) { if (thd && unlikely(thd->check_killed())) { - thd->send_kill_message(); res= 1; break; } @@ -573,7 +571,6 @@ rpl_pause_for_ftwrl(THD *thd) { if (unlikely(thd->check_killed())) { - thd->send_kill_message(); err= 1; break; } @@ -838,8 +835,8 @@ retry_event_group(rpl_group_info *rgi, rpl_parallel_thread *rpt, } DBUG_EXECUTE_IF("inject_mdev8031", { /* Simulate pending KILL caught in read_relay_log_description_event(). */ - if (unlikely(thd->check_killed())) { - thd->send_kill_message(); + if (unlikely(thd->check_killed())) + { err= 1; goto err; } @@ -2077,7 +2074,7 @@ rpl_parallel_entry::choose_thread(rpl_group_info *rgi, bool *did_enter_cond, /* The thread is ready to queue into. */ break; } - else if (unlikely(rli->sql_driver_thd->check_killed())) + else if (unlikely(rli->sql_driver_thd->check_killed(1))) { unlock_or_exit_cond(rli->sql_driver_thd, &thr->LOCK_rpl_thread, did_enter_cond, old_stage); @@ -2405,7 +2402,6 @@ rpl_parallel::wait_for_workers_idle(THD *thd) { if (unlikely(thd->check_killed())) { - thd->send_kill_message(); err= 1; break; } diff --git a/sql/sql_class.cc b/sql/sql_class.cc index a5f69d63ad0fb..f937a8fad9f15 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -7503,7 +7503,7 @@ wait_for_commit::wait_for_prior_commit2(THD *thd) thd->ENTER_COND(&COND_wait_commit, &LOCK_wait_commit, &stage_waiting_for_prior_transaction_to_commit, &old_stage); - while ((loc_waitee= this->waitee) && likely(!thd->check_killed())) + while ((loc_waitee= this->waitee) && likely(!thd->check_killed(1))) mysql_cond_wait(&COND_wait_commit, &LOCK_wait_commit); if (!loc_waitee) { diff --git a/sql/sql_class.h b/sql/sql_class.h index 2112765a1b270..8295043bf028d 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3043,10 +3043,14 @@ class THD :public Statement, } *killed_err; /* See also thd_killed() */ - inline bool check_killed() + inline bool check_killed(bool dont_send_error_message= 0) { if (unlikely(killed)) + { + if (!dont_send_error_message) + send_kill_message(); return TRUE; + } if (apc_target.have_apc_requests()) apc_target.process_apc_requests(); return FALSE; diff --git a/sql/sql_join_cache.cc b/sql/sql_join_cache.cc index b8231c6285b93..53c5e992ca984 100644 --- a/sql/sql_join_cache.cc +++ b/sql/sql_join_cache.cc @@ -2262,7 +2262,6 @@ enum_nested_loop_state JOIN_CACHE::join_matching_records(bool skip_last) if (unlikely(join->thd->check_killed())) { /* The user has aborted the execution of the query */ - join->thd->send_kill_message(); rc= NESTED_LOOP_KILLED; goto finish; } @@ -2536,7 +2535,6 @@ enum_nested_loop_state JOIN_CACHE::join_null_complements(bool skip_last) if (unlikely(join->thd->check_killed())) { /* The user has aborted the execution of the query */ - join->thd->send_kill_message(); rc= NESTED_LOOP_KILLED; goto finish; } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 1285835fe7c6b..3dfc41ae86cce 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -309,7 +309,7 @@ void dbug_serve_apcs(THD *thd, int n_calls) thd_proc_info(thd, "show_explain_trap"); my_sleep(30000); thd_proc_info(thd, save_proc_info); - if (unlikely(thd->check_killed())) + if (unlikely(thd->check_killed(1))) break; } } @@ -18482,10 +18482,7 @@ create_internal_tmp_table_from_heap(THD *thd, TABLE *table, if (write_err) goto err; if (unlikely(thd->check_killed())) - { - thd->send_kill_message(); goto err_killed; - } } if (!new_table.no_rows && new_table.file->ha_end_bulk_insert()) goto err; @@ -19045,7 +19042,6 @@ sub_select_cache(JOIN *join, JOIN_TAB *join_tab, bool end_of_records) if (unlikely(join->thd->check_killed())) { /* The user has aborted the execution of the query */ - join->thd->send_kill_message(); DBUG_RETURN(NESTED_LOOP_KILLED); } if (!test_if_use_dynamic_range_scan(join_tab)) @@ -19345,7 +19341,6 @@ evaluate_join_record(JOIN *join, JOIN_TAB *join_tab, DBUG_RETURN(NESTED_LOOP_NO_MORE_ROWS); if (unlikely(join->thd->check_killed())) // Aborted by user { - join->thd->send_kill_message(); DBUG_RETURN(NESTED_LOOP_KILLED); /* purecov: inspected */ } @@ -20705,7 +20700,6 @@ end_write(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)), end: if (unlikely(join->thd->check_killed())) { - join->thd->send_kill_message(); DBUG_RETURN(NESTED_LOOP_KILLED); /* purecov: inspected */ } DBUG_RETURN(NESTED_LOOP_OK); @@ -20795,7 +20789,6 @@ end_update(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)), end: if (unlikely(join->thd->check_killed())) { - join->thd->send_kill_message(); DBUG_RETURN(NESTED_LOOP_KILLED); /* purecov: inspected */ } DBUG_RETURN(NESTED_LOOP_OK); @@ -20845,7 +20838,6 @@ end_unique_update(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)), } if (unlikely(join->thd->check_killed())) { - join->thd->send_kill_message(); DBUG_RETURN(NESTED_LOOP_KILLED); /* purecov: inspected */ } DBUG_RETURN(NESTED_LOOP_OK); @@ -20939,7 +20931,6 @@ end_write_group(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)), end: if (unlikely(join->thd->check_killed())) { - join->thd->send_kill_message(); DBUG_RETURN(NESTED_LOOP_KILLED); /* purecov: inspected */ } DBUG_RETURN(NESTED_LOOP_OK); @@ -22572,7 +22563,6 @@ static int remove_dup_with_compare(THD *thd, TABLE *table, Field **first_field, { if (unlikely(thd->check_killed())) { - thd->send_kill_message(); error=0; goto err; } @@ -22688,7 +22678,6 @@ static int remove_dup_with_hash_index(THD *thd, TABLE *table, uchar *org_key_pos; if (unlikely(thd->check_killed())) { - thd->send_kill_message(); error=0; goto err; } diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 0bf421ef0f62f..a5c9a4541d730 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -4763,7 +4763,7 @@ handler *mysql_create_frm_image(THD *thd, @retval 0 OK @retval 1 error - @retval -1 table existed but IF EXISTS was used + @retval -1 table existed but IF NOT EXISTS was used */ static @@ -5044,6 +5044,12 @@ int create_table_impl(THD *thd, /** Simple wrapper around create_table_impl() to be used in various version of CREATE TABLE statement. + + @result + 1 unspefied error + 2 error; Don't log create statement + 0 ok + -1 Table was used with IF NOT EXISTS and table existed (warning, not error) */ int mysql_create_table_no_lock(THD *thd, @@ -5090,6 +5096,24 @@ int mysql_create_table_no_lock(THD *thd, else table_list->table= 0; res= sequence_insert(thd, thd->lex, table_list); + if (res) + { + DBUG_ASSERT(thd->is_error()); + /* Drop the table as it wasn't completely done */ + if (!mysql_rm_table_no_locks(thd, table_list, 1, + create_info->tmp_table(), + false, true /* Sequence*/, + true /* Don't log_query */, + true /* Don't free locks */ )) + { + /* + From the user point of view, the table creation failed + We return 2 to indicate that this statement doesn't have + to be logged. + */ + res= 2; + } + } } return res; @@ -5507,7 +5531,7 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table, TABLE_LIST *pos_in_locked_tables= 0; Alter_info local_alter_info; Alter_table_ctx local_alter_ctx; // Not used - bool res= TRUE; + int res= 1; bool is_trans= FALSE; bool do_logging= FALSE; uint not_used; @@ -5812,12 +5836,15 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table, */ log_drop_table(thd, &table->db, &table->table_name, create_info->tmp_table()); } - else if (write_bin_log(thd, res ? FALSE : TRUE, thd->query(), - thd->query_length(), is_trans)) + else if (res != 2) // Table was not dropped + { + if (write_bin_log(thd, res ? FALSE : TRUE, thd->query(), + thd->query_length(), is_trans)) res= 1; + } } - DBUG_RETURN(res); + DBUG_RETURN(res != 0); } diff --git a/sql/table.cc b/sql/table.cc index 6d4387aba8ac0..85c126098c03d 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -7964,10 +7964,7 @@ bool TABLE::insert_all_rows_into_tmp_table(THD *thd, } if (unlikely(thd->check_killed())) - { - thd->send_kill_message(); goto err_killed; - } } if (!tmp_table->no_rows && tmp_table->file->ha_end_bulk_insert()) goto err;