MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Ingo Struewing Date:January 4 2010 3:46pm
Subject:bzr commit into mysql-backup-backport branch (ingo.struewing:3023)
Bug#42519 WL#5101
View as plain text  
#At file:///home2/mydev/bzrroot/mysql-5.6-backup-backport-ms09-2/ based on revid:ingo.struewing@stripped

 3023 Ingo Struewing	2010-01-04
      WL#5101 - MySQL Backup back port - MS09
      Merged (part of) revid:guilhem@stripped
        Maria online backup: BUG#42519 "Maria: RESTORE leads to corrupted table and assertion":
        * working around deficiencies of the restore kernel
        (occuring also in
        BUG 41716 "Backup Error when sending data (for table #1) to Default/Snapshot restore driver"
        BUG 40944 "Backup: crash after myisampack"); this fixes BUG#42519.
        * same workaround for MyISAM (which suffered from a bug similar to BUG#42519).
        * fixing a Maria restore driver bug found during debugging this ("DELETE FROM <table>;" was
        not handled during backup).
      Only included the changes in storage/myisam/
      The changes from sql/backup/
      Left away all maria changes.
      original changeset: 2617.26.9
     @ storage/myisam/
        WL#5101 - MySQL Backup back port - MS09
            Same changes as in (though simplified).
        Comments from storage/maria/
            * Working around a deficiency of the restore kernel:
             - it leaves a window between creating the table empty and locking it; in this window,
            another thread can do an update to the table (which conflicts with what the restore
            driver will put into files).
             - it does not always really close tables at end of restore, so restore driver has to
            make sure to invalidate any cached info before table is unlocked.
            - see this file's comments for more details about the two problems above;
            they were the causes of BUG#42519 (various corruptions or assertion failures
            in the first DMLs after RESTORE, depending on concurrency).
            * The same logic is needed independently of rebuilding the index or not.
            * keep MARIA_SHARE::close_lock when iterating over states of table,
            to protect against a concurrent checkpoint (safer).

=== modified file 'storage/myisam/'
--- a/storage/myisam/	2010-01-04 14:01:39 +0000
+++ b/storage/myisam/	2010-01-04 15:46:18 +0000
@@ -119,6 +119,23 @@ using backup::Buffer;
+  Restore kernel opens tables and locks them for the duration of restore
+  (after having created them empty); this means that cached objects stay
+  around (MYISAM_SHARE, MI_INFO) and can become out-of-sync with the
+  data/index file filled by the driver, unless we take precautions which are
+  recognizable by this symbol.
+  Restore kernel leaves a time windows between end of creation of table (via
+  execution of CREATE TABLE) and locking of this table; in this window another
+  client can open/lock/modify/unlock the table, which conflicts with what the
+  driver is going to write to the data/index file, unless we take precautions
+  which are recognizable by this symbol.
 /** Like Table_ref but with file name added */
 class Myisam_table_ref
@@ -1664,6 +1681,19 @@ Table_restore::Table_restore(const Table
     /* table does not exist or is corrupted? not normal, it's just created */
     goto err;
+  /*
+    Restore kernel leaves a window between creation of table and locking it;
+    in this window, another thread can modify the table, put pages in page
+    cache, alter state, increase the files's length to greater than what the
+    driver has to write...
+    So we re-empty it here. We know we are alone using the table at this
+    point, as restore kernel has finished locking tables.
+    See BUG#42519, BUG#41716.
+  */
+  if (mi_delete_all_rows(mi_info))
+    goto err;
     It's ok to copy the kfile descriptor and write() to it as the upper layers
     guarantee that we are the only user of the brand new table (nobody will
@@ -1717,6 +1747,7 @@ result_t Table_restore::close()
       (kfile_restore.close_file() != backup::OK))
     CAUTION! Ugliest hack ever!
     This hack tries to recover from bypassing the MyISAM interface
@@ -1809,6 +1840,7 @@ result_t Table_restore::close()
   end :
     do {} while (0); /* Empty statement, syntactically required. */
@@ -1850,7 +1882,6 @@ result_t Table_restore::post_restore()
   Vio* save_vio;
-  if (!rebuild_index)
     MI_INFO *mi_info;
     MYISAM_SHARE *share;
@@ -1877,9 +1908,11 @@ result_t Table_restore::post_restore()
       error= mi_state_info_write(share, share->kfile, &share->state, 1);
     error|= mi_close(mi_info);
-    goto err;
+  if (!rebuild_index)
+    goto err;
     myisamchk() as well as ha_myisam::repair() do a lot of operations before
     and after mi_repair(); to not duplicate code we reuse one of them.

Attachment: [text/bzr-bundle] bzr/
bzr commit into mysql-backup-backport branch (ingo.struewing:3023)Bug#42519 WL#5101Ingo Struewing4 Jan