List:Commits« Previous MessageNext Message »
From:Mattias Jonsson Date:December 19 2008 8:01am
Subject:bzr commit into mysql-5.1-bugteam branch (mattias.jonsson:2742)
Bug#30102
View as plain text  
#At file:///Users/mattiasj/clones/bzrroot/b30102-51-bugteam/ based on revid:sergey.glukhov@stripped

 2742 Mattias Jonsson	2008-12-19
      Bug#30102: Rename table does corrupt tables with partition files on failure
      
      Problem was that a failing rename just left the partitions at the state
      it was at the failure
      
      Solution was to try to revert the started rename if a failure occured.
added:
  mysql-test/r/partition_not_embedded.result
  mysql-test/t/partition_not_embedded.test
modified:
  sql/ha_partition.cc
  sql/handler.cc

per-file messages:
  mysql-test/r/partition_not_embedded.result
    Bug#30102: Rename table does corrupt tables with partition files on failure
    
    New result file
  mysql-test/t/partition_not_embedded.test
    Bug#30102: Rename table does corrupt tables with partition files on failure
    
    New result file
    
    The error text and list_files did not produce the same output when
    running as embedded server, so this test does is not enabled under
    embedded.
  sql/ha_partition.cc
    Bug#30102: Rename table does corrupt tables with partition files on failure
    
    Better error handling for rename partitions (reverting the started rename
    operation).
    
    Different order of files for delete.
  sql/handler.cc
    Bug#30102: Rename table does corrupt tables with partition files on failure
    
    Does try to remove as many table files as possible, even after an error.
=== added file 'mysql-test/r/partition_not_embedded.result'
--- a/mysql-test/r/partition_not_embedded.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/partition_not_embedded.result	2008-12-19 08:01:26 +0000
@@ -0,0 +1,107 @@
+DROP TABLE IF EXISTS t1, t2;
+# Bug#30102 test
+CREATE TABLE t1 (a INT)
+PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (6),
+PARTITION `p1....................` VALUES LESS THAN (9),
+PARTITION p2 VALUES LESS THAN MAXVALUE);
+# List of files in database `test`, all original t1-files here
+t1#P#p0.MYD
+t1#P#p0.MYI
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p2.MYD
+t1#P#p2.MYI
+t1.frm
+t1.par
+INSERT INTO t1 VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10);
+# Renaming to a file name where the first partition is 250 chars
+# and the second partition is 350 chars
+RENAME TABLE t1 TO `t2_new..............................................end`;
+ERROR HY000: Error on rename of './test/t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped' to './test/t2_new@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@00
+# List of files in database `test`, should not be any t2-files here
+# List of files in database `test`, should be all t1-files here
+t1#P#p0.MYD
+t1#P#p0.MYI
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p2.MYD
+t1#P#p2.MYI
+t1.frm
+t1.par
+SELECT * FROM t1;
+a
+1
+10
+2
+3
+4
+5
+6
+7
+8
+9
+# Renaming to a file name where the first partition is 155 chars
+# and the second partition is 255 chars
+RENAME TABLE t1 TO `t2_............................end`;
+# List of files in database `test`, should not be any t1-files here
+# List of files in database `test`, should be all t2-files here
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p0.MYD
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p0.MYI
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p2.MYD
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002eend#P#p2.MYI
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+SELECT * FROM `t2_............................end`;
+a
+1
+10
+2
+3
+4
+5
+6
+7
+8
+9
+RENAME TABLE `t2_............................end` to t1;
+# List of files in database `test`, should be all t1-files here
+t1#P#p0.MYD
+t1#P#p0.MYI
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p2.MYD
+t1#P#p2.MYI
+t1.frm
+t1.par
+# Renaming to a file name where the first partition is 156 chars
+# and the second partition is 256 chars
+RENAME TABLE t1 TO `t2_............................_end`;
+ERROR HY000: Error on rename of './test/t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped' to './test/t2_@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@
+# List of files in database `test`, should not be any t2-files here
+# List of files in database `test`, should be all t1-files here
+t1#P#p0.MYD
+t1#P#p0.MYI
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p1@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@002e@stripped
+t1#P#p2.MYD
+t1#P#p2.MYI
+t1.frm
+t1.par
+SELECT * FROM t1;
+a
+1
+10
+2
+3
+4
+5
+6
+7
+8
+9
+DROP TABLE t1;
+# Should not be any files left here
+# End of bug#30102 test.

=== added file 'mysql-test/t/partition_not_embedded.test'
--- a/mysql-test/t/partition_not_embedded.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/partition_not_embedded.test	2008-12-19 08:01:26 +0000
@@ -0,0 +1,67 @@
+-- source include/have_partition.inc
+-- source include/not_embedded.inc
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2;
+--enable_warnings
+let $MYSQLD_DATADIR= `SELECT @@datadir`;
+
+#
+# Bug#30102: rename table does corrupt tables with partition files on failure
+#
+--echo # Bug#30102 test
+CREATE TABLE t1 (a INT)
+PARTITION BY RANGE (a)
+(PARTITION p0 VALUES LESS THAN (6),
+ PARTITION `p1....................` VALUES LESS THAN (9),
+ PARTITION p2 VALUES LESS THAN MAXVALUE);
+# partition p1 is 't1#P#p1' + @002e * 20 = 107 characters + file ending
+# total path lenght of './test/t1#P#p1@002e@002e<...>@002e.MY[ID]' is 118 chars
+--echo # List of files in database `test`, all original t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+INSERT INTO t1 VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10);
+--echo # Renaming to a file name where the first partition is 250 chars
+--echo # and the second partition is 350 chars
+--error 7
+RENAME TABLE t1 TO `t2_new..............................................end`;
+#                         1234567890123456789012345678901234567890123456
+--echo # List of files in database `test`, should not be any t2-files here
+--list_files $MYSQLD_DATADIR/test t2*
+--echo # List of files in database `test`, should be all t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+--sorted_result
+SELECT * FROM t1;
+--echo # Renaming to a file name where the first partition is 155 chars
+--echo # and the second partition is 255 chars
+RENAME TABLE t1 TO `t2_............................end`;
+#                      1234567890123456789012345678
+# t2_ + end
+#     .MY[ID] or .frm
+#         #P#p[012]
+#             28 * @002e
+# 6 + 4 + 5 + 28 * 5 = 155
+--echo # List of files in database `test`, should not be any t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+--echo # List of files in database `test`, should be all t2-files here
+--list_files $MYSQLD_DATADIR/test t2*
+--sorted_result
+SELECT * FROM `t2_............................end`;
+RENAME TABLE `t2_............................end` to t1;
+--echo # List of files in database `test`, should be all t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+--echo # Renaming to a file name where the first partition is 156 chars
+--echo # and the second partition is 256 chars
+--error 7
+RENAME TABLE t1 TO `t2_............................_end`;
+#                      1234567890123456789012345678
+# 7 + 4 + 5 + 28 * 5 = 16 + 140 = 156
+--echo # List of files in database `test`, should not be any t2-files here
+--list_files $MYSQLD_DATADIR/test t2*
+--echo # List of files in database `test`, should be all t1-files here
+--list_files $MYSQLD_DATADIR/test t1*
+--sorted_result
+SELECT * FROM t1;
+DROP TABLE t1;
+--echo # Should not be any files left here
+--list_files $MYSQLD_DATADIR/test t1*
+--list_files $MYSQLD_DATADIR/test t2*
+--echo # End of bug#30102 test.

=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc	2008-12-16 11:44:18 +0000
+++ b/sql/ha_partition.cc	2008-12-19 08:01:26 +0000
@@ -423,12 +423,9 @@ bool ha_partition::initialize_partition(
 
 int ha_partition::delete_table(const char *name)
 {
-  int error;
   DBUG_ENTER("ha_partition::delete_table");
 
-  if ((error= del_ren_cre_table(name, NULL, NULL, NULL)))
-    DBUG_RETURN(error);
-  DBUG_RETURN(handler::delete_table(name));
+  DBUG_RETURN(del_ren_cre_table(name, NULL, NULL, NULL));
 }
 
 
@@ -456,12 +453,9 @@ int ha_partition::delete_table(const cha
 
 int ha_partition::rename_table(const char *from, const char *to)
 {
-  int error;
   DBUG_ENTER("ha_partition::rename_table");
 
-  if ((error= del_ren_cre_table(from, to, NULL, NULL)))
-    DBUG_RETURN(error);
-  DBUG_RETURN(handler::rename_table(from, to));
+  DBUG_RETURN(del_ren_cre_table(from, to, NULL, NULL));
 }
 
 
@@ -1807,6 +1801,15 @@ uint ha_partition::del_ren_cre_table(con
   DBUG_PRINT("enter", ("from: (%s) to: (%s)", from, to));
   name_buffer_ptr= m_name_buffer_ptr;
   file= m_file;
+  if (to == NULL && table_arg == NULL)
+  {
+    /*
+      Delete table, start by delete the .par file, to not leave an unusable 
+      table behind, only non accessible partition files
+    */
+    if ((error= handler::delete_table(from)))
+      DBUG_RETURN(error);
+  }
   /*
     Since ha_partition has HA_FILE_BASED, it must alter underlying table names
     if they do not have HA_FILE_BASED and lower_case_table_names == 2.
@@ -1828,6 +1831,8 @@ uint ha_partition::del_ren_cre_table(con
       create_partition_name(to_buff, to_path, name_buffer_ptr,
                             NORMAL_PART_NAME, FALSE);
       error= (*file)->ha_rename_table(from_buff, to_buff);
+      if (error)
+        goto rename_error;
     }
     else if (table_arg == NULL)			// delete branch
       error= (*file)->ha_delete_table(from_buff);
@@ -1843,6 +1848,15 @@ uint ha_partition::del_ren_cre_table(con
       save_error= error;
     i++;
   } while (*(++file));
+  if (to != NULL)
+  {
+    if ((error= handler::rename_table(from, to)))
+    {
+      /* Try to revert everything, ignore errors */
+      (void) handler::rename_table(to, from);
+      goto rename_error;
+    }
+  }
   DBUG_RETURN(save_error);
 create_error:
   name_buffer_ptr= m_name_buffer_ptr;
@@ -1850,7 +1864,21 @@ create_error:
   {
     create_partition_name(from_buff, from_path, name_buffer_ptr, NORMAL_PART_NAME,
                           FALSE);
-    VOID((*file)->ha_delete_table((const char*) from_buff));
+    (void) (*file)->ha_delete_table((const char*) from_buff);
+    name_buffer_ptr= strend(name_buffer_ptr) + 1;
+  }
+  DBUG_RETURN(error);
+rename_error:
+  name_buffer_ptr= m_name_buffer_ptr;
+  for (abort_file= file, file= m_file; file < abort_file; file++)
+  {
+    /* Revert the rename, back from 'to' to the original 'from' */
+    create_partition_name(from_buff, from_path, name_buffer_ptr,
+                          NORMAL_PART_NAME, FALSE);
+    create_partition_name(to_buff, to_path, name_buffer_ptr,
+                          NORMAL_PART_NAME, FALSE);
+    /* Ignore error here */
+    (void) (*file)->ha_rename_table(to_buff, from_buff);
     name_buffer_ptr= strend(name_buffer_ptr) + 1;
   }
   DBUG_RETURN(error);

=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2008-11-25 09:55:30 +0000
+++ b/sql/handler.cc	2008-12-19 08:01:26 +0000
@@ -2943,6 +2943,7 @@ uint handler::get_dup_key(int error)
 */
 int handler::delete_table(const char *name)
 {
+  int saved_error= 0;
   int error= 0;
   int enoent_or_zero= ENOENT;                   // Error if no file was deleted
   char buff[FN_REFLEN];
@@ -2952,21 +2953,23 @@ int handler::delete_table(const char *na
     fn_format(buff, name, "", *ext, MY_UNPACK_FILENAME|MY_APPEND_EXT);
     if (my_delete_with_symlink(buff, MYF(0)))
     {
-      if ((error= my_errno) != ENOENT)
-	break;
+      if (my_errno != ENOENT)
+	saved_error= my_errno;
     }
     else
       enoent_or_zero= 0;                        // No error for ENOENT
     error= enoent_or_zero;
   }
-  return error;
+  return saved_error ? saved_error : error;
 }
 
 
 int handler::rename_table(const char * from, const char * to)
 {
   int error= 0;
-  for (const char **ext= bas_ext(); *ext ; ext++)
+  const char **ext, **start_ext;
+  start_ext= bas_ext();
+  for (ext= start_ext; *ext ; ext++)
   {
     if (rename_file_ext(from, to, *ext))
     {
@@ -2975,6 +2978,12 @@ int handler::rename_table(const char * f
       error= 0;
     }
   }
+  if (error)
+  {
+    /* Try to revert the rename. Ignore errors. */
+    for (; ext >= start_ext; ext--)
+      rename_file_ext(to, from, *ext);
+  }
   return error;
 }
 

Thread
bzr commit into mysql-5.1-bugteam branch (mattias.jonsson:2742)Bug#30102Mattias Jonsson19 Dec