#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/myisam_backup_engine.cc.
The changes from sql/backup/kernel.cc.
Left away all maria changes.
original changeset: 2617.26.9
@ storage/myisam/myisam_backup_engine.cc
WL#5101 - MySQL Backup back port - MS09
Same changes as in maria_backup_engine.cc (though simplified).
Comments from storage/maria/maria_backup_engine.cc:
* 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:
storage/myisam/myisam_backup_engine.cc
=== modified file 'storage/myisam/myisam_backup_engine.cc'
--- a/storage/myisam/myisam_backup_engine.cc 2010-01-04 14:01:39 +0000
+++ b/storage/myisam/myisam_backup_engine.cc 2010-01-04 15:46:18 +0000
@@ -119,6 +119,23 @@ using backup::Buffer;
*/
#define MYISAM_BACKUP_VERSION 1
+/**
+ 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.
+*/
+#define RESTORE_KERNEL_KEEPS_OPEN_TABLES 1
+/**
+ 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.
+*/
+#define RESTORE_KERNEL_NOT_ATOMIC 1
+
/** 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;
}
+#ifdef RESTORE_KERNEL_NOT_ATOMIC
+ /*
+ 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;
+#endif
/*
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))
SET_STATE_TO_ERROR_AND_DBUG_RETURN;
+#ifdef RESTORE_KERNEL_KEEPS_OPEN_TABLES
/*
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. */
}
+#endif
DBUG_RETURN(backup::OK);
}
@@ -1850,7 +1882,6 @@ result_t Table_restore::post_restore()
Vio* save_vio;
DBUG_ENTER("myisam_backup::Table_restore::post_restore");
- 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/ingo.struewing@sun.com-20100104154618-muar5988oplnvmzi.bundle
Thread |
---|
• bzr commit into mysql-backup-backport branch (ingo.struewing:3023)Bug#42519 WL#5101 | Ingo Struewing | 4 Jan |