Skip to content

Commit

Permalink
Fixed close_cached_connection_tables() flushing
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
Sergey Vojtovich committed Apr 3, 2020
1 parent 54c03cb commit bfdd30d
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 88 deletions.
87 changes: 0 additions & 87 deletions sql/sql_base.cc
View file
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion sql/sql_base.h
View file
Expand Up @@ -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);
Expand Down
73 changes: 73 additions & 0 deletions sql/sql_servers.cc
View file
Expand Up @@ -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
Expand Down

0 comments on commit bfdd30d

Please sign in to comment.