maria-developers team mailing list archive

[Jan Lindström <jan.lindstrom@xxxxxxxxxxx>]

Hi Sergei,


>
>
> 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.

10.2 :
https://github.com/MariaDB/server/commit/d112c363b51768b9e1d7bc7c600a4358a10dee9f
10.3 :
https://github.com/MariaDB/server/commit/79b22723d87459f2bf6265b7fcc163ef63aa6e5f
10.6 :
https://github.com/MariaDB/server/commit/cede9020fa7da400bb285780ae6d8a3e15abe3e4

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