Hello, Jon Olav!
Here are my comments about your patch:
* Jon Olav Hauglid <jon.hauglid@stripped> [11/03/04 19:03]:
> #At file:///export/home/x/mysql-5.5-bug57649/ based on
> revid:jorgen.loland@stripped
>
> 3372 Jon Olav Hauglid 2011-03-04
> Bug #11764779 (former 57649)
> FLUSH TABLES under FLUSH TABLES <list> WITH READ LOCK leads
> to assert failure.
>
> This assert was triggered if a statement tried up upgrade a metadata
> lock with an active FLUSH TABLE <list> WITH READ LOCK. The assert
> checks that the connection already holds a global intention exclusive
> metadata lock. However, FLUSH TABLE <list> WITH READ LOCK does not
> acquire this lock in order to be compatible with FLUSH TABLES WITH
> READ LOCK. Therefore any metadata lock upgrade caused the assert to
> be triggered.
>
> This patch fixes the problem by preventing metadata lock upgrade
> if the connection has an active FLUSH TABLE <list> WITH READ LOCK.
> ER_TABLE_NOT_LOCKED_FOR_WRITE will instead be reported to the client.
...
> === modified file 'sql/sql_base.cc'
> --- a/sql/sql_base.cc 2011-02-08 15:47:33 +0000
> +++ b/sql/sql_base.cc 2011-03-04 15:54:29 +0000
> @@ -3120,7 +3120,7 @@ TABLE *find_locked_table(TABLE *list, co
> lock from the list of open tables, emit error if no such table
> found.
>
> - @param list List of TABLE objects to be searched
> + @param thd Thread context
> @param db Database name.
> @param table_name Name of table.
> @param no_error Don't emit error if no suitable TABLE
IMO it makes sense add a note to the description of this function
mentioning that it also checks for the presence of global IX lock.
...
> @@ -3143,19 +3142,28 @@ TABLE *find_table_for_mdl_upgrade(TABLE
> my_error(ER_TABLE_NOT_LOCKED, MYF(0), table_name);
> return NULL;
> }
> - else
> +
> + /*
> + It is not safe to upgrade the metadata lock without GLOBAL IX lock.
> + This can happen with FLUSH TABLES <list> WITH READ LOCK as we in these
> + cases don't take a GLOBAL IX lock in order to be compatible with
> + global read lock.
> + */
> + if (!thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, "", "",
> + MDL_INTENTION_EXCLUSIVE))
> {
> - while (tab->mdl_ticket != NULL &&
> - !tab->mdl_ticket->is_upgradable_or_exclusive() &&
> - (tab= find_locked_table(tab->next, db, table_name)))
> - continue;
> - if (!tab)
> - {
> - if (!no_error)
> - my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name);
> - return 0;
> - }
> + my_error(ER_TABLE_NOT_LOCKED_FOR_WRITE, MYF(0), table_name);
> + return NULL;
> }
I think in this branch we should also respect value of no_error argument
and skip emitting error if it is set to true.
...
Otherwise I am OK with your patch and think that it can be pushed
after addressing/discussing the above issues.
Thank you for fixing this!!!
--
Dmitry Lenev, Software Developer
Oracle Development SPB/MySQL, www.mysql.com
Are you MySQL certified? http://www.mysql.com/certification
