Hi, Sergey! On Jan 31, Sergey Vojtovich wrote:
At lp:maria/10.0
------------------------------------------------------------ revno: 3967 revision-id: svoj@mariadb.org-20140131141943-fjyyih3h92lek62n parent: svoj@mariadb.org-20140130124905-vy28zbs9swlpg8uq committer: Sergey Vojtovich <svoj@mariadb.org> branch nick: 10.0 timestamp: Fri 2014-01-31 18:19:43 +0400 message: MDEV-5597 - Reduce usage of LOCK_open: LOCK_flush
Replaced LOCK_flush with per-share conditional variable. === modified file 'sql/table_cache.cc' --- a/sql/table_cache.cc 2014-01-30 12:49:05 +0000 +++ b/sql/table_cache.cc 2014-01-31 14:19:43 +0000 @@ -945,6 +949,7 @@ bool tdc_remove_table(THD *thd, enum_tdc if ((share= tdc_delete_share(db, table_name))) { I_P_List <TABLE, TABLE_share> purge_tables; + uint my_refs= 1;
mysql_mutex_lock(&LOCK_open); /* @@ -969,28 +974,48 @@ bool tdc_remove_table(THD *thd, enum_tdc if (kill_delayed_threads) kill_delayed_threads_for_table(share);
-#ifndef DBUG_OFF - if (remove_type == TDC_RT_REMOVE_NOT_OWN) + if (remove_type == TDC_RT_REMOVE_NOT_OWN || + remove_type == TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE) { TABLE_SHARE::All_share_tables_list::Iterator it(share->tdc.all_tables); while ((table= it++)) + { + my_refs++; DBUG_ASSERT(table->in_use == thd); + } } -#endif - - mysql_rwlock_rdlock(&LOCK_flush); + DBUG_ASSERT(share->tdc.all_tables.is_empty() || remove_type != TDC_RT_REMOVE_ALL); mysql_mutex_unlock(&LOCK_open);
while ((table= purge_tables.pop_front())) intern_close_table(table); - mysql_rwlock_unlock(&LOCK_flush);
- DBUG_ASSERT(share->tdc.all_tables.is_empty() || remove_type != TDC_RT_REMOVE_ALL); - tdc_release_share(share); + if (remove_type != TDC_RT_REMOVE_UNUSED) + { + /* + Even though current thread holds exclusive metadata lock on this share + (asserted above), concurrent FLUSH TABLES threads may be in process of + closing unused table instances belonging to this share. E.g.: + thr1 (FLUSH TABLES): table= share->tdc.free_tables.pop_front(); + thr1 (FLUSH TABLES): share->tdc.all_tables.remove(table); + thr2 (ALTER TABLE): tdc_remove_table(); + thr1 (FLUSH TABLES): intern_close_table(table); + + Current remove type assumes that all table instances (except for those + that are owned by current thread) must be closed before + thd_remove_table() returns. Wait for such tables here. + + Unfortunately TABLE_SHARE::wait_for_old_version() cannot be used here + because it waits for all table instances, whereas we have to wait only + for those that are not owned by current thread. + */ + mysql_mutex_lock(&share->tdc.LOCK_table_share); + while (share->tdc.ref_count > my_refs) + mysql_cond_wait(&share->tdc.COND_release, &share->tdc.LOCK_table_share); + mysql_mutex_unlock(&share->tdc.LOCK_table_share);
I'm sorry, I don't understand that at all. Why my_refs is only incremented in debug builds? In non-debug builds you wait until all tables but one are closed, in debug builds you may keep more than one table opened. You wait here on a condition, how do you know there's FLUSH TABLES (or anything at all) running that will signal this condition?
+ }
- /* Wait for concurrent threads to free unused objects. */ - mysql_rwlock_wrlock(&LOCK_flush); - mysql_rwlock_unlock(&LOCK_flush); + tdc_release_share(share);
found= true; }
Regards, Sergei