From bfdd30d3e92d3bf5570cc050967098c6b5a2d73a Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Sat, 21 Dec 2019 01:22:09 +0400 Subject: [PATCH] Fixed close_cached_connection_tables() flushing Let DROP SERVER and ALTER SERVER perform fair affected tables flushing. That is acquire MDL_EXCLUSIVE and do tdc_remove_table(TDC_RT_REMOVE_ALL). Aim of this patch is elimination of another inconsistent use of TDC_RT_REMOVE_UNUSED. It fixes (to some extent) a problem described in the beginning of sql_server.cc, when close_cached_connection_tables() interferes with concurrent transaction. A better fix should probably introduce proper MDL locks for server objects? Part of MDEV-17882 - Cleanup refresh version --- sql/sql_base.cc | 87 ---------------------------------------------- sql/sql_base.h | 1 - sql/sql_servers.cc | 73 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 88 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 3a04c14dee993..770975f79fed6 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -630,93 +630,6 @@ bool flush_tables(THD *thd, flush_tables_type flag) } -/** - Close all tables which match specified connection string or - if specified string is NULL, then any table with a connection string. -*/ - -struct close_cached_connection_tables_arg -{ - THD *thd; - LEX_CSTRING *connection; - TABLE_LIST *tables; -}; - - -static my_bool close_cached_connection_tables_callback( - TDC_element *element, close_cached_connection_tables_arg *arg) -{ - TABLE_LIST *tmp; - - mysql_mutex_lock(&element->LOCK_table_share); - /* Ignore if table is not open or does not have a connect_string */ - if (!element->share || !element->share->connect_string.length || - !element->ref_count) - goto end; - - /* Compare the connection string */ - if (arg->connection && - (arg->connection->length > element->share->connect_string.length || - (arg->connection->length < element->share->connect_string.length && - (element->share->connect_string.str[arg->connection->length] != '/' && - element->share->connect_string.str[arg->connection->length] != '\\')) || - strncasecmp(arg->connection->str, element->share->connect_string.str, - arg->connection->length))) - goto end; - - /* close_cached_tables() only uses these elements */ - if (!(tmp= (TABLE_LIST*) alloc_root(arg->thd->mem_root, sizeof(TABLE_LIST))) || - !(arg->thd->make_lex_string(&tmp->db, element->share->db.str, element->share->db.length)) || - !(arg->thd->make_lex_string(&tmp->table_name, element->share->table_name.str, - element->share->table_name.length))) - { - mysql_mutex_unlock(&element->LOCK_table_share); - return TRUE; - } - - tmp->next_local= arg->tables; - arg->tables= tmp; - -end: - mysql_mutex_unlock(&element->LOCK_table_share); - return FALSE; -} - - -/** - Close cached connections - - @return false ok - @return true If there was an error from closed_cached_connection_tables or - if there was any open connections that we had to force closed -*/ - -bool close_cached_connection_tables(THD *thd, LEX_CSTRING *connection) -{ - bool res= false; - close_cached_connection_tables_arg argument; - DBUG_ENTER("close_cached_connections"); - DBUG_ASSERT(thd); - - argument.thd= thd; - argument.connection= connection; - argument.tables= NULL; - - if (tdc_iterate(thd, - (my_hash_walk_action) close_cached_connection_tables_callback, - &argument)) - DBUG_RETURN(true); - - for (TABLE_LIST *table= argument.tables; table; table= table->next_local) - res|= tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED, - table->db.str, - table->table_name.str); - - /* Return true if we found any open connections */ - DBUG_RETURN(res); -} - - /* Mark all tables in the list which were used by current substatement as free for reuse. diff --git a/sql/sql_base.h b/sql/sql_base.h index 929dc24507386..773b95b831768 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -306,7 +306,6 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool wait_for_refresh, ulong timeout); void purge_tables(); bool flush_tables(THD *thd, flush_tables_type flag); -bool close_cached_connection_tables(THD *thd, LEX_CSTRING *connect_string); void close_all_tables_for_name(THD *thd, TABLE_SHARE *share, ha_extra_function extra, TABLE *skip_table); diff --git a/sql/sql_servers.cc b/sql/sql_servers.cc index fbf03dce21269..77b7c64422c36 100644 --- a/sql/sql_servers.cc +++ b/sql/sql_servers.cc @@ -124,6 +124,79 @@ static void init_servers_cache_psi_keys(void) } #endif /* HAVE_PSI_INTERFACE */ + +struct close_cached_connection_tables_arg +{ + THD *thd; + LEX_CSTRING *connection; + TABLE_LIST *tables; +}; + + +static my_bool close_cached_connection_tables_callback( + TDC_element *element, close_cached_connection_tables_arg *arg) +{ + TABLE_LIST *tmp; + + mysql_mutex_lock(&element->LOCK_table_share); + /* Ignore if table is not open or does not have a connect_string */ + if (!element->share || !element->share->connect_string.length || + !element->ref_count) + goto end; + + /* Compare the connection string */ + if (arg->connection && + (arg->connection->length > element->share->connect_string.length || + (arg->connection->length < element->share->connect_string.length && + (element->share->connect_string.str[arg->connection->length] != '/' && + element->share->connect_string.str[arg->connection->length] != '\\')) || + strncasecmp(arg->connection->str, element->share->connect_string.str, + arg->connection->length))) + goto end; + + /* close_cached_tables() only uses these elements */ + if (!(tmp= (TABLE_LIST*) alloc_root(arg->thd->mem_root, sizeof(TABLE_LIST))) || + !(arg->thd->make_lex_string(&tmp->db, element->share->db.str, element->share->db.length)) || + !(arg->thd->make_lex_string(&tmp->table_name, element->share->table_name.str, + element->share->table_name.length))) + { + mysql_mutex_unlock(&element->LOCK_table_share); + return TRUE; + } + + tmp->next_local= arg->tables; + arg->tables= tmp; + +end: + mysql_mutex_unlock(&element->LOCK_table_share); + return FALSE; +} + + +/** + Close all tables which match specified connection string or + if specified string is NULL, then any table with a connection string. + + @return false ok + @return true error, some tables may keep using old server info +*/ + +static bool close_cached_connection_tables(THD *thd, LEX_CSTRING *connection) +{ + close_cached_connection_tables_arg argument= { thd, connection, 0 }; + DBUG_ENTER("close_cached_connections"); + + if (tdc_iterate(thd, + (my_hash_walk_action) close_cached_connection_tables_callback, + &argument)) + DBUG_RETURN(true); + + DBUG_RETURN(argument.tables ? + close_cached_tables(thd, argument.tables, true, + thd->variables.lock_wait_timeout) : false); +} + + /* Initialize structures responsible for servers used in federated server scheme information for them from the server