Hi Sergei,
After QA runs done by Ramesh, we now know the latest fix candidate i.e.
what is in bb-10.2-MDEV-25114-galera-v2 is incorrect. Problem is in
wsrep_close_connections() as it holds LOCK_thread_count while it does
abort_replicated that will call wsrep_abort_transaction and there we use
find_thread_by_id that would also take LOCK_thread_count. As there is
another code path here, the problem is not easily fixed. We can't just
release LOCK_thread_count at wsrep_close_connections as we iterate the
thread list.
I must say I'm not sure what to do now.
(gdb) bt
#0 __pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at
../sysdeps/unix/sysv/linux/pthread_kill.c:56
#1 0x000056333963a2e8 in my_write_core (sig=sig@entry=6) at
/test/mtest/10.2_dbg/mysys/stacktrace.c:382
#2 0x0000563338f2993d in handle_fatal_signal (sig=6) at
/test/mtest/10.2_dbg/sql/signal_handler.cc:355
#3 <signal handler called>
#4 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#5 0x000014b799572859 in __GI_abort () at abort.c:79
#6 0x000056333963edc4 in safe_mutex_lock (mp=0x563339e47220
Hi, Jan!
On Oct 06, Jan Lindström wrote:
+/* This is wrapper for wsrep_break_lock in thr_lock.c */ +static int wsrep_thr_abort_thd(void *bf_thd_ptr, void
*victim_thd_ptr, my_bool signal)
+{ + THD* victim_thd= (THD *) victim_thd_ptr; + /* We need to lock THD::LOCK_thd_data to protect victim + from concurrent usage or disconnect or delete. */
How do you know victim_thd wasn't deleted before you locked LOCK_thd_data below?
I must say the thr_lock code is not familiar to me but there are mysql_mutex_lock() calls to lock->mutex. After code review it is not clear to me what that mutex is.
where are mysql_mutex_lock() calls to lock->mutex?
mysys/thr_lock.c there is function thr_lock() there is call to mysql_mutex_lock(&lock->mutex); this is before wsrep_break_lock where we call wsrep_thd_abort
this is for table locks. `lock` is `data->lock` where `data` is THR_LOCK structure somewhere in the table share. It locks tables and handlers, not threads. And InnoDB isn't using it at all anyway.
if (victim_trx) { + wsrep_thd_UNLOCK(victim_thd);
what keeps victim_trx from disappearing here?
Nothing. Do you have suggestions ?
A couple of thoughts:
* Why do you unlock LOCK_thd_data here at all? I believed the whole point of using TOI was to make sure that even if you lock mutexes in a different order, it will not cause a deadlock.
This is DDL-case when we have a MDL-conflict not user KILL, we need to release it in my opinion because we need to take mutexes in lock_sys -> trx -> THD::LOCK_thd_data order, if I do not release I can see easily safe_mutex warnings
I don't understand. First, lock_sys and trx mutexes are not covered by safe_mutex, so it cannot possibly warn about them.
Second, the reason for taking mutexes always in the same order is to avoid deadlocks. And yor TOI trick should archieve that.
* You cannot pass a THD safely over a gap where no locks keep it in existence. You can pass an integer, thread id, and use find_thread_by_id later.
I have a suggestion for this:
if (victim_trx) { + + const trx_id_t victim_trx_id= victim_trx->id; + const longlong victim_thread= thd_get_thread_id(victim_thd); lock_mutex_enter(); - trx_mutex_enter(victim_trx); - wsrep_innobase_kill_one_trx(bf_thd, bf_trx, victim_trx, signal); + if (trx_t* victim= trx_rw_is_active(victim_trx_id, NULL, true)) // If this succeeds, trx can not go away + { + trx_mutex_enter(victim); + ut_ad(victim->mysql_thd ? + thd_get_thread_id(victim->mysql_thd) == victim_thread : + 1); + wsrep_innobase_kill_one_trx(bf_thd, bf_trx, victim, signal); + trx_mutex_exit(victim); + victim->release_reference(); // Now trx can go away after we have released lock_sys + wsrep_srv_conc_cancel_wait(victim); + } lock_mutex_exit(); - trx_mutex_exit(victim_trx); - wsrep_srv_conc_cancel_wait(victim_trx); DBUG_VOID_RETURN;
I don't understand, sorry. It's not quite clear what this patch should apply to. May be you can paste what the new code should look like, not a patch?
In particular, where are wsrep_thd_UNLOCK and wsrep_thd_LOCK. Are they present in this new variant at all?
By the way, how long can WSREP_TO_ISOLATION_BEGIN() take? Does it need to wait for everything else being replicated?
As long as it takes to start it on all nodes in the cluster.
So, KILL basically won't work when a cluster is starting. Can bf aborts happen during a cluster startup?
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org