Hi, Jan! On Dec 20, Jan Lindström wrote: > 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. ... > 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).
No, I mean that 10.5 has the the same code in THD::~THD() that you have added to free_connection() in 10.2. So when your patch will be merged to 10.5, this old code should be removed from THD::~THD(), there's no need to lock same mutexes twice. Here's your code: https://github.com/MariaDB/server/blob/cede9020fa/sql/sql_class.cc#L1617-L1642 so this old code (in the same file, a bit down) can be removed: https://github.com/MariaDB/server/blob/cede9020fa/sql/sql_class.cc#L1725-L1728 Regards, Sergei VP of MariaDB Server Engineering and secur...@mariadb.org _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
