From 7a947614fbf8b925ae3df70ec8df80c745eafd4c Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Tue, 17 Dec 2019 16:25:15 +0400 Subject: [PATCH] Split tdc_remove_table() TDC_RT_REMOVE_ALL -> tdc_remove_table(). Some occurrences replaced with TDC_element::flush() (whenver TABLE_SHARE is available). TDC_RT_REMOVE_NOT_OWN[_KEEP_SHARE] -> TDC_element::flush(). These modes assume that current thread owns TABLE_SHARE reference, which means we can avoid hash lookup and flush unused TABLE instances directly. TDC_RT_REMOVE_UNUSED -> TDC_element::flush_unused(). Only [ab]used by mysql_admin_table() currently. Should be removed eventually. Part of MDEV-17882 - Cleanup refresh version --- sql/rpl_gtid.cc | 3 +- sql/sql_admin.cc | 3 +- sql/sql_base.cc | 16 +-- sql/sql_partition_admin.cc | 3 +- sql/sql_rename.cc | 6 +- sql/sql_table.cc | 6 +- sql/sql_truncate.cc | 3 +- sql/sql_view.cc | 6 +- sql/table_cache.cc | 206 ++++++++++++++++--------------- sql/table_cache.h | 15 +-- storage/heap/ha_heap.cc | 2 +- storage/tokudb/tokudb_dir_cmd.cc | 2 +- 12 files changed, 127 insertions(+), 144 deletions(-) diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc index be347cc5fad..c5f765f9cda 100644 --- a/sql/rpl_gtid.cc +++ b/sql/rpl_gtid.cc @@ -430,8 +430,7 @@ rpl_slave_state::truncate_state_table(THD *thd) MYSQL_OPEN_IGNORE_LOGGING_FORMAT))) { DBUG_ASSERT(!tlist.table->file->row_logging); - tdc_remove_table(thd, TDC_RT_REMOVE_NOT_OWN, "mysql", - rpl_gtid_slave_state_table_name.str); + tlist.table->s->tdc->flush(thd, true); err= tlist.table->file->ha_truncate(); if (err) diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc index cdec2c08e5c..ea455e43ffc 100644 --- a/sql/sql_admin.cc +++ b/sql/sql_admin.cc @@ -1156,8 +1156,7 @@ send_result_message: } else if (open_for_modify || fatal_error) { - tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED, - table->db.str, table->table_name.str); + table->table->s->tdc->flush_unused(true); /* May be something modified. Consequently, we have to invalidate the query cache. diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 770975f79fe..664071a27f5 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -442,8 +442,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, DBUG_RETURN(true); for (TABLE_LIST *table= tables; table; table= table->next_local) - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db.str, - table->table_name.str); + tdc_remove_table(thd, table->db.str, table->table_name.str); } DBUG_RETURN(false); } @@ -1310,8 +1309,7 @@ bool wait_while_table_is_used(THD *thd, TABLE *table, thd->variables.lock_wait_timeout)) DBUG_RETURN(TRUE); - tdc_remove_table(thd, TDC_RT_REMOVE_NOT_OWN, - table->s->db.str, table->s->table_name.str); + table->s->tdc->flush(thd, true); /* extra() call must come only after all instances above are closed */ if (function != HA_EXTRA_NOT_USED) (void) table->file->extra(function); @@ -1350,9 +1348,8 @@ void drop_open_table(THD *thd, TABLE *table, const LEX_CSTRING *db_name, handlerton *table_type= table->s->db_type(); table->file->extra(HA_EXTRA_PREPARE_FOR_DROP); + table->s->tdc->flush(thd, true); close_thread_table(thd, &thd->open_tables); - /* Remove the table share from the table cache. */ - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, db_name->str, table_name->str); /* Remove the table from the storage engine and rm the .frm. */ quick_rm_table(thd, table_type, db_name, table_name, 0); } @@ -2947,8 +2944,7 @@ static bool auto_repair_table(THD *thd, TABLE_LIST *table_list) tdc_release_share(share); /* Remove the repaired share from the table cache. */ - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, - table_list->db.str, table_list->table_name.str); + tdc_remove_table(thd, table_list->db.str, table_list->table_name.str); end_free: my_free(entry); return result; @@ -3119,7 +3115,7 @@ Open_table_context::recover_from_failed_open() get_timeout(), 0))) break; - tdc_remove_table(m_thd, TDC_RT_REMOVE_ALL, m_failed_table->db.str, + tdc_remove_table(m_thd, m_failed_table->db.str, m_failed_table->table_name.str); m_thd->get_stmt_da()->clear_warning_info(m_thd->query_id); @@ -3155,7 +3151,7 @@ Open_table_context::recover_from_failed_open() get_timeout(), 0))) break; - tdc_remove_table(m_thd, TDC_RT_REMOVE_ALL, m_failed_table->db.str, + tdc_remove_table(m_thd, m_failed_table->db.str, m_failed_table->table_name.str); result= auto_repair_table(m_thd, m_failed_table); diff --git a/sql/sql_partition_admin.cc b/sql/sql_partition_admin.cc index 92ac1b2ce4c..ed77c0938f3 100644 --- a/sql/sql_partition_admin.cc +++ b/sql/sql_partition_admin.cc @@ -825,8 +825,7 @@ bool Sql_cmd_alter_table_truncate_partition::execute(THD *thd) if (thd->mdl_context.upgrade_shared_lock(ticket, MDL_EXCLUSIVE, timeout)) DBUG_RETURN(TRUE); - tdc_remove_table(thd, TDC_RT_REMOVE_NOT_OWN, first_table->db.str, - first_table->table_name.str); + first_table->table->s->tdc->flush(thd, true); partition= (ha_partition*) first_table->table->file; /* Invoke the handler method responsible for truncating the partition. */ diff --git a/sql/sql_rename.cc b/sql/sql_rename.cc index 8d125fbfa06..0200113deae 100644 --- a/sql/sql_rename.cc +++ b/sql/sql_rename.cc @@ -309,8 +309,7 @@ do_rename(THD *thd, TABLE_LIST *ren_table, const LEX_CSTRING *new_db, Shared table. Just drop the old .frm as it's not correct anymore Discovery will find the old table when it's accessed */ - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, - ren_table->db.str, ren_table->table_name.str); + tdc_remove_table(thd, ren_table->db.str, ren_table->table_name.str); quick_rm_table(thd, 0, &ren_table->db, &old_alias, FRM_ONLY, 0); DBUG_RETURN(0); } @@ -329,8 +328,7 @@ do_rename(THD *thd, TABLE_LIST *ren_table, const LEX_CSTRING *new_db, DBUG_RETURN(1); #endif - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, - ren_table->db.str, ren_table->table_name.str); + tdc_remove_table(thd, ren_table->db.str, ren_table->table_name.str); if (hton != view_pseudo_hton) { diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 3b268a04110..13aa3b050bc 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2485,8 +2485,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, table->table= 0; } else - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db.str, - table->table_name.str); + tdc_remove_table(thd, table->db.str, table->table_name.str); /* Check that we have an exclusive lock on the table to be dropped. */ DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, table->db.str, @@ -7753,8 +7752,7 @@ static bool mysql_inplace_alter_table(THD *thd, thd->variables.lock_wait_timeout)) goto cleanup; - tdc_remove_table(thd, TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE, - table->s->db.str, table->s->table_name.str); + table->s->tdc->flush(thd, false); } /* diff --git a/sql/sql_truncate.cc b/sql/sql_truncate.cc index a47822e43ca..1d2ec66eae6 100644 --- a/sql/sql_truncate.cc +++ b/sql/sql_truncate.cc @@ -375,8 +375,7 @@ bool Sql_cmd_truncate_table::lock_table(THD *thd, TABLE_LIST *table_ref, else { /* Table is already locked exclusively. Remove cached instances. */ - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table_ref->db.str, - table_ref->table_name.str); + tdc_remove_table(thd, table_ref->db.str, table_ref->table_name.str); } DBUG_RETURN(FALSE); diff --git a/sql/sql_view.cc b/sql/sql_view.cc index b2e977151fd..db215715ff4 100644 --- a/sql/sql_view.cc +++ b/sql/sql_view.cc @@ -655,8 +655,7 @@ bool mysql_create_view(THD *thd, TABLE_LIST *views, */ if (!res) - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, view->db.str, - view->table_name.str); + tdc_remove_table(thd, view->db.str, view->table_name.str); if (!res && mysql_bin_log.is_open()) { @@ -1876,8 +1875,7 @@ bool mysql_drop_view(THD *thd, TABLE_LIST *views, enum_drop_mode drop_mode) For a view, there is a TABLE_SHARE object. Remove it from the table definition cache, in case the view was cached. */ - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, view->db.str, - view->table_name.str); + tdc_remove_table(thd, view->db.str, view->table_name.str); query_cache_invalidate3(thd, view, 0); sp_cache_invalidate(); } diff --git a/sql/table_cache.cc b/sql/table_cache.cc index 4dda19c0c28..82c60359e88 100644 --- a/sql/table_cache.cc +++ b/sql/table_cache.cc @@ -997,36 +997,9 @@ void tdc_release_share(TABLE_SHARE *share) /** - Remove all or some (depending on parameter) instances of TABLE and - TABLE_SHARE from the table definition cache. + Removes all TABLE instances and corresponding TABLE_SHARE @param thd Thread context - @param remove_type Type of removal: - TDC_RT_REMOVE_ALL - remove all TABLE instances and - TABLE_SHARE instance. There - should be no used TABLE objects - and caller should have exclusive - metadata lock on the table. - TDC_RT_REMOVE_NOT_OWN - remove all TABLE instances - except those that belong to - this thread. There should be - no TABLE objects used by other - threads and caller should have - exclusive metadata lock on the - table. - TDC_RT_REMOVE_UNUSED - remove all unused TABLE - instances (if there are no - used instances will also - remove TABLE_SHARE). - TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE - - remove all TABLE instances - except those that belong to - this thread, but don't mark - TABLE_SHARE as old. There - should be no TABLE objects - used by other threads and - caller should have exclusive - metadata lock on the table. @param db Name of database @param table_name Name of table @@ -1034,27 +1007,21 @@ void tdc_release_share(TABLE_SHARE *share) (other) thread (this should be achieved by using meta-data locks). */ -bool tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, - const char *db, const char *table_name) +void tdc_remove_table(THD *thd, const char *db, const char *table_name) { Share_free_tables::List purge_tables; - TABLE *table; TDC_element *element; - uint my_refs= 1; - bool res= false; DBUG_ENTER("tdc_remove_table"); - DBUG_PRINT("enter",("name: %s remove_type: %d", table_name, remove_type)); + DBUG_PRINT("enter", ("name: %s", table_name)); - DBUG_ASSERT(remove_type == TDC_RT_REMOVE_UNUSED || - thd->mdl_context.is_lock_owner(MDL_key::TABLE, db, table_name, + DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, db, table_name, MDL_EXCLUSIVE)); mysql_mutex_lock(&LOCK_unused_shares); if (!(element= tdc_lock_share(thd, db, table_name))) { mysql_mutex_unlock(&LOCK_unused_shares); - DBUG_ASSERT(remove_type != TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE); - DBUG_RETURN(false); + DBUG_VOID_RETURN; } DBUG_ASSERT(element != MY_ERRPTR); // What can we do about it? @@ -1070,80 +1037,24 @@ bool tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, mysql_mutex_unlock(&LOCK_unused_shares); tdc_delete_share_from_hash(element); - DBUG_RETURN(false); + DBUG_VOID_RETURN; } mysql_mutex_unlock(&LOCK_unused_shares); element->ref_count++; - if (remove_type != TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE) - element->flushed= true; tc_remove_all_unused_tables(element, &purge_tables); - - if (remove_type == TDC_RT_REMOVE_NOT_OWN || - remove_type == TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE) - { - All_share_tables_list::Iterator it(element->all_tables); - while ((table= it++)) - { - if (table->in_use == thd) - my_refs++; - } - } mysql_mutex_unlock(&element->LOCK_table_share); - while ((table= purge_tables.pop_front())) + while (auto table= purge_tables.pop_front()) intern_close_table(table); - 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 now. - - intern_close_table() decrements ref_count and signals COND_release. When - ref_count drops down to number of references owned by current thread - waiting is completed. - - 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(&element->LOCK_table_share); - while (element->ref_count > my_refs) - mysql_cond_wait(&element->COND_release, &element->LOCK_table_share); - DBUG_ASSERT(element->all_tables.is_empty() || - remove_type != TDC_RT_REMOVE_ALL); -#ifndef DBUG_OFF - if (remove_type == TDC_RT_REMOVE_NOT_OWN || - remove_type == TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE) - { - All_share_tables_list::Iterator it(element->all_tables); - while ((table= it++)) - DBUG_ASSERT(table->in_use == thd); - } -#endif - mysql_mutex_unlock(&element->LOCK_table_share); - } - else - { - mysql_mutex_lock(&element->LOCK_table_share); - res= element->ref_count > 1; - mysql_mutex_unlock(&element->LOCK_table_share); - } - - tdc_release_share(element->share); - - DBUG_RETURN(res); + mysql_mutex_lock(&element->LOCK_table_share); + element->wait_for_refs(1); + DBUG_ASSERT(element->all_tables.is_empty()); + element->ref_count--; + tdc_delete_share_from_hash(element); + DBUG_VOID_RETURN; } @@ -1284,3 +1195,94 @@ int show_tc_active_instances(THD *thd, SHOW_VAR *var, char *buff, tc_active_instances.load(std::memory_order_relaxed); return 0; } + + +/** + Waits until ref_count goes down to given number + + @param my_refs Number of references owned by the caller + + Caller must own at least one TABLE_SHARE reference. + + Even though current thread holds exclusive metadata lock on this share, + 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 now. + + intern_close_table() decrements ref_count and signals COND_release. When + ref_count drops down to number of references owned by current thread + waiting is completed. + + 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. +*/ + +void TDC_element::wait_for_refs(uint my_refs) +{ + while (ref_count > my_refs) + mysql_cond_wait(&COND_release, &LOCK_table_share); +} + + +/** + Flushes unused TABLE instances + + @param thd Thread context + @param mark_flushed Whether to destroy TABLE_SHARE when released + + Caller is allowed to own used TABLE instances. + There must be no TABLE objects used by other threads and caller must own + exclusive metadata lock on the table. +*/ + +void TDC_element::flush(THD *thd, bool mark_flushed) +{ + DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, share->db.str, + share->table_name.str, + MDL_EXCLUSIVE)); + + flush_unused(mark_flushed); + + mysql_mutex_lock(&LOCK_table_share); + All_share_tables_list::Iterator it(all_tables); + uint my_refs= 0; + while (auto table= it++) + { + if (table->in_use == thd) + my_refs++; + } + wait_for_refs(my_refs); +#ifndef DBUG_OFF + it.rewind(); + while (auto table= it++) + DBUG_ASSERT(table->in_use == thd); +#endif + mysql_mutex_unlock(&LOCK_table_share); +} + + +/** + Flushes unused TABLE instances +*/ + +void TDC_element::flush_unused(bool mark_flushed) +{ + Share_free_tables::List purge_tables; + + mysql_mutex_lock(&LOCK_table_share); + if (mark_flushed) + flushed= true; + tc_remove_all_unused_tables(this, &purge_tables); + mysql_mutex_unlock(&LOCK_table_share); + + while (auto table= purge_tables.pop_front()) + intern_close_table(table); +} diff --git a/sql/table_cache.h b/sql/table_cache.h index cf880b7d9b4..bffb782849c 100644 --- a/sql/table_cache.h +++ b/sql/table_cache.h @@ -54,17 +54,13 @@ struct TDC_element /** Avoid false sharing between TDC_element and free_tables */ char pad[CPU_LEVEL1_DCACHE_LINESIZE]; Share_free_tables free_tables[1]; + + inline void wait_for_refs(uint my_refs); + void flush(THD *thd, bool mark_flushed); + void flush_unused(bool mark_flushed); }; -enum enum_tdc_remove_table_type -{ - TDC_RT_REMOVE_ALL, - TDC_RT_REMOVE_NOT_OWN, - TDC_RT_REMOVE_UNUSED, - TDC_RT_REMOVE_NOT_OWN_KEEP_SHARE -}; - extern ulong tdc_size; extern ulong tc_size; extern uint32 tc_instances; @@ -81,8 +77,7 @@ int tdc_share_is_cached(THD *thd, const char *db, const char *table_name); extern TABLE_SHARE *tdc_acquire_share(THD *thd, TABLE_LIST *tl, uint flags, TABLE **out_table= 0); extern void tdc_release_share(TABLE_SHARE *share); -extern bool tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, - const char *db, const char *table_name); +void tdc_remove_table(THD *thd, const char *db, const char *table_name); extern int tdc_wait_for_old_version(THD *thd, const char *db, const char *table_name, diff --git a/storage/heap/ha_heap.cc b/storage/heap/ha_heap.cc index 34e2799f71c..ee2c9f0d916 100644 --- a/storage/heap/ha_heap.cc +++ b/storage/heap/ha_heap.cc @@ -24,7 +24,7 @@ #include "sql_priv.h" #include "sql_plugin.h" #include "ha_heap.h" -#include "sql_base.h" // enum_tdc_remove_table_type +#include "sql_base.h" static handler *heap_create_handler(handlerton *hton, TABLE_SHARE *table, diff --git a/storage/tokudb/tokudb_dir_cmd.cc b/storage/tokudb/tokudb_dir_cmd.cc index 0f509fe0d39..871bb712406 100644 --- a/storage/tokudb/tokudb_dir_cmd.cc +++ b/storage/tokudb/tokudb_dir_cmd.cc @@ -67,7 +67,7 @@ static int MDL_and_TDC(THD *thd, table); return error; } - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, db, table); + tdc_remove_table(thd, db, table); return error; }