#At file:///home2/mydev/bzrroot/mysql-6.0-bug40944-4/ based on revid:hema@strippedx
2745 Ingo Struewing 2008-12-30
Bug#40944 - Backup: crash after myisampack
After restore of a compressed MyISAM table, the server crashed
when the table was accessed.
The problem was twofold.
1. The crash happened in the MyISAM keycache. It did not handle
an error in reading a block correctly. This is now fixed in
key_cache_read(), key_cache_insert(), and key_cache_write().
Their BLOCK_ERROR handling does now follow a common pattern.
Erroneous blocks are no longer put to the LRU ring, but freed.
2. After fixing the above, accesses to a restored compressed table
failed with error messages about a corrupted table. Restore of a
table works by dropping the old table if exists, creating a new
one, and loading the data from the backup image. The MyISAM
native driver writes directly to the table files, what was there
at backup time. This works even for compressed tables, as long as
the table is freshly opened after loading the data. But RESTORE
needs to keep the table write locked until it is complete. This
includes keeping the table open. When restore is done, it unlocks
the tables and requests close. But our table cache does not
perform a full close. It marks it unused and keeps it open in the
cache. Later statements reuse the open table. But when the MyISAM
table format changes, we must force a full close. Since this is
difficult to detect, we do now always force close of restored
tables.
added:
mysql-test/r/myisam_keycache_coverage.result
mysql-test/suite/backup/r/backup_myisam.result
mysql-test/suite/backup/r/backup_myisam_coverage.result
mysql-test/suite/backup/t/backup_myisam.test
mysql-test/suite/backup/t/backup_myisam_coverage.test
mysql-test/t/myisam_keycache_coverage.test
renamed:
mysql-test/suite/backup/r/backup_myisam1.result => mysql-test/suite/backup/r/backup_myisam_extlocking.result
mysql-test/suite/backup/r/backup_myisam2.result => mysql-test/suite/backup/r/backup_myisam_sync.result
mysql-test/suite/backup/t/backup_myisam1-master.opt => mysql-test/suite/backup/t/backup_myisam_extlocking-master.opt
mysql-test/suite/backup/t/backup_myisam1.test => mysql-test/suite/backup/t/backup_myisam_extlocking.test
mysql-test/suite/backup/t/backup_myisam2.test => mysql-test/suite/backup/t/backup_myisam_sync.test
modified:
mysql-test/lib/mtr_report.pl
mysys/mf_keycache.c
sql/backup/backup_kernel.h
sql/backup/kernel.cc
storage/myisam/mi_close.c
storage/myisam/mi_open.c
storage/myisam/myisam_backup_engine.cc
mysql-test/suite/backup/r/backup_myisam_extlocking.result
mysql-test/suite/backup/r/backup_myisam_sync.result
mysql-test/suite/backup/t/backup_myisam_extlocking.test
mysql-test/suite/backup/t/backup_myisam_sync.test
per-file messages:
mysql-test/lib/mtr_report.pl
Bug#40944 - Backup: crash after myisampack
Added a filter for intentionally provoked errors
mysql-test/r/myisam_keycache_coverage.result
Bug#40944 - Backup: crash after myisampack
New test result.
mysql-test/suite/backup/r/backup_myisam.result
Bug#40944 - Backup: crash after myisampack
New test result.
mysql-test/suite/backup/r/backup_myisam_coverage.result
Bug#40944 - Backup: crash after myisampack
New test result.
mysql-test/suite/backup/r/backup_myisam_extlocking.result
Bug#40944 - Backup: crash after myisampack
Renamed test case.
Updated test result.
mysql-test/suite/backup/r/backup_myisam_sync.result
Bug#40944 - Backup: crash after myisampack
Renamed test case.
Moved test result for Bug#38045 from here to backup_myisam.result.
Updated test result.
mysql-test/suite/backup/t/backup_myisam.test
Bug#40944 - Backup: crash after myisampack
New test case.
mysql-test/suite/backup/t/backup_myisam_coverage.test
Bug#40944 - Backup: crash after myisampack
New test case.
mysql-test/suite/backup/t/backup_myisam_extlocking-master.opt
Bug#40944 - Backup: crash after myisampack
Renamed test case.
mysql-test/suite/backup/t/backup_myisam_extlocking.test
Bug#40944 - Backup: crash after myisampack
Renamed test case.
Upcased cleanup sections.
Added "USE test".
mysql-test/suite/backup/t/backup_myisam_sync.test
Bug#40944 - Backup: crash after myisampack
Renamed test case.
Moved test for Bug#38045 from here to backup_myisam.test.
Renamed backup file.
Added "USE test".
mysql-test/t/myisam_keycache_coverage.test
Bug#40944 - Backup: crash after myisampack
New test case.
mysys/mf_keycache.c
Bug#40944 - Backup: crash after myisampack
Fixed error handling in unreg_request(), wait_for_readers(),
key_cache_read(), key_cache_insert(), key_cache_write(), and
free_block().
sql/backup/backup_kernel.h
Bug#40944 - Backup: crash after myisampack
Added m_backup_tables to class Backup_restore_ctx.
sql/backup/kernel.cc
Bug#40944 - Backup: crash after myisampack
Added forced refresh for all tables restored to
Backup_restore_ctx::unlock_tables().
storage/myisam/mi_close.c
Bug#40944 - Backup: crash after myisampack
Added DBUG.
storage/myisam/mi_open.c
Bug#40944 - Backup: crash after myisampack
Added DBUG.
storage/myisam/myisam_backup_engine.cc
Bug#40944 - Backup: crash after myisampack
Added a comment.
=== modified file 'mysql-test/lib/mtr_report.pl'
--- a/mysql-test/lib/mtr_report.pl 2008-12-16 20:54:07 +0000
+++ b/mysql-test/lib/mtr_report.pl 2008-12-30 12:13:31 +0000
@@ -365,6 +365,14 @@ sub mtr_report_stats ($) {
/Restore: Open and lock tables failed in RESTORE/
) or
+ # myisam_keycache_coverage intentionally provokes errors
+ ($testname eq 'main.myisam_keycache_coverage') and
+ (
+ /Incorrect key file/ or
+ /Got an error from .* mi_update/ or
+ /MySQL thread id .* localhost root Updating/
+ ) or
+
# The tablespace test triggers error below on purpose
($testname eq 'backup.backup_tablespace') and
(
=== added file 'mysql-test/r/myisam_keycache_coverage.result'
--- a/mysql-test/r/myisam_keycache_coverage.result 1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/myisam_keycache_coverage.result 2008-12-30 12:13:31 +0000
@@ -0,0 +1,52 @@
+#
+# MyISAM keycache coverage tests.
+#
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (c1 VARCHAR(5), c2 int);
+CREATE INDEX i1 ON t1 (c1, c2);
+INSERT INTO t1 VALUES ('A',1);
+INSERT INTO t1 SELECT * FROM t1;
+INSERT INTO t1 SELECT * FROM t1;
+INSERT INTO t1 SELECT * FROM t1;
+#
+# Positive tests.
+#
+SELECT COUNT(*) FROM t1 WHERE c2 < 5;
+COUNT(*)
+8
+LOAD INDEX INTO CACHE t1;
+Table Op Msg_type Msg_text
+test.t1 preload_keys status OK
+UPDATE t1 SET c2=2;
+#
+# Close table and clear cache.
+#
+FLUSH TABLE t1;
+#
+# Inject error key_cache_read_block_error
+#
+SET debug='d,key_cache_read_block_error';
+SELECT COUNT(*) FROM t1 WHERE c2 < 5;
+ERROR HY000: Incorrect key file for table './test/t1.MYI'; try to repair it
+FLUSH TABLE t1;
+#
+# Inject error key_cache_insert_block_error
+#
+SET debug='d,key_cache_insert_block_error';
+LOAD INDEX INTO CACHE t1;
+Table Op Msg_type Msg_text
+test.t1 preload_keys error Failed to read from index file (errno: 5)
+test.t1 preload_keys status Operation failed
+FLUSH TABLE t1;
+#
+# Inject error key_cache_write_block_error
+#
+SET debug='d,key_cache_write_block_error';
+UPDATE t1 SET c2=1;
+ERROR HY000: Incorrect key file for table './test/t1.MYI'; try to repair it
+FLUSH TABLE t1;
+#
+# Cleanup
+#
+SET debug='';
+DROP TABLE t1;
=== added file 'mysql-test/suite/backup/r/backup_myisam.result'
--- a/mysql-test/suite/backup/r/backup_myisam.result 1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup/r/backup_myisam.result 2008-12-30 12:13:31 +0000
@@ -0,0 +1,41 @@
+USE test;
+DROP DATABASE IF EXISTS mysql_db1;
+#
+# Bug#40944 - Backup: crash after myisampack
+#
+CREATE DATABASE mysql_db1;
+CREATE TABLE mysql_db1.t1 (c1 VARCHAR(5), c2 int);
+CREATE INDEX i1 ON mysql_db1.t1 (c1, c2);
+INSERT INTO mysql_db1.t1 VALUES ('A',1);
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+FLUSH TABLE mysql_db1.t1;
+BACKUP DATABASE mysql_db1 to 'bup_myisam.bak';
+backup_id
+#
+RESTORE FROM 'bup_myisam.bak' OVERWRITE;
+backup_id
+#
+SELECT COUNT(*) FROM mysql_db1.t1 WHERE c2 < 5;
+COUNT(*)
+128
+DROP TABLE mysql_db1.t1;
+DROP DATABASE mysql_db1;
+
+#
+# Bug#38045 - Backup, MyISAM and file system encoding
+#
+CREATE DATABASE mysqltest;
+USE mysqltest;
+CREATE TABLE `äöüߣå` (id SERIAL) ENGINE=MyISAM;
+BACKUP DATABASE mysqltest TO 'bup_myisam.bak';
+backup_id
+#
+DROP TABLE `äöüߣå`;
+USE test;
+DROP DATABASE mysqltest;
=== added file 'mysql-test/suite/backup/r/backup_myisam_coverage.result'
--- a/mysql-test/suite/backup/r/backup_myisam_coverage.result 1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup/r/backup_myisam_coverage.result 2008-12-30 12:13:31 +0000
@@ -0,0 +1,27 @@
+USE test;
+DROP DATABASE IF EXISTS mysql_db1;
+CREATE DATABASE mysql_db1;
+CREATE TABLE mysql_db1.t1 (c1 VARCHAR(5), c2 int);
+CREATE INDEX i1 ON mysql_db1.t1 (c1, c2);
+INSERT INTO mysql_db1.t1 VALUES ('A',1);
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+FLUSH TABLE mysql_db1.t1;
+BACKUP DATABASE mysql_db1 to 'bup_myisam3.bak';
+backup_id
+#
+SET debug='d,Backup_restore_ctx_unlock_tables_alloc';
+RESTORE FROM 'bup_myisam3.bak' OVERWRITE;
+backup_id
+#
+SELECT COUNT(*) FROM mysql_db1.t1 WHERE c2 < 5;
+COUNT(*)
+128
+SET debug='';
+DROP TABLE mysql_db1.t1;
+DROP DATABASE mysql_db1;
=== renamed file 'mysql-test/suite/backup/r/backup_myisam1.result' => 'mysql-test/suite/backup/r/backup_myisam_extlocking.result'
--- a/mysql-test/suite/backup/r/backup_myisam1.result 2008-10-07 17:15:44 +0000
+++ b/mysql-test/suite/backup/r/backup_myisam_extlocking.result 2008-12-30 12:13:31 +0000
@@ -1,7 +1,9 @@
-drop database if exists mysqltest;
-create database mysqltest;
-use mysqltest;
+USE test;
+DROP DATABASE IF EXISTS mysqltest;
+CREATE DATABASE mysqltest;
+USE mysqltest;
CREATE TABLE t1 (a int) engine=myisam;
BACKUP DATABASE mysqltest TO 'test.ba';
ERROR HY000: Got error -1 'online backup impossible with --external-locking' from MyISAM
+USE test;
DROP DATABASE mysqltest;
=== renamed file 'mysql-test/suite/backup/r/backup_myisam2.result' => 'mysql-test/suite/backup/r/backup_myisam_sync.result'
--- a/mysql-test/suite/backup/r/backup_myisam2.result 2008-10-07 17:15:44 +0000
+++ b/mysql-test/suite/backup/r/backup_myisam_sync.result 2008-12-30 12:13:31 +0000
@@ -1,4 +1,5 @@
SET DEBUG_SYNC= 'RESET';
+USE test;
DROP DATABASE IF EXISTS mysqltest;
CREATE DATABASE mysqltest;
USE mysqltest;
@@ -7,7 +8,7 @@ CREATE TABLE t1 (c1 LONGTEXT) ENGINE=MyI
connection backup: Start backup
SET DEBUG_SYNC= 'before_backup_data_prepare SIGNAL bup_sync
WAIT_FOR bup_finish';
-BACKUP DATABASE mysqltest TO 'test.ba';
+BACKUP DATABASE mysqltest TO 'bup_myisam_sync.bak';
connection default: Wait for BACKUP to reach its sync point
SET DEBUG_SYNC= 'now WAIT_FOR bup_sync';
@@ -45,7 +46,7 @@ REPAIR TABLE t1 QUICK;
Table Op Msg_type Msg_text
mysqltest.t1 repair status OK
DROP DATABASE mysqltest;
-RESTORE FROM 'test.ba';
+RESTORE FROM 'bup_myisam_sync.bak';
backup_id
#
SELECT LENGTH(c1) FROM t1;
@@ -58,15 +59,6 @@ mysqltest.t1 1728069308
connection default: cleanup
SET DEBUG_SYNC= 'RESET';
-#
-# Bug#38045 - Backup, MyISAM and file system encoding
-#
-CREATE TABLE `�ߣ�(id SERIAL) ENGINE=MyISAM;
-BACKUP DATABASE mysqltest TO 'test.ba';
-backup_id
-#
-DROP TABLE `�ߣ�
-
# final cleanup
USE test;
DROP DATABASE mysqltest;
=== added file 'mysql-test/suite/backup/t/backup_myisam.test'
--- a/mysql-test/suite/backup/t/backup_myisam.test 1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup/t/backup_myisam.test 2008-12-30 12:13:31 +0000
@@ -0,0 +1,62 @@
+#
+# Tests with MyISAM native driver.
+#
+
+--source include/not_embedded.inc
+
+#
+# Precautionary cleanup
+#
+--disable_warnings
+USE test;
+DROP DATABASE IF EXISTS mysql_db1;
+--error 0,1
+--remove_file $MYSQLTEST_VARDIR/master-data/bup_myisam.bak
+--enable_warnings
+
+--echo #
+--echo # Bug#40944 - Backup: crash after myisampack
+--echo #
+CREATE DATABASE mysql_db1;
+CREATE TABLE mysql_db1.t1 (c1 VARCHAR(5), c2 int);
+CREATE INDEX i1 ON mysql_db1.t1 (c1, c2);
+INSERT INTO mysql_db1.t1 VALUES ('A',1);
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+FLUSH TABLE mysql_db1.t1;
+#
+--exec $MYISAMPACK -s $MYSQLTEST_VARDIR/master-data/mysql_db1/t1
+--exec $MYISAMCHK -srq $MYSQLTEST_VARDIR/master-data/mysql_db1/t1
+#
+--replace_column 1 #
+BACKUP DATABASE mysql_db1 to 'bup_myisam.bak';
+--replace_column 1 #
+RESTORE FROM 'bup_myisam.bak' OVERWRITE;
+#
+SELECT COUNT(*) FROM mysql_db1.t1 WHERE c2 < 5;
+#
+# Cleanup
+#
+DROP TABLE mysql_db1.t1;
+DROP DATABASE mysql_db1;
+--remove_file $MYSQLTEST_VARDIR/master-data/bup_myisam.bak
+
+--echo
+--echo #
+--echo # Bug#38045 - Backup, MyISAM and file system encoding
+--echo #
+CREATE DATABASE mysqltest;
+USE mysqltest;
+CREATE TABLE `äöüߣå` (id SERIAL) ENGINE=MyISAM;
+--replace_column 1 #
+BACKUP DATABASE mysqltest TO 'bup_myisam.bak';
+DROP TABLE `äöüߣå`;
+USE test;
+DROP DATABASE mysqltest;
+--remove_file $MYSQLTEST_VARDIR/master-data/bup_myisam.bak
+
=== added file 'mysql-test/suite/backup/t/backup_myisam_coverage.test'
--- a/mysql-test/suite/backup/t/backup_myisam_coverage.test 1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/backup/t/backup_myisam_coverage.test 2008-12-30 12:13:31 +0000
@@ -0,0 +1,53 @@
+#
+# Tests with MyISAM native driver, coverage testing.
+#
+
+--source include/have_debug.inc
+--source include/not_embedded.inc
+
+#
+# Precautionary cleanup
+#
+--disable_warnings
+USE test;
+DROP DATABASE IF EXISTS mysql_db1;
+--error 0,1
+--remove_file $MYSQLTEST_VARDIR/master-data/bup_myisam3.bak
+--enable_warnings
+
+#
+# Bug#40944 - Backup: crash after myisampack
+#
+CREATE DATABASE mysql_db1;
+CREATE TABLE mysql_db1.t1 (c1 VARCHAR(5), c2 int);
+CREATE INDEX i1 ON mysql_db1.t1 (c1, c2);
+INSERT INTO mysql_db1.t1 VALUES ('A',1);
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+INSERT INTO mysql_db1.t1 SELECT * FROM mysql_db1.t1;
+FLUSH TABLE mysql_db1.t1;
+#
+--exec $MYISAMPACK -s $MYSQLTEST_VARDIR/master-data/mysql_db1/t1
+--exec $MYISAMCHK -srq $MYSQLTEST_VARDIR/master-data/mysql_db1/t1
+#
+--replace_column 1 #
+BACKUP DATABASE mysql_db1 to 'bup_myisam3.bak';
+#
+# Inject error at Backup_restore_ctx_unlock_tables_alloc
+SET debug='d,Backup_restore_ctx_unlock_tables_alloc';
+--replace_column 1 #
+RESTORE FROM 'bup_myisam3.bak' OVERWRITE;
+#
+SELECT COUNT(*) FROM mysql_db1.t1 WHERE c2 < 5;
+#
+# Cleanup
+#
+SET debug='';
+DROP TABLE mysql_db1.t1;
+DROP DATABASE mysql_db1;
+--remove_file $MYSQLTEST_VARDIR/master-data/bup_myisam3.bak
+
=== renamed file 'mysql-test/suite/backup/t/backup_myisam1-master.opt' => 'mysql-test/suite/backup/t/backup_myisam_extlocking-master.opt'
=== renamed file 'mysql-test/suite/backup/t/backup_myisam1.test' => 'mysql-test/suite/backup/t/backup_myisam_extlocking.test'
--- a/mysql-test/suite/backup/t/backup_myisam1.test 2008-11-25 17:44:19 +0000
+++ b/mysql-test/suite/backup/t/backup_myisam_extlocking.test 2008-12-30 12:13:31 +0000
@@ -7,13 +7,14 @@
# Cleanup from former test cases
#
--disable_warnings
-drop database if exists mysqltest;
+USE test;
+DROP DATABASE IF EXISTS mysqltest;
--enable_warnings
--error 0,1
--remove_file $MYSQLTEST_VARDIR/master-data/test.ba
-create database mysqltest;
-use mysqltest;
+CREATE DATABASE mysqltest;
+USE mysqltest;
CREATE TABLE t1 (a int) engine=myisam;
--replace_column 1 #
@@ -23,6 +24,7 @@ BACKUP DATABASE mysqltest TO 'test.ba';
#
# Cleanup from this test case
#
+USE test;
DROP DATABASE mysqltest;
# Note: The backup file should not exist as BACKUP command failed.
--error 1
=== renamed file 'mysql-test/suite/backup/t/backup_myisam2.test' => 'mysql-test/suite/backup/t/backup_myisam_sync.test'
--- a/mysql-test/suite/backup/t/backup_myisam2.test 2008-10-07 17:15:44 +0000
+++ b/mysql-test/suite/backup/t/backup_myisam_sync.test 2008-12-30 12:13:31 +0000
@@ -8,9 +8,10 @@
#
--disable_warnings
SET DEBUG_SYNC= 'RESET';
+USE test;
DROP DATABASE IF EXISTS mysqltest;
--error 0,1
---remove_file $MYSQLTEST_VARDIR/master-data/test.ba
+--remove_file $MYSQLTEST_VARDIR/master-data/bup_myisam_sync.bak
--enable_warnings
#
@@ -38,7 +39,7 @@ CREATE TABLE t1 (c1 LONGTEXT) ENGINE=MyI
# to be emitted by another connection.
SET DEBUG_SYNC= 'before_backup_data_prepare SIGNAL bup_sync
WAIT_FOR bup_finish';
- send BACKUP DATABASE mysqltest TO 'test.ba';
+ send BACKUP DATABASE mysqltest TO 'bup_myisam_sync.bak';
--echo
--echo connection default: Wait for BACKUP to reach its sync point
@@ -68,7 +69,7 @@ SET DEBUG_SYNC= 'now SIGNAL bup_finish';
DROP DATABASE mysqltest;
--replace_column 1 #
- RESTORE FROM 'test.ba';
+ RESTORE FROM 'bup_myisam_sync.bak';
SELECT LENGTH(c1) FROM t1;
CHECKSUM TABLE t1;
@@ -78,21 +79,9 @@ SET DEBUG_SYNC= 'now SIGNAL bup_finish';
--echo
--echo connection default: cleanup
connection default;
---remove_file $MYSQLTEST_VARDIR/master-data/test.ba
+--remove_file $MYSQLTEST_VARDIR/master-data/bup_myisam_sync.bak
SET DEBUG_SYNC= 'RESET';
-
---echo
---echo #
---echo # Bug#38045 - Backup, MyISAM and file system encoding
---echo #
-CREATE TABLE `�ߣ�(id SERIAL) ENGINE=MyISAM;
---replace_column 1 #
-BACKUP DATABASE mysqltest TO 'test.ba';
-DROP TABLE `�ߣ�
---remove_file $MYSQLTEST_VARDIR/master-data/test.ba
-
-
#
# Cleanup from this test case
#
=== added file 'mysql-test/t/myisam_keycache_coverage.test'
--- a/mysql-test/t/myisam_keycache_coverage.test 1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/myisam_keycache_coverage.test 2008-12-30 12:13:31 +0000
@@ -0,0 +1,58 @@
+--echo #
+--echo # MyISAM keycache coverage tests.
+--echo #
+
+--source include/have_debug.inc
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+CREATE TABLE t1 (c1 VARCHAR(5), c2 int);
+CREATE INDEX i1 ON t1 (c1, c2);
+INSERT INTO t1 VALUES ('A',1);
+INSERT INTO t1 SELECT * FROM t1;
+INSERT INTO t1 SELECT * FROM t1;
+INSERT INTO t1 SELECT * FROM t1;
+
+--echo #
+--echo # Positive tests.
+--echo #
+SELECT COUNT(*) FROM t1 WHERE c2 < 5;
+LOAD INDEX INTO CACHE t1;
+UPDATE t1 SET c2=2;
+
+--echo #
+--echo # Close table and clear cache.
+--echo #
+FLUSH TABLE t1;
+
+--echo #
+--echo # Inject error key_cache_read_block_error
+--echo #
+SET debug='d,key_cache_read_block_error';
+--error 126
+SELECT COUNT(*) FROM t1 WHERE c2 < 5;
+FLUSH TABLE t1;
+
+--echo #
+--echo # Inject error key_cache_insert_block_error
+--echo #
+SET debug='d,key_cache_insert_block_error';
+LOAD INDEX INTO CACHE t1;
+FLUSH TABLE t1;
+
+--echo #
+--echo # Inject error key_cache_write_block_error
+--echo #
+SET debug='d,key_cache_write_block_error';
+--error 126
+UPDATE t1 SET c2=1;
+FLUSH TABLE t1;
+
+--echo #
+--echo # Cleanup
+--echo #
+SET debug='';
+DROP TABLE t1;
+
=== modified file 'mysys/mf_keycache.c'
--- a/mysys/mf_keycache.c 2008-10-20 09:16:47 +0000
+++ b/mysys/mf_keycache.c 2008-12-30 12:13:31 +0000
@@ -1373,7 +1373,11 @@ static void unreg_request(KEY_CACHE *key
DBUG_ASSERT(block->prev_changed && *block->prev_changed == block);
DBUG_ASSERT(!block->next_used);
DBUG_ASSERT(!block->prev_used);
- if (! --block->requests)
+ /*
+ Unregister the request, but do not link erroneous blocks into the
+ LRU ring.
+ */
+ if (!--block->requests && !(block->status & BLOCK_ERROR))
{
my_bool hot;
if (block->hits_left)
@@ -1455,8 +1459,7 @@ static void wait_for_readers(KEY_CACHE *
#ifdef THREAD
struct st_my_thread_var *thread= my_thread_var;
DBUG_ASSERT(block->status & (BLOCK_READ | BLOCK_IN_USE));
- DBUG_ASSERT(!(block->status & (BLOCK_ERROR | BLOCK_IN_FLUSH |
- BLOCK_CHANGED)));
+ DBUG_ASSERT(!(block->status & (BLOCK_IN_FLUSH | BLOCK_CHANGED)));
DBUG_ASSERT(block->hash_link);
DBUG_ASSERT(block->hash_link->block == block);
/* Linked in file_blocks or changed_blocks hash. */
@@ -2567,7 +2570,6 @@ uchar *key_cache_read(KEY_CACHE *keycach
reg1 BLOCK_LINK *block;
uint read_length;
uint offset;
- uint status;
int page_st;
/*
@@ -2665,7 +2667,7 @@ uchar *key_cache_read(KEY_CACHE *keycach
}
/* block status may have added BLOCK_ERROR in the above 'if'. */
- if (!((status= block->status) & BLOCK_ERROR))
+ if (!(block->status & BLOCK_ERROR))
{
#ifndef THREAD
if (! return_buffer)
@@ -2691,14 +2693,22 @@ uchar *key_cache_read(KEY_CACHE *keycach
remove_reader(block);
- /*
- Link the block into the LRU ring if it's the last submitted
- request for the block. This enables eviction for the block.
- */
- unreg_request(keycache, block, 1);
+ /* Error injection for coverage testing. */
+ DBUG_EXECUTE_IF("key_cache_read_block_error",
+ block->status|= BLOCK_ERROR;);
- if (status & BLOCK_ERROR)
+ /* Do not link erroneous blocks into the LRU ring, but free them. */
+ if (!(block->status & BLOCK_ERROR))
+ {
+ /*
+ Link the block into the LRU ring if it's the last submitted
+ request for the block. This enables eviction for the block.
+ */
+ unreg_request(keycache, block, 1);
+ }
+ else
{
+ free_block(keycache, block);
error= 1;
break;
}
@@ -2947,16 +2957,25 @@ int key_cache_insert(KEY_CACHE *keycache
remove_reader(block);
- /*
- Link the block into the LRU ring if it's the last submitted
- request for the block. This enables eviction for the block.
- */
- unreg_request(keycache, block, 1);
-
- error= (block->status & BLOCK_ERROR);
+ /* Error injection for coverage testing. */
+ DBUG_EXECUTE_IF("key_cache_insert_block_error",
+ block->status|= BLOCK_ERROR; errno=EIO;);
- if (error)
+ /* Do not link erroneous blocks into the LRU ring, but free them. */
+ if (!(block->status & BLOCK_ERROR))
+ {
+ /*
+ Link the block into the LRU ring if it's the last submitted
+ request for the block. This enables eviction for the block.
+ */
+ unreg_request(keycache, block, 1);
+ }
+ else
+ {
+ free_block(keycache, block);
+ error= 1;
break;
+ }
buff+= read_length;
filepos+= read_length+offset;
@@ -3245,14 +3264,24 @@ int key_cache_write(KEY_CACHE *keycache,
*/
remove_reader(block);
- /*
- Link the block into the LRU ring if it's the last submitted
- request for the block. This enables eviction for the block.
- */
- unreg_request(keycache, block, 1);
+ /* Error injection for coverage testing. */
+ DBUG_EXECUTE_IF("key_cache_write_block_error",
+ block->status|= BLOCK_ERROR;);
- if (block->status & BLOCK_ERROR)
+ /* Do not link erroneous blocks into the LRU ring, but free them. */
+ if (!(block->status & BLOCK_ERROR))
{
+ /*
+ Link the block into the LRU ring if it's the last submitted
+ request for the block. This enables eviction for the block.
+ */
+ unreg_request(keycache, block, 1);
+ }
+ else
+ {
+ /* Pretend a "clean" block to avoid complications. */
+ block->status&= ~(BLOCK_CHANGED);
+ free_block(keycache, block);
error= 1;
break;
}
@@ -3328,8 +3357,9 @@ static void free_block(KEY_CACHE *keycac
{
KEYCACHE_THREAD_TRACE("free block");
KEYCACHE_DBUG_PRINT("free_block",
- ("block %u to be freed, hash_link %p",
- BLOCK_NUMBER(block), block->hash_link));
+ ("block %u to be freed, hash_link %p status: %u",
+ BLOCK_NUMBER(block), block->hash_link,
+ block->status));
/*
Assert that the block is not free already. And that it is in a clean
state. Note that the block might just be assigned to a hash_link and
@@ -3411,10 +3441,14 @@ static void free_block(KEY_CACHE *keycac
if (block->status & BLOCK_IN_EVICTION)
return;
- /* Here the block must be in the LRU ring. Unlink it again. */
- DBUG_ASSERT(block->next_used && block->prev_used &&
- *block->prev_used == block);
- unlink_block(keycache, block);
+ /* Error blocks are not put into the LRU ring. */
+ if (!(block->status & BLOCK_ERROR))
+ {
+ /* Here the block must be in the LRU ring. Unlink it again. */
+ DBUG_ASSERT(block->next_used && block->prev_used &&
+ *block->prev_used == block);
+ unlink_block(keycache, block);
+ }
if (block->temperature == BLOCK_WARM)
keycache->warm_blocks--;
block->temperature= BLOCK_COLD;
=== modified file 'sql/backup/backup_kernel.h'
--- a/sql/backup/backup_kernel.h 2008-12-18 21:46:36 +0000
+++ b/sql/backup/backup_kernel.h 2008-12-30 12:13:31 +0000
@@ -150,7 +150,13 @@ private:
/**
Indicates if tables have been locked with @c lock_tables_for_restore()
*/
- bool m_tables_locked;
+ bool m_tables_locked;
+
+ /**
+ Table list created by lock_tables_for_restore() and used by
+ unlock_tables(). Members are allocated from m_thd->mem_root.
+ */
+ TABLE_LIST *m_backup_tables;
/**
Indicates we must turn binlog back on in the close method. This is
=== modified file 'sql/backup/kernel.cc'
--- a/sql/backup/kernel.cc 2008-12-18 21:46:36 +0000
+++ b/sql/backup/kernel.cc 2008-12-30 12:13:31 +0000
@@ -848,15 +848,17 @@ Backup_restore_ctx::prepare_for_restore(
*/
int Backup_restore_ctx::lock_tables_for_restore()
{
- TABLE_LIST *tables= NULL;
int ret;
/*
- Iterate over all tables in all snapshots and create a linked TABLE_LIST
- for call to open_and_lock_tables(). Store pointers to TABLE_LIST structures
- in the restore catalogue for later access to opened tables.
- */
+ Iterate over all tables in all snapshots and create a linked
+ TABLE_LIST for call to open_and_lock_tables(). Remember the
+ TABLE_LIST for later use in Backup_restore_ctx::unlock_tables().
+ Store pointers to TABLE_LIST structures in the restore catalogue for
+ later access to opened tables.
+ */
+ m_backup_tables= NULL;
for (uint s= 0; s < m_catalog->snap_count(); ++s)
{
backup::Snapshot_info *snap= m_catalog->m_snap[s];
@@ -873,7 +875,8 @@ int Backup_restore_ctx::lock_tables_for_
return fatal_error(log_error(ER_OUT_OF_RESOURCES));
}
- tables= backup::link_table_list(*ptr, tables); // Never errors
+ m_backup_tables= backup::link_table_list(*ptr, m_backup_tables);
+ DBUG_ASSERT(m_backup_tables);
tbl->m_table= ptr;
}
}
@@ -887,7 +890,7 @@ int Backup_restore_ctx::lock_tables_for_
Note 2: Skiping tmp tables is also important because otherwise a tmp table
can occlude a regular table with the same name (BUG#33574).
*/
- ret= open_and_lock_tables_derived(m_thd, tables,
+ ret= open_and_lock_tables_derived(m_thd, m_backup_tables,
FALSE, /* do not process derived tables */
MYSQL_OPEN_SKIP_TEMPORARY
/* do not open tmp tables */
@@ -910,6 +913,22 @@ void Backup_restore_ctx::unlock_tables()
DBUG_PRINT("restore",("unlocking tables"));
+ /*
+ Refresh tables that have been restored. Some restore drivers might
+ restore a table layout that differs from the version created by
+ materialize(). We need to force a final close after restore with
+ close_cached_tables(). Note that we do this before we unlock the
+ tables. Otherwise other threads could use the still open tables
+ before we refresh them.
+
+ For information about a concrete problem, see the comment in
+ myisam_backup_engine.cc:Table_restore::close().
+
+ Use the restore table list as created by lock_tables_for_restore().
+ */
+ if (m_backup_tables)
+ close_cached_tables(m_thd, m_backup_tables, FALSE, FALSE);
+
close_thread_tables(m_thd); // Never errors
m_tables_locked= FALSE;
=== modified file 'storage/myisam/mi_close.c'
--- a/storage/myisam/mi_close.c 2008-07-09 07:12:43 +0000
+++ b/storage/myisam/mi_close.c 2008-12-30 12:13:31 +0000
@@ -30,6 +30,7 @@ int mi_close(register MI_INFO *info)
DBUG_PRINT("enter",("base: %p reopen: %u locks: %u",
info, (uint) share->reopen,
(uint) share->tot_locks));
+ DBUG_PRINT("myisam", ("close '%s'", share->unresolv_file_name));
pthread_mutex_lock(&THR_LOCK_myisam);
if (info->lock_type == F_EXTRA_LCK)
@@ -65,6 +66,7 @@ int mi_close(register MI_INFO *info)
my_free(mi_get_rec_buff_ptr(info, info->rec_buff), MYF(MY_ALLOW_ZERO_PTR));
if (flag)
{
+ DBUG_PRINT("myisam", ("close share '%s'", share->unresolv_file_name));
if (share->kfile >= 0 &&
flush_key_blocks(share->key_cache, share->kfile,
share->temporary ? FLUSH_IGNORE_CHANGED :
=== modified file 'storage/myisam/mi_open.c'
--- a/storage/myisam/mi_open.c 2008-10-20 09:16:47 +0000
+++ b/storage/myisam/mi_open.c 2008-12-30 12:13:31 +0000
@@ -89,6 +89,7 @@ MI_INFO *mi_open(const char *name, int m
my_off_t key_root[HA_MAX_POSSIBLE_KEY],key_del[MI_MAX_KEY_BLOCK_SIZE];
ulonglong max_key_file_length, max_data_file_length;
DBUG_ENTER("mi_open");
+ DBUG_PRINT("myisam", ("open '%s'", name));
LINT_INIT(m_info);
kfile= -1;
@@ -115,6 +116,7 @@ MI_INFO *mi_open(const char *name, int m
pthread_mutex_lock(&THR_LOCK_myisam);
if (!(old_info=test_if_reopen(name_buff)))
{
+ DBUG_PRINT("myisam", ("open share '%s'", name));
share= &share_buff;
bzero((uchar*) &share_buff,sizeof(share_buff));
share_buff.state.rec_per_key_part=rec_per_key_part;
=== modified file 'storage/myisam/myisam_backup_engine.cc'
--- a/storage/myisam/myisam_backup_engine.cc 2008-08-11 16:06:30 +0000
+++ b/storage/myisam/myisam_backup_engine.cc 2008-12-30 12:13:31 +0000
@@ -1754,6 +1754,25 @@ result_t Table_restore::close()
But since the share does now cache the new values from the
index file, the backup kernel's close writes the correct
information back to the file.
+
+ This used to work until a brave soul tried to backup and restore
+ compressed tables. Now we know, that replacing the state info is
+ insufficient. The table is always re-created as a non-compressed
+ table. The setup of the share is pretty different between normal and
+ compressed tables. We could try to replace all relevant information.
+ But that would make quite some code duplication with mi_open().
+ Changes there might be forgotten here. And it might still be
+ insufficient. The table instance MI_OPEN might have some setup
+ differences too. Perhaps even the handler ha_myisam. In theory it
+ might even happen that we create fixed length records, while the
+ restored MYI has dynamic records or vice versa. Or we restore a
+ table that had been created by a former MySQL version and has
+ different field types, e.g. varchar.
+
+ So the only practical solution seems to be to re-open the table
+ after restore. But this must be done in the server. The fix here is
+ still required to defeat writing of wrong share data at close as
+ decribed above.
*/
{
MI_INFO *mi_info;
| Thread |
|---|
| • bzr commit into mysql-6.0-backup branch (ingo.struewing:2745) Bug#40944 | Ingo Struewing | 30 Dec |
