Not exactly. In 10.2 THD::~THD() only has
mysql_mutex_lock(&LOCK_thd_data);
mysql_mutex_unlock(&LOCK_thd_data);
it's not what Jan did. So I thought it'd be safer to leave it in place.
But 10.5 has
if (WSREP_NNULL(this)) mysql_mutex_lock(&LOCK_thd_data);
mysql_mutex_lock(&LOCK_thd_kill);
mysql_mutex_unlock(&LOCK_thd_kill);
if (WSREP_NNULL(this)) mysql_mutex_unlock(&LOCK_thd_data);
which is *exactly* what Jan did, so yes, when merging this chunk should
go away (and it's quite likely that you'll be doing this merge, so
please don't forget). Because of Jan's change in THD::~THD() there will
be a merge conflict in the destructor, so it cannot slip unnoticed and
auto-merged.
I do not follow merging this chunk should go away here.
Looking into KILL:
10.2: KILL find_thread_by_id acquires LOCK_THD_data and THD::set_killed aquires LOCK_thd_kill
10.3-10.6: KILL find_thread_by_id acquires LOCK_thd_data (wsrep) and LOCK_thd_data
THD::set_killed acquires LOCK_thd_kill
In all cases I do:
void THD::free_connection()
{
DBUG_ASSERT(free_connection_done == 0);
#ifdef WITH_WSREP
/*
wsrep_rgi should be protected with LOCK_thd_data mutex and
mutexing ordering rules state that LOCK_thd_data must be taken
before LOCK_thd_kill.
*/
if (wsrep_rgi) mysql_mutex_lock(&LOCK_thd_data);
#endif
/*
Other threads may have a lock on LOCK_thd_kill to ensure that this
THD is not deleted while they access it. The following mutex_lock
ensures that no one else is using this THD and it's now safe to
continue.
*/
mysql_mutex_lock(&LOCK_thd_kill);
mysql_mutex_unlock(&LOCK_thd_kill);
#ifdef WITH_WSREP
if (rpl_group_info *rgi= wsrep_rgi)
{
assert(WSREP(this));
wsrep_rgi= 0;
mysql_mutex_unlock(&LOCK_thd_data);
delete rgi;
}
#endif
Code looks like that because Marko requested not to do delete rgi under a mutex.
So do you mean that I should take LOCK_thd_data even when we do not have wsrep_rgi for wsrep as if I correctly understand
wsrep_rgi is there only for applier not all THDs with WSREP(thd).
R: Jan