List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:November 15 2010 6:13pm
Subject:bzr commit into mysql-5.5-runtime branch (jon.hauglid:3185) Bug#57663
View as plain text  
#At file:///export/home/x/mysql-5.5-runtime-prereq/ based on revid:jon.hauglid@stripped

 3185 Jon Olav Hauglid	2010-11-15
      Bug #57663 Concurrent statement using stored function and DROP DATABASE
                 breaks SBR
      
      This pre-requisite patch refactors the code for dropping tables, used
      by DROP TABLE and DROP DATABASE. The patch moves the code for acquiring
      metadata locks out of mysql_rm_table_part2() and makes it the
      responsibility of the caller. This in preparation of changing the
      DROP DATABASE implementation to acquire all metadata locks before any
      changes are made. mysql_rm_table_part2() is renamed
      mysql_rm_table_no_locks() to reflect the change.

    modified:
      mysql-test/r/partition_debug_sync.result
      mysql-test/t/partition_debug_sync.test
      sql/sql_db.cc
      sql/sql_table.cc
      sql/sql_table.h
=== modified file 'mysql-test/r/partition_debug_sync.result'
--- a/mysql-test/r/partition_debug_sync.result	2010-07-01 13:53:46 +0000
+++ b/mysql-test/r/partition_debug_sync.result	2010-11-15 18:13:30 +0000
@@ -25,7 +25,7 @@ ALTER TABLE t1 REMOVE PARTITIONING;
 # Con default
 SET DEBUG_SYNC= 'now WAIT_FOR removing_partitioning';
 SET DEBUG_SYNC= 'mdl_acquire_lock_wait SIGNAL waiting_for_alter';
-SET DEBUG_SYNC= 'rm_table_part2_before_delete_table WAIT_FOR partitioning_removed';
+SET DEBUG_SYNC= 'rm_table_no_locks_before_delete_table WAIT_FOR partitioning_removed';
 DROP TABLE IF EXISTS t1;
 # Con 1
 SET SESSION debug= "-d,sleep_before_create_table_no_lock";
@@ -51,12 +51,12 @@ SET DEBUG_SYNC= 'alter_table_before_open
 SET DEBUG_SYNC= 'alter_table_before_rename_result_table WAIT_FOR delete_done';
 ALTER TABLE t2 REMOVE PARTITIONING;
 # Con default
-SET SESSION debug= "+d,sleep_before_part2_delete_table";
+SET SESSION debug= "+d,sleep_before_no_locks_delete_table";
 SET DEBUG_SYNC= 'now WAIT_FOR removing_partitions';
-SET DEBUG_SYNC= 'rm_table_part2_before_delete_table SIGNAL waiting_for_alter';
-SET DEBUG_SYNC= 'rm_table_part2_before_binlog SIGNAL delete_done';
+SET DEBUG_SYNC= 'rm_table_no_locks_before_delete_table SIGNAL waiting_for_alter';
+SET DEBUG_SYNC= 'rm_table_no_locks_before_binlog SIGNAL delete_done';
 DROP TABLE IF EXISTS t2;
-SET SESSION debug= "-d,sleep_before_part2_delete_table";
+SET SESSION debug= "-d,sleep_before_no_locks_delete_table";
 # Con 1
 ERROR 42S02: Table 'test.t2' doesn't exist
 SET DEBUG_SYNC= 'RESET';

=== modified file 'mysql-test/t/partition_debug_sync.test'
--- a/mysql-test/t/partition_debug_sync.test	2010-07-01 13:53:46 +0000
+++ b/mysql-test/t/partition_debug_sync.test	2010-11-15 18:13:30 +0000
@@ -38,7 +38,7 @@ connection default;
 --echo # Con default
 SET DEBUG_SYNC= 'now WAIT_FOR removing_partitioning';
 SET DEBUG_SYNC= 'mdl_acquire_lock_wait SIGNAL waiting_for_alter';
-SET DEBUG_SYNC= 'rm_table_part2_before_delete_table WAIT_FOR partitioning_removed';
+SET DEBUG_SYNC= 'rm_table_no_locks_before_delete_table WAIT_FOR partitioning_removed';
 DROP TABLE IF EXISTS t1;
 --echo # Con 1
 connection con1;
@@ -70,12 +70,12 @@ SET DEBUG_SYNC= 'alter_table_before_rena
 --send ALTER TABLE t2 REMOVE PARTITIONING
 connection default;
 --echo # Con default
-SET SESSION debug= "+d,sleep_before_part2_delete_table";
+SET SESSION debug= "+d,sleep_before_no_locks_delete_table";
 SET DEBUG_SYNC= 'now WAIT_FOR removing_partitions';
-SET DEBUG_SYNC= 'rm_table_part2_before_delete_table SIGNAL waiting_for_alter';
-SET DEBUG_SYNC= 'rm_table_part2_before_binlog SIGNAL delete_done';
+SET DEBUG_SYNC= 'rm_table_no_locks_before_delete_table SIGNAL waiting_for_alter';
+SET DEBUG_SYNC= 'rm_table_no_locks_before_binlog SIGNAL delete_done';
 DROP TABLE IF EXISTS t2;
-SET SESSION debug= "-d,sleep_before_part2_delete_table";
+SET SESSION debug= "-d,sleep_before_no_locks_delete_table";
 --echo # Con 1
 connection con1;
 --error ER_NO_SUCH_TABLE

=== modified file 'sql/sql_db.cc'
--- a/sql/sql_db.cc	2010-11-15 11:32:49 +0000
+++ b/sql/sql_db.cc	2010-11-15 18:13:30 +0000
@@ -28,6 +28,8 @@
 #include "sql_acl.h"                     // SELECT_ACL, DB_ACLS,
                                          // acl_get, check_grant_db
 #include "log_event.h"                   // Query_log_event
+#include "sql_base.h"                    // lock_table_names, tdc_remove_table
+#include "sql_handler.h"                 // mysql_ha_rm_tables
 #include <mysys_err.h>
 #include "sp.h"
 #include "events.h"
@@ -944,6 +946,7 @@ static long mysql_rm_known_files(THD *th
   ulong found_other_files=0;
   char filePath[FN_REFLEN];
   TABLE_LIST *tot_list=0, **tot_list_next_local, **tot_list_next_global;
+  TABLE_LIST *table;
   DBUG_ENTER("mysql_rm_known_files");
   DBUG_PRINT("enter",("path: %s", org_path));
 
@@ -1040,8 +1043,39 @@ static long mysql_rm_known_files(THD *th
       }
     }
   }
+
+  /*
+    Disable drop of enabled log tables, must be done before name locking.
+    This check is only needed if we are dropping the "mysql" database.
+  */
+  if ((my_strcasecmp(system_charset_info, MYSQL_SCHEMA_NAME.str, db) == 0))
+  {
+    for (table= tot_list; table; table= table->next_local)
+    {
+      if (check_if_log_table(table->db_length, table->db,
+                             table->table_name_length, table->table_name, true))
+      {
+        my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP");
+        goto err;
+      }
+    }
+  }
+
+  /* mysql_ha_rm_tables() requires a non-null TABLE_LIST. */
+  if (tot_list)
+    mysql_ha_rm_tables(thd, tot_list);
+
+  if (lock_table_names(thd, tot_list, NULL, thd->variables.lock_wait_timeout,
+                       MYSQL_OPEN_SKIP_TEMPORARY))
+    goto err;
+
+  for (table= tot_list; table; table= table->next_local)
+    tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name,
+                     false);
+
   if (thd->killed ||
-      (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1)))
+      (tot_list && mysql_rm_table_no_locks(thd, tot_list, true,
+                                           false, true, true)))
     goto err;
 
   my_dirend(dirp);  

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2010-11-12 11:23:17 +0000
+++ b/sql/sql_table.cc	2010-11-15 18:13:30 +0000
@@ -1849,12 +1849,77 @@ bool mysql_rm_table(THD *thd,TABLE_LIST
 {
   bool error;
   Drop_table_error_handler err_handler;
+  TABLE_LIST *table;
 
   DBUG_ENTER("mysql_rm_table");
 
+  /* Disable drop of enabled log tables, must be done before name locking */
+  for (table= tables; table; table= table->next_local)
+  {
+    if (check_if_log_table(table->db_length, table->db,
+                           table->table_name_length, table->table_name, true))
+    {
+      my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP");
+      DBUG_RETURN(true);
+    }
+  }
+
+  mysql_ha_rm_tables(thd, tables);
+
+  if (!drop_temporary)
+  {
+    if (!thd->locked_tables_mode)
+    {
+      if (lock_table_names(thd, tables, NULL, thd->variables.lock_wait_timeout,
+                           MYSQL_OPEN_SKIP_TEMPORARY))
+        DBUG_RETURN(true);
+      for (table= tables; table; table= table->next_local)
+        tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name,
+                         false);
+    }
+    else
+    {
+      for (table= tables; table; table= table->next_local)
+        if (table->open_type != OT_BASE_ONLY &&
+	    find_temporary_table(thd, table))
+        {
+          /*
+            A temporary table.
+
+            Don't try to find a corresponding MDL lock or assign it
+            to table->mdl_request.ticket. There can't be metadata
+            locks for temporary tables: they are local to the session.
+
+            Later in this function we release the MDL lock only if
+            table->mdl_requeset.ticket is not NULL. Thus here we
+            ensure that we won't release the metadata lock on the base
+            table locked with LOCK TABLES as a side effect of temporary
+            table drop.
+          */
+          DBUG_ASSERT(table->mdl_request.ticket == NULL);
+        }
+        else
+        {
+          /*
+            Not a temporary table.
+
+            Since 'tables' list can't contain duplicates (this is ensured
+            by parser) it is safe to cache pointer to the TABLE instances
+            in its elements.
+          */
+          table->table= find_table_for_mdl_upgrade(thd->open_tables, table->db,
+                                                   table->table_name, false);
+          if (!table->table)
+            DBUG_RETURN(true);
+          table->mdl_request.ticket= table->table->mdl_ticket;
+        }
+    }
+  }
+
   /* mark for close and remove all cached entries */
   thd->push_internal_handler(&err_handler);
-  error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0);
+  error= mysql_rm_table_no_locks(thd, tables, if_exists, drop_temporary,
+                                 false, false);
   thd->pop_internal_handler();
 
   if (error)
@@ -1863,39 +1928,37 @@ bool mysql_rm_table(THD *thd,TABLE_LIST
   DBUG_RETURN(FALSE);
 }
 
-/*
-  Execute the drop of a normal or temporary table
 
-  SYNOPSIS
-    mysql_rm_table_part2()
-    thd			Thread handler
-    tables		Tables to drop
-    if_exists		If set, don't give an error if table doesn't exists.
-			In this case we give an warning of level 'NOTE'
-    drop_temporary	Only drop temporary tables
-    drop_view		Allow to delete VIEW .frm
-    dont_log_query	Don't write query to log files. This will also not
-			generate warnings if the handler files doesn't exists  
-
-  TODO:
-    When logging to the binary log, we should log
-    tmp_tables and transactional tables as separate statements if we
-    are in a transaction;  This is needed to get these tables into the
-    cached binary log that is only written on COMMIT.
-
-   The current code only writes DROP statements that only uses temporary
-   tables to the cache binary log.  This should be ok on most cases, but
-   not all.
-
- RETURN
-   0	ok
-   1	Error
-   -1	Thread was killed
+/**
+  Execute the drop of a normal or temporary table.
+
+  @param  thd             Thread handler
+  @param  tables          Tables to drop
+  @param  if_exists       If set, don't give an error if table doesn't exists.
+                          In this case we give an warning of level 'NOTE'
+  @param  drop_temporary  Only drop temporary tables
+  @param  drop_view       Allow to delete VIEW .frm
+  @param  dont_log_query  Don't write query to log files. This will also not
+                          generate warnings if the handler files doesn't exists
+
+  @retval  0  ok
+  @retval  1  Error
+  @retval -1  Thread was killed
+
+  @note This function assumes that metadata locks have already been taken.
+
+  @todo When logging to the binary log, we should log
+        tmp_tables and transactional tables as separate statements if we
+        are in a transaction;  This is needed to get these tables into the
+        cached binary log that is only written on COMMIT.
+        The current code only writes DROP statements that only uses temporary
+        tables to the cache binary log.  This should be ok on most cases, but
+        not all.
 */
 
-int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
-			 bool drop_temporary, bool drop_view,
-			 bool dont_log_query)
+int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
+                            bool drop_temporary, bool drop_view,
+                            bool dont_log_query)
 {
   TABLE_LIST *table;
   char path[FN_REFLEN + 1], *alias= NULL;
@@ -1909,7 +1972,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
   bool non_tmp_table_deleted= 0;
   String built_query;
   String built_trans_tmp_query, built_non_trans_tmp_query;
-  DBUG_ENTER("mysql_rm_table_part2");
+  DBUG_ENTER("mysql_rm_table_no_locks");
 
   /*
     Prepares the drop statements that will be written into the binary
@@ -1963,71 +2026,6 @@ int mysql_rm_table_part2(THD *thd, TABLE
     }
   }
 
-  mysql_ha_rm_tables(thd, tables);
-
-  /* Disable drop of enabled log tables, must be done before name locking */
-  for (table= tables; table; table= table->next_local)
-  {
-    if (check_if_log_table(table->db_length, table->db,
-                           table->table_name_length, table->table_name, 1))
-    {
-      my_error(ER_BAD_LOG_STATEMENT, MYF(0), "DROP");
-      DBUG_RETURN(1);
-    }
-  }
-
-  if (!drop_temporary)
-  {
-    if (!thd->locked_tables_mode)
-    {
-      if (lock_table_names(thd, tables, NULL, thd->variables.lock_wait_timeout,
-                           MYSQL_OPEN_SKIP_TEMPORARY))
-        DBUG_RETURN(1);
-      for (table= tables; table; table= table->next_local)
-      {
-        tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name,
-                         FALSE);
-      }
-    }
-    else
-    {
-      for (table= tables; table; table= table->next_local)
-        if (table->open_type != OT_BASE_ONLY &&
-	    find_temporary_table(thd, table))
-        {
-          /*
-            A temporary table.
-
-            Don't try to find a corresponding MDL lock or assign it
-            to table->mdl_request.ticket. There can't be metadata
-            locks for temporary tables: they are local to the session.
-
-            Later in this function we release the MDL lock only if
-            table->mdl_requeset.ticket is not NULL. Thus here we
-            ensure that we won't release the metadata lock on the base
-            table locked with LOCK TABLES as a side effect of temporary
-            table drop.
-          */
-          DBUG_ASSERT(table->mdl_request.ticket == NULL);
-        }
-        else
-        {
-          /*
-            Not a temporary table.
-
-            Since 'tables' list can't contain duplicates (this is ensured
-            by parser) it is safe to cache pointer to the TABLE instances
-            in its elements.
-          */
-          table->table= find_table_for_mdl_upgrade(thd->open_tables, table->db,
-                                                   table->table_name, FALSE);
-          if (!table->table)
-            DBUG_RETURN(1);
-          table->mdl_request.ticket= table->table->mdl_ticket;
-        }
-    }
-  }
-
   for (table= tables; table; table= table->next_local)
   {
     bool is_trans;
@@ -2040,6 +2038,16 @@ int mysql_rm_table_part2(THD *thd, TABLE
                          table->table ? (long) table->table->s : (long) -1));
 
     /*
+      If we are in locked tables mode and are dropping a temporary table,
+      the ticket should be NULL to ensure that we don't release a lock
+      on a base table later.
+    */
+    DBUG_ASSERT(!(thd->locked_tables_mode &&
+                  table->open_type != OT_BASE_ONLY &&
+                  find_temporary_table(thd, table) &&
+                  table->mdl_request.ticket != NULL));
+
+    /*
       drop_temporary_table may return one of the following error codes:
       .  0 - a temporary table was successfully dropped.
       .  1 - a temporary table was not found.
@@ -2114,6 +2122,10 @@ int mysql_rm_table_part2(THD *thd, TABLE
         table->table= 0;
       }
 
+      /* Check that we have an exclusive lock on the table to be dropped. */
+      DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, table->db,
+                                                 table->table_name,
+                                                 MDL_EXCLUSIVE));
       if (thd->killed)
       {
         error= -1;
@@ -2156,8 +2168,8 @@ int mysql_rm_table_part2(THD *thd, TABLE
         built_query.append("`,");
       }
     }
-    DEBUG_SYNC(thd, "rm_table_part2_before_delete_table");
-    DBUG_EXECUTE_IF("sleep_before_part2_delete_table",
+    DEBUG_SYNC(thd, "rm_table_no_locks_before_delete_table");
+    DBUG_EXECUTE_IF("sleep_before_no_locks_delete_table",
                     my_sleep(100000););
     error= 0;
     if (drop_temporary ||
@@ -2244,7 +2256,7 @@ int mysql_rm_table_part2(THD *thd, TABLE
                                     ER(ER_BAD_TABLE_ERROR), MYF(0),
                                     table->table_name););
   }
-  DEBUG_SYNC(thd, "rm_table_part2_before_binlog");
+  DEBUG_SYNC(thd, "rm_table_no_locks_before_binlog");
   thd->thread_specific_used|= (trans_tmp_table_deleted ||
                                non_trans_tmp_table_deleted);
   error= 0;

=== modified file 'sql/sql_table.h'
--- a/sql/sql_table.h	2010-08-20 17:15:48 +0000
+++ b/sql/sql_table.h	2010-11-15 18:13:30 +0000
@@ -174,8 +174,9 @@ bool mysql_checksum_table(THD* thd, TABL
                           HA_CHECK_OPT* check_opt);
 bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists,
                     my_bool drop_temporary);
-int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists,
-                         bool drop_temporary, bool drop_view, bool log_query);
+int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists,
+                            bool drop_temporary, bool drop_view,
+                            bool log_query);
 bool quick_rm_table(handlerton *base,const char *db,
                     const char *table_name, uint flags);
 void close_cached_table(THD *thd, TABLE *table);


Attachment: [text/bzr-bundle] bzr/jon.hauglid@oracle.com-20101115181330-kupas4zlg01h2l5p.bundle
Thread
bzr commit into mysql-5.5-runtime branch (jon.hauglid:3185) Bug#57663Jon Olav Hauglid15 Nov
  • Re: bzr commit into mysql-5.5-runtime branch (jon.hauglid:3185)Bug#57663Dmitry Lenev16 Nov