List:Commits« Previous MessageNext Message »
From:Ingo Struewing Date:December 30 2008 12:13pm
Subject:bzr commit into mysql-6.0-backup branch (ingo.struewing:2745) Bug#40944
View as plain text  
#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#40944Ingo Struewing30 Dec