maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #12150
Re: eeba5b2b158: Fixed close_cached_connection_tables() flushing
Hi, Sergey!
On Apr 02, Sergey Vojtovich wrote:
> revision-id: eeba5b2b158 (mariadb-10.5.0-72-geeba5b2b158)
> parent(s): a737b71295e
> author: Sergey Vojtovich <svoj@xxxxxxxxxxx>
> committer: Sergey Vojtovich <svoj@xxxxxxxxxxx>
> timestamp: 2019-12-25 20:24:26 +0400
> message:
>
> 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
>
> diff --git a/sql/sql_base.cc b/sql/sql_base.cc
> index 9494c0b7bd2..5c6d366ce6f 100644
> --- a/sql/sql_base.cc
> +++ b/sql/sql_base.cc
> @@ -689,33 +689,21 @@ static my_bool close_cached_connection_tables_callback(
> Close cached connections
Could you, perhaps, move this function to sql_server.cc ?
Also it could be made static there.
And please clarify the comment, "Close cached connections' was not
helpful at all. May be "Close all tables with a given CONNECTION= value"
> @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
> + @return true error, some tables may keep using old server info
> */
>
> bool close_cached_connection_tables(THD *thd, LEX_CSTRING *connection)
> {
> - bool res= false;
> - close_cached_connection_tables_arg argument;
> + close_cached_connection_tables_arg argument= { thd, connection, 0 };
> 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);
> + DBUG_RETURN(close_cached_tables(thd, argument.tables, true,
> + thd->variables.lock_wait_timeout));
are you sure you want to do it when argument.tables is empty? It'll do
purge_tables() which seems to be a bit extreme for DROP SERVER
> }
>
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx
Follow ups