Commit 3326614d authored by Dmitry Lenev's avatar Dmitry Lenev

Fix for bug #55273 "FLUSH TABLE tm WITH READ LOCK for Merge

table causes assert failure".

Attempting to use FLUSH TABLE table_list WITH READ LOCK
statement for a MERGE table led to an assertion failure if
one of its children was not present in the list of tables
to be flushed. The problem was not visible in non-debug builds.

The assertion failure was caused by the fact that in such
situations FLUSH TABLES table_list WITH READ LOCK implementation
tried to use (e.g. lock) such child tables without acquiring
metadata lock on them. This happened because when opening tables
we assumed metadata locks on all tables were already acquired
earlier during statement execution and a such assumption was
false for MERGE children.

This patch fixes the problem by ensuring at open_tables() time
that we try to acquire metadata locks on all tables to be opened. 
For normal tables such requests are satisfied instantly since
locks are already acquired for them. For MERGE children metadata
locks are acquired in normal fashion.

Note that FLUSH TABLES merge_table WITH READ LOCK will lock for
read both the MERGE table and its children but will flush only 
the MERGE table. To flush children one has to mention them in table
list explicitly. This is expected behavior and it is consistent with
usage patterns for this statement (e.g. in mysqlhotcopy script).
parent 51a3375c
...@@ -373,3 +373,53 @@ commit; ...@@ -373,3 +373,53 @@ commit;
# --> connection con2 # --> connection con2
# --> connection default # --> connection default
drop table t1; drop table t1;
#
# Test for bug #55273 "FLUSH TABLE tm WITH READ LOCK for Merge table
# causes assert failure".
#
drop table if exists t1, t2, tm;
create table t1 (i int);
create table t2 (i int);
create table tm (i int) engine=merge union=(t1, t2);
insert into t1 values (1), (2);
insert into t2 values (3), (4);
# The below statement should succeed and lock merge
# table for read. Only merge table gets flushed and
# not underlying tables.
flush tables tm with read lock;
select * from tm;
i
1
2
3
4
# Check that underlying tables are locked.
select * from t1;
i
1
2
select * from t2;
i
3
4
unlock tables;
# This statement should succeed as well and flush
# all tables in the list.
flush tables tm, t1, t2 with read lock;
select * from tm;
i
1
2
3
4
# Naturally, underlying tables should be locked in this case too.
select * from t1;
i
1
2
select * from t2;
i
3
4
unlock tables;
drop tables tm, t1, t2;
...@@ -546,3 +546,34 @@ disconnect con2; ...@@ -546,3 +546,34 @@ disconnect con2;
connection default; connection default;
drop table t1; drop table t1;
--echo #
--echo # Test for bug #55273 "FLUSH TABLE tm WITH READ LOCK for Merge table
--echo # causes assert failure".
--echo #
--disable_warnings
drop table if exists t1, t2, tm;
--enable_warnings
create table t1 (i int);
create table t2 (i int);
create table tm (i int) engine=merge union=(t1, t2);
insert into t1 values (1), (2);
insert into t2 values (3), (4);
--echo # The below statement should succeed and lock merge
--echo # table for read. Only merge table gets flushed and
--echo # not underlying tables.
flush tables tm with read lock;
select * from tm;
--echo # Check that underlying tables are locked.
select * from t1;
select * from t2;
unlock tables;
--echo # This statement should succeed as well and flush
--echo # all tables in the list.
flush tables tm, t1, t2 with read lock;
select * from tm;
--echo # Naturally, underlying tables should be locked in this case too.
select * from t1;
select * from t2;
unlock tables;
drop tables tm, t1, t2;
...@@ -4522,13 +4522,15 @@ lock_table_names(THD *thd, ...@@ -4522,13 +4522,15 @@ lock_table_names(THD *thd,
! (flags & MYSQL_OPEN_SKIP_TEMPORARY) && ! (flags & MYSQL_OPEN_SKIP_TEMPORARY) &&
find_temporary_table(thd, table)))) find_temporary_table(thd, table))))
{ {
if (schema_set.insert(table)) if (! (flags & MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK) &&
schema_set.insert(table))
return TRUE; return TRUE;
mdl_requests.push_front(&table->mdl_request); mdl_requests.push_front(&table->mdl_request);
} }
} }
if (! mdl_requests.is_empty()) if (! (flags & MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK) &&
! mdl_requests.is_empty())
{ {
/* /*
Scoped locks: Take intention exclusive locks on all involved Scoped locks: Take intention exclusive locks on all involved
......
...@@ -122,6 +122,11 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update, ...@@ -122,6 +122,11 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update,
(LONG_TIMEOUT = 1 year) rather than the user-supplied timeout value. (LONG_TIMEOUT = 1 year) rather than the user-supplied timeout value.
*/ */
#define MYSQL_LOCK_IGNORE_TIMEOUT 0x0800 #define MYSQL_LOCK_IGNORE_TIMEOUT 0x0800
/**
When acquiring "strong" (SNW, SNRW, X) metadata locks on tables to
be open do not acquire global and schema-scope IX locks.
*/
#define MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK 0x1000
/** Please refer to the internals manual. */ /** Please refer to the internals manual. */
#define MYSQL_OPEN_REOPEN (MYSQL_OPEN_IGNORE_FLUSH |\ #define MYSQL_OPEN_REOPEN (MYSQL_OPEN_IGNORE_FLUSH |\
......
...@@ -328,7 +328,6 @@ bool reload_acl_and_cache(THD *thd, unsigned long options, ...@@ -328,7 +328,6 @@ bool reload_acl_and_cache(THD *thd, unsigned long options,
------------------------------------- -------------------------------------
- you can't flush WITH READ LOCK a non-existent table - you can't flush WITH READ LOCK a non-existent table
- you can't flush WITH READ LOCK under LOCK TABLES - you can't flush WITH READ LOCK under LOCK TABLES
- currently incompatible with the GRL (@todo: fix)
Effect on views and temporary tables. Effect on views and temporary tables.
------------------------------------ ------------------------------------
...@@ -338,6 +337,13 @@ bool reload_acl_and_cache(THD *thd, unsigned long options, ...@@ -338,6 +337,13 @@ bool reload_acl_and_cache(THD *thd, unsigned long options,
if there is a base table, it's used, otherwise ER_NO_SUCH_TABLE if there is a base table, it's used, otherwise ER_NO_SUCH_TABLE
is returned. is returned.
Handling of MERGE tables
------------------------
For MERGE table this statement will open and lock child tables
for read (it is impossible to lock parent table without it).
Child tables won't be flushed unless they are explicitly present
in the statement's table list.
Implicit commit Implicit commit
--------------- ---------------
This statement causes an implicit commit before and This statement causes an implicit commit before and
...@@ -354,7 +360,6 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) ...@@ -354,7 +360,6 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables)
{ {
Lock_tables_prelocking_strategy lock_tables_prelocking_strategy; Lock_tables_prelocking_strategy lock_tables_prelocking_strategy;
TABLE_LIST *table_list; TABLE_LIST *table_list;
MDL_request_list mdl_requests;
/* /*
This is called from SQLCOM_FLUSH, the transaction has This is called from SQLCOM_FLUSH, the transaction has
...@@ -368,17 +373,13 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) ...@@ -368,17 +373,13 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables)
} }
/* /*
Acquire SNW locks on tables to be flushed. We can't use Acquire SNW locks on tables to be flushed. Don't acquire global
lock_table_names() here as this call will also acquire global IX IX and database-scope IX locks on the tables as this will make
and database-scope IX locks on the tables, and this will make
this statement incompatible with FLUSH TABLES WITH READ LOCK. this statement incompatible with FLUSH TABLES WITH READ LOCK.
*/ */
for (table_list= all_tables; table_list; if (lock_table_names(thd, all_tables, NULL,
table_list= table_list->next_global) thd->variables.lock_wait_timeout,
mdl_requests.push_front(&table_list->mdl_request); MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK))
if (thd->mdl_context.acquire_locks(&mdl_requests,
thd->variables.lock_wait_timeout))
goto error; goto error;
DEBUG_SYNC(thd,"flush_tables_with_read_lock_after_acquire_locks"); DEBUG_SYNC(thd,"flush_tables_with_read_lock_after_acquire_locks");
...@@ -390,21 +391,24 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables) ...@@ -390,21 +391,24 @@ bool flush_tables_with_read_lock(THD *thd, TABLE_LIST *all_tables)
tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED, tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED,
table_list->db, table_list->db,
table_list->table_name, FALSE); table_list->table_name, FALSE);
/* Reset ticket to satisfy asserts in open_tables(). */
/* Skip views and temporary tables. */ table_list->mdl_request.ticket= NULL;
table_list->required_type= FRMTYPE_TABLE; /* Don't try to flush views. */
table_list->open_type= OT_BASE_ONLY; /* Ignore temporary tables. */
} }
/* /*
Before opening and locking tables the below call also waits Before opening and locking tables the below call also waits
for old shares to go away, so the fact that we don't pass for old shares to go away, so the fact that we don't pass
MYSQL_LOCK_IGNORE_FLUSH flag to it is important. MYSQL_LOCK_IGNORE_FLUSH flag to it is important.
Also we don't pass MYSQL_OPEN_HAS_MDL_LOCK flag as we want
to open underlying tables if merge table is flushed.
For underlying tables of the merge the below call has to
acquire SNW locks to ensure that they can be locked for
read without further waiting.
*/ */
if (open_and_lock_tables(thd, all_tables, FALSE, if (open_and_lock_tables(thd, all_tables, FALSE,
MYSQL_OPEN_HAS_MDL_LOCK, MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK,
&lock_tables_prelocking_strategy) || &lock_tables_prelocking_strategy) ||
thd->locked_tables_list.init_locked_tables(thd)) thd->locked_tables_list.init_locked_tables(thd))
{ {
goto error; goto error;
} }
......
...@@ -11278,7 +11278,11 @@ opt_with_read_lock: ...@@ -11278,7 +11278,11 @@ opt_with_read_lock:
TABLE_LIST *tables= Lex->query_tables; TABLE_LIST *tables= Lex->query_tables;
Lex->type|= REFRESH_READ_LOCK; Lex->type|= REFRESH_READ_LOCK;
for (; tables; tables= tables->next_global) for (; tables; tables= tables->next_global)
{
tables->mdl_request.set_type(MDL_SHARED_NO_WRITE); tables->mdl_request.set_type(MDL_SHARED_NO_WRITE);
tables->required_type= FRMTYPE_TABLE; /* Don't try to flush views. */
tables->open_type= OT_BASE_ONLY; /* Ignore temporary tables. */
}
} }
; ;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment