maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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