mirror of
https://github.com/MariaDB/server.git
synced 2025-07-29 05:21:33 +03:00
MDEV-23328 Server hang due to Galera lock conflict resolution
Mutex order violation when wsrep bf thread kills a conflicting trx, the stack is wsrep_thd_LOCK() wsrep_kill_victim() lock_rec_other_has_conflicting() lock_clust_rec_read_check_and_lock() row_search_mvcc() ha_innobase::index_read() ha_innobase::rnd_pos() handler::ha_rnd_pos() handler::rnd_pos_by_record() handler::ha_rnd_pos_by_record() Rows_log_event::find_row() Update_rows_log_event::do_exec_row() Rows_log_event::do_apply_event() Log_event::apply_event() wsrep_apply_events() and mutexes are taken in the order lock_sys->mutex -> victim_trx->mutex -> victim_thread->LOCK_thd_data When a normal KILL statement is executed, the stack is innobase_kill_query() kill_handlerton() plugin_foreach_with_mask() ha_kill_query() THD::awake() kill_one_thread() and mutexes are victim_thread->LOCK_thd_data -> lock_sys->mutex -> victim_trx->mutex This patch is the plan D variant for fixing potetial mutex locking order exercised by BF aborting and KILL command execution. In this approach, KILL command is replicated as TOI operation. This guarantees total isolation for the KILL command execution in the first node: there is no concurrent replication applying and no concurrent DDL executing. Therefore there is no risk of BF aborting to happen in parallel with KILL command execution either. Potential mutex deadlocks between the different mutex access paths with KILL command execution and BF aborting cannot therefore happen. TOI replication is used, in this approach, purely as means to provide isolated KILL command execution in the first node. KILL command should not (and must not) be applied in secondary nodes. In this patch, we make this sure by skipping KILL execution in secondary nodes, in applying phase, where we bail out if applier thread is trying to execute KILL command. This is effective, but skipping the applying of KILL command could happen much earlier as well. This also fixed unprotected calls to wsrep_thd_abort that will use wsrep_abort_transaction. This is fixed by holding THD::LOCK_thd_data while we abort transaction. Reviewed-by: Jan Lindström <jan.lindstrom@mariadb.com>
This commit is contained in:
committed by
Oleksandr Byelkin
parent
d5bc05798f
commit
ef2dbb8dbc
@ -9248,7 +9248,7 @@ THD *find_thread_by_id(longlong id, bool query_id)
|
||||
return arg.thd;
|
||||
}
|
||||
|
||||
mysql_mutex_lock(&thd->LOCK_thd_data);
|
||||
|
||||
/**
|
||||
kill one thread.
|
||||
|
||||
@ -9292,7 +9292,8 @@ kill_one_thread(THD *thd, longlong id, killed_state kill_signal, killed_type typ
|
||||
faster and do a harder kill than KILL_SYSTEM_THREAD;
|
||||
*/
|
||||
|
||||
mysql_mutex_lock(&tmp->LOCK_thd_data); // for various wsrep* checks below
|
||||
mysql_mutex_lock(&tmp->LOCK_thd_data); // Lock from concurrent usage
|
||||
|
||||
#ifdef WITH_WSREP
|
||||
if (((thd->security_ctx->master_access & PRIV_KILL_OTHER_USER_PROCESS) ||
|
||||
thd->security_ctx->user_matches(tmp->security_ctx)) &&
|
||||
@ -9307,23 +9308,23 @@ kill_one_thread(THD *thd, longlong id, killed_state kill_signal, killed_type typ
|
||||
if (tmp->wsrep_aborter && tmp->wsrep_aborter != thd->thread_id)
|
||||
{
|
||||
/* victim is in hit list already, bail out */
|
||||
WSREP_DEBUG("victim has wsrep aborter: %lu, skipping awake()",
|
||||
tmp->wsrep_aborter);
|
||||
WSREP_DEBUG("victim %llu has wsrep aborter: %lu, skipping awake()",
|
||||
id, tmp->wsrep_aborter);
|
||||
error= 0;
|
||||
}
|
||||
else
|
||||
#endif /* WITH_WSREP */
|
||||
{
|
||||
WSREP_DEBUG("kill_one_thread %llu, victim: %llu wsrep_aborter %llu by signal %d",
|
||||
thd->thread_id, id, tmp->wsrep_aborter, kill_signal);
|
||||
WSREP_DEBUG("kill_one_thread victim: %llu wsrep_aborter %lu by signal %d",
|
||||
id, tmp->wsrep_aborter, kill_signal);
|
||||
tmp->awake_no_mutex(kill_signal);
|
||||
WSREP_DEBUG("victim: %llu taken care of", id);
|
||||
error= 0;
|
||||
}
|
||||
}
|
||||
else
|
||||
error= (type == KILL_TYPE_QUERY ? ER_KILL_QUERY_DENIED_ERROR :
|
||||
ER_KILL_DENIED_ERROR);
|
||||
|
||||
mysql_mutex_unlock(&tmp->LOCK_thd_data);
|
||||
}
|
||||
mysql_mutex_unlock(&tmp->LOCK_thd_kill);
|
||||
@ -9438,6 +9439,18 @@ static
|
||||
void sql_kill(THD *thd, longlong id, killed_state state, killed_type type)
|
||||
{
|
||||
uint error;
|
||||
#ifdef WITH_WSREP
|
||||
if (WSREP(thd))
|
||||
{
|
||||
WSREP_DEBUG("sql_kill called");
|
||||
if (thd->wsrep_applier)
|
||||
{
|
||||
WSREP_DEBUG("KILL in applying, bailing out here");
|
||||
return;
|
||||
}
|
||||
WSREP_TO_ISOLATION_BEGIN(WSREP_MYSQL_DB, NULL, NULL)
|
||||
}
|
||||
#endif /* WITH_WSREP */
|
||||
if (likely(!(error= kill_one_thread(thd, id, state, type))))
|
||||
{
|
||||
if (!thd->killed)
|
||||
@ -9447,6 +9460,11 @@ void sql_kill(THD *thd, longlong id, killed_state state, killed_type type)
|
||||
}
|
||||
else
|
||||
my_error(error, MYF(0), id);
|
||||
#ifdef WITH_WSREP
|
||||
return;
|
||||
wsrep_error_label:
|
||||
my_error(ER_CANNOT_USER, MYF(0), wsrep_thd_query(thd));
|
||||
#endif /* WITH_WSREP */
|
||||
}
|
||||
|
||||
|
||||
@ -9455,6 +9473,18 @@ sql_kill_user(THD *thd, LEX_USER *user, killed_state state)
|
||||
{
|
||||
uint error;
|
||||
ha_rows rows;
|
||||
#ifdef WITH_WSREP
|
||||
if (WSREP(thd))
|
||||
{
|
||||
WSREP_DEBUG("sql_kill_user called");
|
||||
if (thd->wsrep_applier)
|
||||
{
|
||||
WSREP_DEBUG("KILL in applying, bailing out here");
|
||||
return;
|
||||
}
|
||||
WSREP_TO_ISOLATION_BEGIN(WSREP_MYSQL_DB, NULL, NULL)
|
||||
}
|
||||
#endif /* WITH_WSREP */
|
||||
if (likely(!(error= kill_threads_for_user(thd, user, state, &rows))))
|
||||
my_ok(thd, rows);
|
||||
else
|
||||
@ -9465,6 +9495,11 @@ sql_kill_user(THD *thd, LEX_USER *user, killed_state state)
|
||||
*/
|
||||
my_error(error, MYF(0), user->host.str, user->user.str);
|
||||
}
|
||||
#ifdef WITH_WSREP
|
||||
return;
|
||||
wsrep_error_label:
|
||||
my_error(ER_CANNOT_USER, MYF(0), user->user.str);
|
||||
#endif /* WITH_WSREP */
|
||||
}
|
||||
|
||||
|
||||
|
Reference in New Issue
Block a user