bzr commit into mysql-trunk-bugfixing branch (mattias.jonsson:3093) Bug#53676

[Mattias Jonsson]

#At file:///Users/mattiasj/mysql-bzr/b53676-trunk-bf/ based on revid:mattias.jonsson@stripped

 3093 Mattias Jonsson	2010-07-08
      Bug#53676: Unexpected errors and possible table corruption on
                 ADD PARTITION and LOCK TABLE
      
      Review fixes after Mikaels review.
     @ sql/sql_partition.cc
        Changed from using mysql_unlock_tables to use mysql_lock_remove
        instead, to avoid unlocking tables under LOCK TABLE.
        
        Fixed comments and style.

    modified:
      sql/sql_partition.cc
=== modified file 'sql/sql_partition.cc'
--- a/sql/sql_partition.cc	2010-06-11 11:03:03 +0000
+++ b/sql/sql_partition.cc	2010-07-08 12:46:07 +0000
@@ -6133,9 +6133,10 @@ static bool write_log_add_change_partiti
     goto error;
   log_entry= part_info->first_log_entry;
 
-  /* Reuse the old execute ddl_log_entry */
   if (write_execute_ddl_log_entry(log_entry->entry_pos,
-                                    FALSE, &exec_log_entry))
+                                  FALSE,
+                                  /* Reuse the old execute ddl_log_entry */
+                                  &exec_log_entry))
     goto error;
   mysql_mutex_unlock(&LOCK_gdl);
   set_part_info_exec_log_entry(part_info, exec_log_entry);
@@ -6161,9 +6162,15 @@ error:
     TRUE                     Error
     FALSE                    Success
   DESCRIPTION
-    We will write log entries that specify to remove all partitions reorganised,
-    to rename others to reflect the new naming scheme and to install the shadow
-    frm file.
+    We will write log entries that specify to
+    1) Install the shadow frm file.
+    2) Remove all partitions reorganized. (To be able to reorganize a partition
+       to the same name. Like in REORGANIZE p0 INTO (p0, p1),
+       so that the later rename from the new p0-temporary name to p0 don't
+       fail because the partition already exists.
+    3) Rename others to reflect the new naming scheme.
+
+    Note that it is written in the ddl log in reverse.
 */
 
 static bool write_log_final_change_partition(ALTER_PARTITION_PARAM_TYPE *lpt)
@@ -6323,7 +6330,7 @@ void handle_alter_part_error(ALTER_PARTI
       But we do not want to remove the table instance since we need the
       part_info object.
     */
-    mysql_unlock_tables(thd, thd->lock);
+    mysql_lock_remove(thd, thd->lock, table);
     thd->lock= NULL;
     table->file->close();
     table->db_stat= 0;
@@ -6431,6 +6438,22 @@ void handle_alter_part_error(ALTER_PARTI
 }
 
 
+/**
+  Downgrade an exclusive MDL lock if under LOCK TABLE.
+
+  If we don't downgrade the lock, it will not be downgraded or released
+  until the table is unlocked, resulting in blocking other threads using
+  the table.
+*/
+
+static void downgrade_mdl_if_lock_tables_mode(THD *thd, MDL_ticket *ticket,
+                                              enum_mdl_type type)
+{
+  if (thd->locked_tables_mode)
+    ticket->downgrade_exclusive_lock(type);
+}
+
+
 /*
   Actually perform the change requested by ALTER TABLE of partitions
   previously prepared.
@@ -6580,7 +6603,7 @@ uint fast_alter_partition_table(THD *thd
       4) Close all tables that have already been opened but didn't stumble on
          the abort locked previously. This is done as part of the
          alter_close_tables call.
-      7) Write the bin log
+      5) Write the bin log
          Unfortunately the writing of the binlog is not synchronised with
          other logging activities. So no matter in which order the binlog
          is written compared to other activities there will always be cases
@@ -6591,12 +6614,12 @@ uint fast_alter_partition_table(THD *thd
          require writing the statement first in the ddl log and then
          when recovering from the crash read the binlog and insert it into
          the binlog if not written already.
-      8) Install the previously written shadow frm file
-      9) Prepare handlers for drop of partitions
-      10) Drop the partitions
-      11) Remove entries from ddl log
-      12) Reopen table if under lock tables
-      13) Complete query
+      6) Install the previously written shadow frm file
+      7) Prepare handlers for drop of partitions
+      8) Drop the partitions
+      9) Remove entries from ddl log
+      10) Reopen table if under lock tables
+      11) Complete query
 
       We insert Error injections at all places where it could be interesting
       to test if recovery is properly done.
@@ -6614,8 +6637,8 @@ uint fast_alter_partition_table(THD *thd
         write_log_drop_partition(lpt) ||
         ERROR_INJECT_CRASH("crash_drop_partition_4") ||
         ERROR_INJECT_ERROR("fail_drop_partition_4") ||
-        (close_table_on_failure= FALSE) ||
-        (not_completed= FALSE) ||
+        (close_table_on_failure= FALSE, FALSE) ||
+        (not_completed= FALSE, FALSE) ||
         alter_close_tables(lpt) ||
         ERROR_INJECT_CRASH("crash_drop_partition_5") ||
         ERROR_INJECT_ERROR("fail_drop_partition_5") ||
@@ -6624,9 +6647,9 @@ uint fast_alter_partition_table(THD *thd
                         thd->query(), thd->query_length()), FALSE)) ||
         ERROR_INJECT_CRASH("crash_drop_partition_6") ||
         ERROR_INJECT_ERROR("fail_drop_partition_6") ||
-        ((frm_install= TRUE), FALSE) ||
+        (frm_install= TRUE, FALSE) ||
         mysql_write_frm(lpt, WFRM_INSTALL_SHADOW) ||
-        ((frm_install= FALSE), FALSE) ||
+        (frm_install= FALSE, FALSE) ||
         ERROR_INJECT_CRASH("crash_drop_partition_7") ||
         ERROR_INJECT_ERROR("fail_drop_partition_7") ||
         mysql_drop_partitions(lpt) ||
@@ -6662,7 +6685,7 @@ uint fast_alter_partition_table(THD *thd
          can release all other locks on the table and since no one can open
          the table, there can be no new threads accessing the table. They
          will be hanging on this exclusive lock.
-      3) Log the changes to happen in ddl log
+      3) Write an entry to remove the new parttions if crash occurs
       4) Add the new partitions.
       5) Close all instances of the table and remove them from the table cache.
       6) Write binlog
@@ -6691,7 +6714,7 @@ uint fast_alter_partition_table(THD *thd
         mysql_change_partitions(lpt) ||
         ERROR_INJECT_CRASH("crash_add_partition_5") ||
         ERROR_INJECT_ERROR("fail_add_partition_5") ||
-        (close_table_on_failure= FALSE) ||
+        (close_table_on_failure= FALSE, FALSE) ||
         alter_close_tables(lpt) ||
         ERROR_INJECT_CRASH("crash_add_partition_6") ||
         ERROR_INJECT_ERROR("fail_add_partition_6") ||
@@ -6701,7 +6724,8 @@ uint fast_alter_partition_table(THD *thd
         ERROR_INJECT_CRASH("crash_add_partition_7") ||
         ERROR_INJECT_ERROR("fail_add_partition_7") ||
         write_log_rename_frm(lpt) ||
-        ((frm_install= TRUE), (not_completed= FALSE)) ||
+        (frm_install= TRUE, FALSE) ||
+        (not_completed= FALSE, FALSE) ||
         ERROR_INJECT_CRASH("crash_add_partition_8") ||
         ERROR_INJECT_ERROR("fail_add_partition_8") ||
         mysql_write_frm(lpt, WFRM_INSTALL_SHADOW) ||
@@ -6821,8 +6845,7 @@ uint fast_alter_partition_table(THD *thd
       goto err;
     }
   }
-  if (thd->locked_tables_mode)
-    mdl_ticket->downgrade_exclusive_lock(MDL_SHARED_NO_READ_WRITE);
+  downgrade_mdl_if_lock_tables_mode(thd, mdl_ticket, MDL_SHARED_NO_READ_WRITE);
   /*
     A final step is to write the query to the binlog and send ok to the
     user
@@ -6831,8 +6854,7 @@ uint fast_alter_partition_table(THD *thd
                                  table_list, FALSE, NULL,
                                  written_bin_log));
 err:
-  if (thd->locked_tables_mode)
-    mdl_ticket->downgrade_exclusive_lock(MDL_SHARED_NO_READ_WRITE);
+  downgrade_mdl_if_lock_tables_mode(thd, mdl_ticket, MDL_SHARED_NO_READ_WRITE);
   table->m_needs_reopen= TRUE;
   close_thread_tables(thd);
   DBUG_RETURN(TRUE);


Attachment: [text/bzr-bundle] bzr/[e-mail]