List:Commits« Previous MessageNext Message »
From:Sergey Vojtovich Date:May 5 2011 11:03am
Subject:bzr commit into mysql-5.1 branch (sergey.vojtovich:3682) Bug#11763712
View as plain text  
#At file:///home/svoj/mysql/server/mysql-5.1-bug56458/ based on revid:alexander.nozdrin@stripped

 3682 Sergey Vojtovich	2011-05-05
      BUG#11763712 - 56458: KILLING A FLUSH TABLE FOR A MERGE/CHILD
                            CRASHES SERVER
      
      Flushing objects of MERGE table, which is locked by flushing
      thread using LOCK TABLES, may cause deadlocks, assertion
      failures, invalid memory access.
     @ mysql-test/r/merge_debug_sync.result
        A test case for BUG#11763712.
     @ mysql-test/t/merge_debug_sync.test
        A test case for BUG#11763712
     @ sql/sql_base.cc
        When flushing tables under LOCK TABLES, all locked
        and flushed tables are released and then reopened.
        It may happen that we failed to reopen some tables,
        in this case we reopen as much tables as possible.
        
        If it was not possible to reopen MERGE child, MERGE
        parent is unusable and must be removed from thread
        open tables list.
        
        If it was not possible to reopen MERGE parent, all
        MERGE child table objects are unusable as well, at
        least because their locks are handled by MERGE parent.
        They must also be removed from thread open tables
        list.
        
        In other words if it was impossible to reopen any
        object of a MERGE table, all objects of this MERGE
        table must be considered unusable and closed.
        
        When flushing objects of MERGE table, which is locked
        by flushing thread, flush MERGE objects altogether.
        Since table locks for MERGE objects released altogether,
        another thread may acquire a lock for a MERGE child,
        which is not to be flushed.

    added:
      mysql-test/r/merge_debug_sync.result
      mysql-test/t/merge_debug_sync.test
    modified:
      sql/mysql_priv.h
      sql/sql_base.cc
=== added file 'mysql-test/r/merge_debug_sync.result'
--- a/mysql-test/r/merge_debug_sync.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/merge_debug_sync.result	2011-05-05 11:03:29 +0000
@@ -0,0 +1,30 @@
+#
+# BUG#11763712 - 56458: KILLING A FLUSH TABLE FOR A MERGE/CHILD
+#                       CRASHES SERVER
+#
+CREATE TABLE t1(a INT);
+CREATE TABLE t2(a INT);
+CREATE TABLE m1(a INT) ENGINE=MERGE UNION=(t1, t2);
+# Test reopen merge parent failure
+LOCK TABLES m1 READ;
+SET SESSION debug='d,simulate_reopen_merge_parent_error';
+FLUSH TABLES;
+ERROR HY000: Can't reopen table: 'm1'
+UNLOCK TABLES;
+# Test reopen merge child failure
+LOCK TABLES m1 READ;
+SET SESSION debug='d,simulate_reopen_merge_child_error';
+FLUSH TABLES;
+ERROR HY000: Can't reopen table: 't2'
+UNLOCK TABLES;
+SET SESSION debug=DEFAULT;
+# Test deadlock on LOCK_open
+LOCK TABLE m1 WRITE;
+SET DEBUG_SYNC='before_lock_tables_takes_lock SIGNAL opened WAIT_FOR flushed';
+LOCK TABLE t1 WRITE;
+SET DEBUG_SYNC='now WAIT_FOR opened';
+SET DEBUG_SYNC='after_flush_unlock SIGNAL flushed';
+FLUSH TABLE t2;
+UNLOCK TABLES;
+SET DEBUG_SYNC='RESET';
+DROP TABLES t1, t2, m1;

=== added file 'mysql-test/t/merge_debug_sync.test'
--- a/mysql-test/t/merge_debug_sync.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/merge_debug_sync.test	2011-05-05 11:03:29 +0000
@@ -0,0 +1,50 @@
+--source include/have_debug.inc
+--source include/have_debug_sync.inc
+
+--echo #
+--echo # BUG#11763712 - 56458: KILLING A FLUSH TABLE FOR A MERGE/CHILD
+--echo #                       CRASHES SERVER
+--echo #
+CREATE TABLE t1(a INT);
+CREATE TABLE t2(a INT);
+CREATE TABLE m1(a INT) ENGINE=MERGE UNION=(t1, t2);
+
+--echo # Test reopen merge parent failure
+LOCK TABLES m1 READ;
+SET SESSION debug='d,simulate_reopen_merge_parent_error';
+--error ER_CANT_REOPEN_TABLE
+FLUSH TABLES;
+UNLOCK TABLES;
+
+--echo # Test reopen merge child failure
+LOCK TABLES m1 READ;
+SET SESSION debug='d,simulate_reopen_merge_child_error';
+--error ER_CANT_REOPEN_TABLE
+FLUSH TABLES;
+UNLOCK TABLES;
+
+SET SESSION debug=DEFAULT;
+
+--echo # Test deadlock on LOCK_open
+let $ID= `select connection_id()`;
+LOCK TABLE m1 WRITE;
+connect(con1,localhost,root,,);
+
+connection con1;
+SET DEBUG_SYNC='before_lock_tables_takes_lock SIGNAL opened WAIT_FOR flushed';
+send LOCK TABLE t1 WRITE;
+
+connection default;
+SET DEBUG_SYNC='now WAIT_FOR opened';
+SET DEBUG_SYNC='after_flush_unlock SIGNAL flushed';
+FLUSH TABLE t2;
+UNLOCK TABLES;
+
+connection con1;
+reap;
+disconnect con1;
+
+connection default;
+SET DEBUG_SYNC='RESET';
+
+DROP TABLES t1, t2, m1;

=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h	2011-03-15 11:36:12 +0000
+++ b/sql/mysql_priv.h	2011-05-05 11:03:29 +0000
@@ -1606,6 +1606,7 @@ uint prep_alter_part_table(THD *thd, TAB
 #define RTFC_OWNED_BY_THD_FLAG      0x0001
 #define RTFC_WAIT_OTHER_THREAD_FLAG 0x0002
 #define RTFC_CHECK_KILLED_FLAG      0x0004
+#define RTFC_MERGE_EXPANSION_FLAG   0x0008
 bool remove_table_from_cache(THD *thd, const char *db, const char *table,
                              uint flags);
 

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2011-03-29 08:09:05 +0000
+++ b/sql/sql_base.cc	2011-05-05 11:03:29 +0000
@@ -930,7 +930,8 @@ bool close_cached_tables(THD *thd, TABLE
     for (TABLE_LIST *table= tables; table; table= table->next_local)
     {
       if (remove_table_from_cache(thd, table->db, table->table_name,
-                                  RTFC_OWNED_BY_THD_FLAG))
+                                  RTFC_OWNED_BY_THD_FLAG |
+                                  RTFC_MERGE_EXPANSION_FLAG))
 	found=1;
     }
     if (!found)
@@ -3068,6 +3069,10 @@ bool reopen_table(TABLE *table)
   table_list.table_name= table->s->table_name.str;
   table_list.table=      table;
 
+  DBUG_EXECUTE_IF("simulate_reopen_merge_parent_error",
+    if (table->child_l) DBUG_RETURN(1););
+  DBUG_EXECUTE_IF("simulate_reopen_merge_child_error",
+    if (table->parent) DBUG_RETURN(1););
   if (wait_for_locked_table_names(thd, &table_list))
     DBUG_RETURN(1);                             // Thread was killed
 
@@ -3335,20 +3340,38 @@ bool reopen_tables(THD *thd, bool get_lo
                           (long) table, (long) table->parent, db_stat));
     if (table->child_l && !db_stat)
       merge_table_found= TRUE;
-    if (!tables || (!db_stat && reopen_table(table)))
+    if (!tables || table->parent == (TABLE *) 1 ||
+        (!db_stat && reopen_table(table)))
     {
-      my_error(ER_CANT_REOPEN_TABLE, MYF(0), table->alias);
+      TABLE *parent= table->child_l ? table : table->parent;
       /*
         If we could not allocate 'tables', we may close open tables
-        here. If a MERGE table is affected, detach the children first.
-        It is not necessary to clear the child or parent table reference
-        of this table because the TABLE is freed. But we need to clear
-        the child or parent references of the other belonging tables so
-        that they cannot be moved into the unused_tables chain with
-        these pointers set.
-      */
-      if (table->child_l || table->parent)
-        detach_merge_children(table, TRUE);
+        here.
+
+        If a MERGE table is affected for the first time, mark all
+        relevant tables invalid. Otherwise handle it as usual.
+        All in all we must end up with:
+        - child tables are detached from parent. Child tables were
+          detached earlier, but child<->parent references kept
+          valid for reopen.
+        - parent is not in the to-be-locked tables.
+        - all child tables and parent are not in the THD::open_tables.
+        - all child tables and parent are not in the open_cache.
+      */
+      if (parent && table->parent != (TABLE *) 1)
+      {
+        TABLE_LIST *child_l;
+        parent->parent= (TABLE *) 1;
+        for (child_l= parent->child_l; ; child_l= child_l->next_global)
+        {
+          child_l->table->parent= (TABLE *) 1;
+          if (&child_l->next_global == parent->child_last_l)
+            break;
+        }
+      }
+      /* Reset parent reference, it may be used while freeing the table. */
+      table->parent= NULL;
+      my_error(ER_CANT_REOPEN_TABLE, MYF(0), table->alias);
       VOID(hash_delete(&open_cache,(uchar*) table));
       error=1;
     }
@@ -3369,6 +3392,57 @@ bool reopen_tables(THD *thd, bool get_lo
     }
   }
   *prev=0;
+
+  /*
+    It may happen that we opened a few objects of a MERGE table
+    before we detected an error. Cleanup these objects now.
+  */
+  if (merge_table_found && error)
+  {
+    prev= &thd->open_tables;
+    for (table= thd->open_tables; table; table= next)
+    {
+      next= table->next;
+      if (table->parent == (TABLE *) 1)
+      {
+        /*
+          MERGE parent must have been handled before and we must
+          never catch it here. It is expected because MERGE parent
+          always comes after it's children in THD::open_tables list.
+          If this assumption is wrong, we must remove parent from
+          the to-be-locked tables array.
+        */
+        DBUG_ASSERT(!table->child_l);
+#if 0
+        /* Remove merge parent from to-be-locked tables array. */
+        if (get_locks && table->child_l)
+        {
+          TABLE **t;
+          for (t= tables; t < tables_ptr; t++)
+          {
+            if (*t == table)
+            {
+              tables_ptr--;
+              memmove(t, t + 1, (tables_ptr - t) * sizeof(TABLE *));
+              break;
+            }
+          }
+        }
+#endif
+        /* Reset parent reference, it may be used while freeing the table. */
+        table->parent= NULL;
+        /* Free table. */
+        VOID(hash_delete(&open_cache, (uchar *) table));
+      }
+      else
+      {
+        *prev= table;
+        prev= &table->next;
+      }
+    }
+    *prev= 0;
+  }
+
   /*
     When all tables are open again, we can re-attach MERGE children to
     their parents. All TABLE objects are still present.
@@ -8524,6 +8598,7 @@ bool remove_table_from_cache(THD *thd, c
   char key[MAX_DBKEY_LENGTH];
   uint key_length;
   TABLE *table;
+  TABLE *parent= NULL;
   TABLE_SHARE *share;
   bool result= 0, signalled= 0;
   DBUG_ENTER("remove_table_from_cache");
@@ -8600,6 +8675,7 @@ bool remove_table_from_cache(THD *thd, c
       }
       else
       {
+        parent= table->child_l ? table : table->parent;
         DBUG_PRINT("info", ("Table was in use by current thread. db_stat: %u",
                             table->db_stat));
         result= result || (flags & RTFC_OWNED_BY_THD_FLAG);
@@ -8658,6 +8734,28 @@ bool remove_table_from_cache(THD *thd, c
     }
     break;
   }
+  if (flags & RTFC_MERGE_EXPANSION_FLAG)
+  {
+    /* Flush objects in a MERGE table altogether. */
+    if (parent)
+    {
+      TABLE_LIST *child_l;
+      if (parent->s->version)
+        result|= remove_table_from_cache(thd, parent->s->db.str,
+                                         parent->s->table_name.str,
+                                         flags & ~RTFC_MERGE_EXPANSION_FLAG);
+      for (child_l= parent->child_l; ; child_l= child_l->next_global)
+      {
+        TABLE *child= child_l->table;
+        if (child->s->version)
+          result|= remove_table_from_cache(thd, child->s->db.str,
+                                           child->s->table_name.str,
+                                           flags & ~RTFC_MERGE_EXPANSION_FLAG);
+        if (&child_l->next_global == parent->child_last_l)
+          break;
+      }
+    }
+  }
   DBUG_RETURN(result);
 }
 


Attachment: [text/bzr-bundle] bzr/sergey.vojtovich@oracle.com-20110505110329-2ncyowhr0enox1wo.bundle
Thread
bzr commit into mysql-5.1 branch (sergey.vojtovich:3682) Bug#11763712Sergey Vojtovich5 May