diff --git a/mysql-test/r/open_low_fail_on_rename.result b/mysql-test/r/open_low_fail_on_rename.result new file mode 100644 index 00000000000..66117b4e6e6 --- /dev/null +++ b/mysql-test/r/open_low_fail_on_rename.result @@ -0,0 +1,13 @@ +SET GLOBAL DEBUG="+d,debug_stats,dict_cache"; +SET GLOBAL DEBUG="+d,master_debug_evict"; +SET GLOBAL DEBUG="+d,stats_bg_debug_open"; +create table tx (c1 int, c2 varchar(10), primary key (c1)); +insert tx values (1,'a'), (2,'b'); +SET DEBUG_SYNC="now WAIT_FOR recalc_pool_get"; +SET DEBUG_SYNC="after_table_close_for_rename_tables SIGNAL dict_object_closed"; +SET DEBUG_SYNC="before_commit_for_rename_tables WAIT_FOR se_priv_id_resolved"; +SET DEBUG_SYNC="after_commit_for_rename_tables SIGNAL rename_commited"; +rename table tx to ty; +SET DEBUG_SYNC="now WAIT_FOR stats_updated"; +drop table ty; +SET GLOBAL DEBUG=RESET; diff --git a/mysql-test/t/open_low_fail_on_rename.test b/mysql-test/t/open_low_fail_on_rename.test new file mode 100644 index 00000000000..f03c8e907a6 --- /dev/null +++ b/mysql-test/t/open_low_fail_on_rename.test @@ -0,0 +1,26 @@ +SET GLOBAL DEBUG="+d,debug_stats,dict_cache"; + +#-- master thread +SET GLOBAL DEBUG="+d,master_debug_evict"; +#-- SET GLOBAL DEBUG="+d,dict_cache_force_out_all_can_be_evicted"; +#-- SET DEBUG_SYNC="before_evict_from_table_cache WAIT_FOR dict_object_closed"; +#-- SET DEBUG_SYNC="after_evict_from_table_cache SIGNAL dict_object_evicted"; + +#-- stats bg thread +SET GLOBAL DEBUG="+d,stats_bg_debug_open"; +#-- SET DEBUG_SYNC="stats_bg_before_table_open WAIT_FOR dict_object_evicted"; +#-- SET DEBUG_SYNC="after_table_open_low_se_private_id SIGNAL se_priv_id_resolved"; +#-- SET DEBUG_SYNC="before_table_open_low_mdl WAIT_FOR rename_commited"; + +#-- rename session +create table tx (c1 int, c2 varchar(10), primary key (c1)); +insert tx values (1,'a'), (2,'b'); +SET DEBUG_SYNC="now WAIT_FOR recalc_pool_get"; +SET DEBUG_SYNC="after_table_close_for_rename_tables SIGNAL dict_object_closed"; +SET DEBUG_SYNC="before_commit_for_rename_tables WAIT_FOR se_priv_id_resolved"; +SET DEBUG_SYNC="after_commit_for_rename_tables SIGNAL rename_commited"; +rename table tx to ty; +SET DEBUG_SYNC="now WAIT_FOR stats_updated"; +drop table ty; + +SET GLOBAL DEBUG=RESET; diff --git a/sql/sql_rename.cc b/sql/sql_rename.cc index e1792ec6b8e..79f794928da 100644 --- a/sql/sql_rename.cc +++ b/sql/sql_rename.cc @@ -386,9 +386,11 @@ bool mysql_rename_tables(THD *thd, Table_ref *table_list) { } if (!error && !int_commit_done) { + DEBUG_SYNC(thd, "before_commit_for_rename_tables"); error = (trans_commit_stmt(thd) || trans_commit_implicit(thd)); if (!error) { + DEBUG_SYNC(thd, "after_commit_for_rename_tables"); /* Don't try to invalidate foreign key parents on error, as we might miss necessary locks on them. diff --git a/sql/sql_thd_internal_api.h b/sql/sql_thd_internal_api.h index 764f6c38223..de516143825 100644 --- a/sql/sql_thd_internal_api.h +++ b/sql/sql_thd_internal_api.h @@ -38,6 +38,8 @@ #include "m_ctype.h" #include "mysql/components/services/bits/psi_thread_bits.h" #include "sql/handler.h" // enum_tx_isolation +#include "sql/current_thd.h" // current_thd +#include "sql/debug_sync.h" // debug_sync_* class THD; class partition_info; diff --git a/storage/innobase/dict/dict0dd.cc b/storage/innobase/dict/dict0dd.cc index 9b2968f9f4a..c1bed08ca63 100644 --- a/storage/innobase/dict/dict0dd.cc +++ b/storage/innobase/dict/dict0dd.cc @@ -539,6 +539,19 @@ static dict_table_t *dd_table_open_on_id_low(THD *thd, MDL_ticket **mdl, } } + /* RENAME TABLE closes the table before commit. Although it is protected + by exclusive MDL locks, the master thread of InnoDB only respects the + dict sys lock when making room for the dict cache. When openning a table + by id, a dict cache miss leads to open low. + Note that get_table_name_by_se_private_id() reads from storage and is not + blocked by an exclusive MDL lock. Here is the stop for MDL. + When the RENAME session commits and releases the MDL lock, we can move on + to acquire the MDL lock and DD object, however, with the old name. It is + doomed and should be retried. + */ + DEBUG_SYNC(thd, "after_table_open_low_se_private_id"); + DEBUG_SYNC(thd, "before_table_open_low_mdl"); + /* Now we have tablename, and MDL locked it if necessary. */ if (mdl != nullptr) { if (*mdl == nullptr && @@ -553,7 +566,8 @@ static dict_table_t *dd_table_open_on_id_low(THD *thd, MDL_ticket **mdl, if (mdl != nullptr) { dd_mdl_release(thd, mdl); } - return nullptr; + // The table could have been renamed. Retry. + continue; } const bool is_part = dd_table_is_partitioned(*dd_table); diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index b0d29612b8b..8b323ad1514 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -1192,6 +1192,8 @@ void dict_table_add_to_cache(dict_table_t *table, bool can_be_evicted) { ut_ad(dict_lru_validate()); ut_ad(dict_sys_mutex_own()); + DBUG_PRINT("dict_cache", ("add %lu %s", table->id, table->name.m_name)); + table->cached = true; const auto name_hash_value = ut::hash_string(table->name.m_name); @@ -1323,6 +1325,11 @@ ulint dict_make_room_in_cache( ulint check_up_to; ulint n_evicted = 0; + DBUG_EXECUTE_IF("dict_cache_force_out_all_can_be_evicted", { + max_tables = 0; + pct_check = 100; + }); + ut_a(pct_check > 0); ut_a(pct_check <= 100); ut_ad(dict_sys_mutex_own()); @@ -1882,6 +1889,8 @@ static void dict_table_remove_from_cache_low( dict_foreign_t *foreign; dict_index_t *index; + DBUG_PRINT("dict_cache", ("remove %lu %s", table->id, table->name.m_name)); + ut_ad(table); ut_ad(dict_lru_validate()); ut_a(table->get_ref_count() == 0); diff --git a/storage/innobase/dict/dict0stats_bg.cc b/storage/innobase/dict/dict0stats_bg.cc index 07b15f38884..76dda294f6d 100644 --- a/storage/innobase/dict/dict0stats_bg.cc +++ b/storage/innobase/dict/dict0stats_bg.cc @@ -275,16 +275,42 @@ static void dict_stats_process_entry_from_recalc_pool(THD *thd) { dict_table_t *table; MDL_ticket *mdl = nullptr; + DBUG_PRINT("debug_stats", ("recalc pool get %lu", table_id)); + + DBUG_EXECUTE_IF("stats_bg_debug_open", { + { + const char act[] = "stats_bg_before_table_open signal recalc_pool_get wait_for dict_object_evicted"; + assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act))); + } + { + const char act[] = "after_table_open_low_se_private_id signal se_priv_id_resolved"; + assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act))); + } + { + const char act[] = "before_table_open_low_mdl wait_for rename_commited"; + assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act))); + } + { + const char act[] = "stats_bg_after_update signal stats_updated"; + assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act))); + } + }); + + DEBUG_SYNC(thd, "stats_bg_before_table_open"); + /* We need to enter dict_sys->mutex for setting table->stats_bg_flag. This is for blocking other DDL, like drop table. */ dict_sys_mutex_enter(); table = dd_table_open_on_id(table_id, thd, &mdl, true, true); + DEBUG_SYNC(thd, "stats_bg_after_table_open"); + if (table == nullptr) { /* table does not exist, must have been DROPped after its id was enqueued */ dict_sys_mutex_exit(); + DBUG_EXECUTE_IF("stats_bg_debug_open", { assert(0); }); return; } @@ -313,10 +339,15 @@ static void dict_stats_process_entry_from_recalc_pool(THD *thd) { too frequent stats updates we put back the table on the auto recalc list and do nothing. */ + DBUG_PRINT("debug_stats", ("recalc pool add %lu %s", + table_id, table->name.m_name)); dict_stats_recalc_pool_add(table); } else { + DBUG_PRINT("debug_stats", ("update %lu %s", table_id, table->name.m_name)); dict_stats_update(table, DICT_STATS_RECALC_PERSISTENT); + + DEBUG_SYNC(thd, "stats_bg_after_update"); } dict_sys_mutex_enter(); @@ -354,6 +385,7 @@ void dict_stats_disabled_debug_update(THD *, SYS_VAR *, void *, the auto recalc list and proceeds them, eventually recalculating their statistics. */ void dict_stats_thread() { + DBUG_TRACE; ut_a(!srv_read_only_mode); THD *thd = create_internal_thd(); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index db5fb9bdd21..3da1d3f5051 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -14360,6 +14360,8 @@ int innobase_basic_ddl::rename_impl(THD *thd, const char *from, const char *to, dd_table_close(table, thd, nullptr, false); + DEBUG_SYNC(thd, "after_table_close_for_rename_tables"); + /* Serialize data dictionary operations with dictionary mutex: no deadlocks can occur then in these operations. */ diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index e21d972d954..67462dfa59b 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -1120,6 +1120,9 @@ static inline void row_update_statistics_if_needed( if (counter > n_rows / 10 /* 10% */ && dict_stats_auto_recalc_is_enabled(table)) { dict_stats_recalc_pool_add(table); + DBUG_PRINT("debug_stats", ("recalc pool add %lu %s (mod %lu)", + table->id, table->name.m_name, + table->stat_modified_counter)); table->stat_modified_counter = 0; } return; diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc index 4580a13f131..41b6acb4161 100644 --- a/storage/innobase/srv/srv0srv.cc +++ b/storage/innobase/srv/srv0srv.cc @@ -1994,6 +1994,20 @@ static ulint srv_master_evict_from_table_cache( { ulint n_tables_evicted = 0; + DBUG_EXECUTE_IF("master_debug_evict", { + { + const char act[] = "before_evict_from_table_cache wait_for dict_object_closed"; + assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act))); + } + { + const char act[] = "after_evict_from_table_cache signal dict_object_evicted"; + assert(!debug_sync_set_action(current_thd, STRING_WITH_LEN(act))); + } + DBUG_SET("+d,dict_cache_force_out_all_can_be_evicted"); + }); + + DEBUG_SYNC_C("before_evict_from_table_cache"); + rw_lock_x_lock(dict_operation_lock, UT_LOCATION_HERE); dict_mutex_enter_for_mysql(); @@ -2005,6 +2019,12 @@ static ulint srv_master_evict_from_table_cache( rw_lock_x_unlock(dict_operation_lock); + DEBUG_SYNC_C("after_evict_from_table_cache"); + + DBUG_EXECUTE_IF("master_debug_evict", { + DBUG_SET("-d,dict_cache_force_out_all_can_be_evicted"); + }); + return (n_tables_evicted); }