Skip to content

Commit

Permalink
MDEV-16286 Killed CREATE SEQUENCE leaves sequence in unusable state
Browse files Browse the repository at this point in the history
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().
  • Loading branch information
montywi committed May 27, 2018
1 parent b3a2761 commit 58721c3
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 38 deletions.
1 change: 0 additions & 1 deletion sql/group_by_handler.cc
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion sql/handler.cc
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion sql/log.cc
Expand Up @@ -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",
Expand Down
5 changes: 2 additions & 3 deletions sql/rpl_gtid.cc
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down
12 changes: 4 additions & 8 deletions sql/rpl_parallel.cc
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -573,7 +571,6 @@ rpl_pause_for_ftwrl(THD *thd)
{
if (unlikely(thd->check_killed()))
{
thd->send_kill_message();
err= 1;
break;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -2405,7 +2402,6 @@ rpl_parallel::wait_for_workers_idle(THD *thd)
{
if (unlikely(thd->check_killed()))
{
thd->send_kill_message();
err= 1;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion sql/sql_class.cc
Expand Up @@ -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)
{
Expand Down
6 changes: 5 additions & 1 deletion sql/sql_class.h
Expand Up @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions sql/sql_join_cache.cc
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
13 changes: 1 addition & 12 deletions sql/sql_select.cc
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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 */
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
37 changes: 32 additions & 5 deletions sql/sql_table.cc
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}


Expand Down
3 changes: 0 additions & 3 deletions sql/table.cc
Expand Up @@ -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;
Expand Down

0 comments on commit 58721c3

Please sign in to comment.