MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:September 9 2010 2:29pm
Subject:bzr commit into mysql-5.5-runtime branch (dlenev:3134) Bug#55273
View as plain text  
#At file:///home/dlenev/src/bzr/mysql-5.5-rt-55273/ based on revid:jon.hauglid@stripped

 3134 Dmitry Lenev	2010-09-09
      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).
     @ mysql-test/r/flush.result
        Added test case for bug #55273 "FLUSH TABLE tm WITH READ LOCK
        for Merge table causes assert failure".
     @ mysql-test/t/flush.test
        Added test case for bug #55273 "FLUSH TABLE tm WITH READ LOCK
        for Merge table causes assert failure".
     @ sql/sql_base.cc
        Changed lock_table_names() to support newly introduced
        MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK flag.
     @ sql/sql_base.h
        Introduced MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK flag for
        open_tables() and lock_table_names() which allows to skip
        acquiring of global and schema-scope locks when SNW, SNRW or
        X metadata locks are acquired.
     @ sql/sql_reload.cc
        Changed "FLUSH TABLES table_list WITH READ LOCK" code not to
        cause assert about missing metadata locks when MERGE table is
        flushed without one of its underlying tables.
        To achieve this we no longer call open_and_lock_tables() with
        MYSQL_OPEN_HAS_MDL_LOCK flag so this function automatically
        acquires metadata locks on MERGE children if such lock has
        not been already acquired at earlier stage. Instead we call
        this function with MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK flag to
        suppress acquiring of global IX lock in order to keep FLUSH
        TABLES  table_list WITH READ LOCK compatible with FLUSH TABLE
        WITH READ LOCK.
        Also changed implementation to use lock_table_names() function
        for pre-acquiring of metadata locks instead of custom code.
        To implement this change moved setting of open_type member for
        table list elements to parser.
     @ sql/sql_yacc.yy
        Now we set acceptable type of table for FLUSH TABLES table_list
        WITH READ LOCK at parsing time instead of execution time.

    modified:
      mysql-test/r/flush.result
      mysql-test/t/flush.test
      sql/sql_base.cc
      sql/sql_base.h
      sql/sql_reload.cc
      sql/sql_yacc.yy
=== modified file 'mysql-test/r/flush.result'
--- a/mysql-test/r/flush.result	2010-08-13 14:14:36 +0000
+++ b/mysql-test/r/flush.result	2010-09-09 14:29:14 +0000
@@ -373,3 +373,53 @@ commit;
 # --> connection con2
 # --> connection default
 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;

=== modified file 'mysql-test/t/flush.test'
--- a/mysql-test/t/flush.test	2010-08-13 14:14:36 +0000
+++ b/mysql-test/t/flush.test	2010-09-09 14:29:14 +0000
@@ -546,3 +546,34 @@ disconnect con2;
 connection default;
 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;

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2010-08-31 10:03:36 +0000
+++ b/sql/sql_base.cc	2010-09-09 14:29:14 +0000
@@ -4522,13 +4522,15 @@ lock_table_names(THD *thd,
            ! (flags & MYSQL_OPEN_SKIP_TEMPORARY) &&
            find_temporary_table(thd, table))))
     {
-      if (schema_set.insert(table))
+      if (! (flags & MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK) &&
+          schema_set.insert(table))
         return TRUE;
       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

=== modified file 'sql/sql_base.h'
--- a/sql/sql_base.h	2010-08-31 10:03:36 +0000
+++ b/sql/sql_base.h	2010-09-09 14:29:14 +0000
@@ -122,6 +122,11 @@ TABLE *open_ltable(THD *thd, TABLE_LIST 
   (LONG_TIMEOUT = 1 year) rather than the user-supplied timeout value.
 */
 #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. */
 #define MYSQL_OPEN_REOPEN  (MYSQL_OPEN_IGNORE_FLUSH |\

=== modified file 'sql/sql_reload.cc'
--- a/sql/sql_reload.cc	2010-08-13 09:51:48 +0000
+++ b/sql/sql_reload.cc	2010-09-09 14:29:14 +0000
@@ -328,7 +328,6 @@ bool reload_acl_and_cache(THD *thd, unsi
   -------------------------------------
   - you can't flush WITH READ LOCK a non-existent table
   - you can't flush WITH READ LOCK under LOCK TABLES
-  - currently incompatible with the GRL (@todo: fix)
 
   Effect on views and temporary tables.
   ------------------------------------
@@ -338,6 +337,13 @@ bool reload_acl_and_cache(THD *thd, unsi
   if there is a base table, it's used, otherwise ER_NO_SUCH_TABLE
   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
   ---------------
   This statement causes an implicit commit before and
@@ -354,7 +360,6 @@ bool flush_tables_with_read_lock(THD *th
 {
   Lock_tables_prelocking_strategy lock_tables_prelocking_strategy;
   TABLE_LIST *table_list;
-  MDL_request_list mdl_requests;
 
   /*
     This is called from SQLCOM_FLUSH, the transaction has
@@ -368,17 +373,13 @@ bool flush_tables_with_read_lock(THD *th
   }
 
   /*
-    Acquire SNW locks on tables to be flushed. We can't use
-    lock_table_names() here as this call will also acquire global IX
-    and database-scope IX locks on the tables, and this will make
+    Acquire SNW locks on tables to be flushed. Don't acquire global
+    IX and database-scope IX locks on the tables as this will make
     this statement incompatible with FLUSH TABLES WITH READ LOCK.
   */
-  for (table_list= all_tables; table_list;
-       table_list= table_list->next_global)
-    mdl_requests.push_front(&table_list->mdl_request);
-
-  if (thd->mdl_context.acquire_locks(&mdl_requests,
-                                     thd->variables.lock_wait_timeout))
+  if (lock_table_names(thd, all_tables, NULL,
+                       thd->variables.lock_wait_timeout,
+                       MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK))
     goto error;
 
   DEBUG_SYNC(thd,"flush_tables_with_read_lock_after_acquire_locks");
@@ -390,21 +391,24 @@ bool flush_tables_with_read_lock(THD *th
     tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED,
                      table_list->db,
                      table_list->table_name, FALSE);
-
-    /* Skip views and temporary tables. */
-    table_list->required_type= FRMTYPE_TABLE; /* Don't try to flush views. */
-    table_list->open_type= OT_BASE_ONLY;      /* Ignore temporary tables. */
+    /* Reset ticket to satisfy asserts in open_tables(). */
+    table_list->mdl_request.ticket= NULL;
   }
 
   /*
     Before opening and locking tables the below call also waits
     for old shares to go away, so the fact that we don't pass
     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,
-                            MYSQL_OPEN_HAS_MDL_LOCK,
-                            &lock_tables_prelocking_strategy) ||
-       thd->locked_tables_list.init_locked_tables(thd))
+  if (open_and_lock_tables(thd, all_tables, FALSE,
+                           MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK,
+                           &lock_tables_prelocking_strategy) ||
+      thd->locked_tables_list.init_locked_tables(thd))
   {
     goto error;
   }

=== modified file 'sql/sql_yacc.yy'
--- a/sql/sql_yacc.yy	2010-09-01 13:12:42 +0000
+++ b/sql/sql_yacc.yy	2010-09-09 14:29:14 +0000
@@ -11278,7 +11278,11 @@ opt_with_read_lock:
             TABLE_LIST *tables= Lex->query_tables;
             Lex->type|= REFRESH_READ_LOCK;
             for (; tables; tables= tables->next_global)
+            {
               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. */
+            }
           }
         ;
 


Attachment: [text/bzr-bundle] bzr/dlenev@mysql.com-20100909142914-yuh7yxxw0gt1zkjd.bundle
Thread
bzr commit into mysql-5.5-runtime branch (dlenev:3134) Bug#55273Dmitry Lenev9 Sep