From: Mattias Jonsson
Date: April 18 2012 6:19pm
Subject: bzr push into mysql-trunk branch (mattias.jonsson:3878 to 3879) Bug#11940249
List-Archive: http://lists.mysql.com/commits/143520
X-Bug: 11940249
Message-Id: <201204181819.q3IIJEQX003631@acsmt356.oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

 3879 Mattias Jonsson	2012-04-17
      Bug#11940249: RBR: MYISAM TABLE CORRUPTION AFTER FIRST LARGE INSERT
                                 ON SLAVE
      
      Problem was during repair of myisam (as a part of bulk insert optimization)
      it unlocked the table, which results in when the statement used multiple
      rpl row events for its new rows,  the following events was not properly
      locked.
      
      Fixed by not taking/release locks in ha_myisam::repair if already
      holding a write lock.
      
      Updated with reviewers comments.

    added:
      mysql-test/r/myisam_row_rpl.result
      mysql-test/t/myisam_row_rpl-master.opt
      mysql-test/t/myisam_row_rpl-slave.opt
      mysql-test/t/myisam_row_rpl.test
    modified:
      storage/myisam/ha_myisam.cc
 3878 Alexander Barkov	2012-04-18
      Fixing "conversion from 'size_t' to 'uint', possible loss of data"
      warnings on Windows.

    modified:
      sql/item.h
      sql/sql_show.cc
      sql/sql_string.h
=== added file 'mysql-test/r/myisam_row_rpl.result'
--- a/mysql-test/r/myisam_row_rpl.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/myisam_row_rpl.result	revid:mattias.jonsson@stripped
@@ -0,0 +1,60 @@
+include/master-slave.inc
+Warnings:
+Note	####	Sending passwords in plain text without SSL/TLS is extremely insecure.
+Note	####	Storing MySQL user name or password information in the master.info repository is not secure and is therefore not recommended. Please see the MySQL Manual for more about this issue and possible alternatives.
+[connection master]
+#
+# Bug#11940249: RBR: MYISAM TABLE CORRUPTION AFTER FIRST LARGE INSERT
+#               ON SLAVE
+#
+# Must have > 100 rows in the first rpl event (to trigger bulk_insert
+# optimization for insert into an empty table, by disable all non-unique
+# indexes and recreate them afterwards.)
+# and then it must be a second rpl event for the same insert (i.e.
+# during the same lock).
+# Note that --binlog-row-event-max-size=1024 is set in the .opt files
+# to enforce the default size.
+CREATE TABLE tmp (a VARCHAR(10), b INT) ENGINE=Memory;
+INSERT INTO tmp VALUES ('aZa', 1), ('zAz', 2), ('M', 3);
+INSERT INTO tmp SELECT * FROM tmp;
+INSERT INTO tmp SELECT * FROM tmp;
+INSERT INTO tmp SELECT * FROM tmp;
+INSERT INTO tmp SELECT * FROM tmp;
+INSERT INTO tmp SELECT * FROM tmp;
+INSERT INTO tmp SELECT * FROM tmp;
+CREATE TABLE t
+(a VARCHAR(10),
+b INT,
+KEY a (a))
+ENGINE = MyISAM;
+INSERT INTO t SELECT * FROM tmp;
+# on slave:
+SELECT COUNT(*) FROM t WHERE b > -1;
+COUNT(*)
+192
+# on master:
+include/show_binlog_events.inc
+Log_name	Pos	Event_type	Server_id	End_log_pos	Info
+master-bin.000001	#	Query	#	#	BEGIN
+master-bin.000001	#	Table_map	#	#	table_id: # (test.t)
+master-bin.000001	#	Write_rows	#	#	table_id: #
+master-bin.000001	#	Write_rows	#	#	table_id: # flags: STMT_END_F
+master-bin.000001	#	Query	#	#	COMMIT
+RENAME TABLE t to t_2;
+RENAME TABLE t_2 to t;
+# on slave:
+SELECT COUNT(*) FROM t WHERE b > -1;
+COUNT(*)
+192
+CHECK TABLE t;
+Table	Op	Msg_type	Msg_text
+test.t	check	status	OK
+REPAIR TABLE t;
+Table	Op	Msg_type	Msg_text
+test.t	repair	status	OK
+SELECT COUNT(*) FROM t WHERE b > -1;
+COUNT(*)
+192
+include/diff_tables.inc [master:t, slave:t]
+DROP TABLE t, tmp;
+include/rpl_end.inc

=== added file 'mysql-test/t/myisam_row_rpl-master.opt'
--- a/mysql-test/t/myisam_row_rpl-master.opt	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/myisam_row_rpl-master.opt	revid:mattias.jonsson@stripped
@@ -0,0 +1 @@
+--binlog-format=row --binlog-row-event-max-size=1024

=== added file 'mysql-test/t/myisam_row_rpl-slave.opt'
--- a/mysql-test/t/myisam_row_rpl-slave.opt	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/myisam_row_rpl-slave.opt	revid:mattias.jonsson@stripped
@@ -0,0 +1 @@
+--binlog-format=row --binlog-row-event-max-size=1024

=== added file 'mysql-test/t/myisam_row_rpl.test'
--- a/mysql-test/t/myisam_row_rpl.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/myisam_row_rpl.test	revid:mattias.jonsson@stripped
@@ -0,0 +1,67 @@
+--source include/have_binlog_format_row.inc
+--source include/master-slave.inc
+
+--echo #
+--echo # Bug#11940249: RBR: MYISAM TABLE CORRUPTION AFTER FIRST LARGE INSERT
+--echo #               ON SLAVE
+--echo #
+
+--echo # Must have > 100 rows in the first rpl event (to trigger bulk_insert
+--echo # optimization for insert into an empty table, by disable all non-unique
+--echo # indexes and recreate them afterwards.)
+--echo # and then it must be a second rpl event for the same insert (i.e.
+--echo # during the same lock).
+--echo # Note that --binlog-row-event-max-size=1024 is set in the .opt files
+--echo # to enforce the default size.
+
+CREATE TABLE tmp (a VARCHAR(10), b INT) ENGINE=Memory;
+INSERT INTO tmp VALUES ('aZa', 1), ('zAz', 2), ('M', 3);
+# 6 rows
+INSERT INTO tmp SELECT * FROM tmp;
+# 12 rows
+INSERT INTO tmp SELECT * FROM tmp;
+# 24
+INSERT INTO tmp SELECT * FROM tmp;
+# 48
+INSERT INTO tmp SELECT * FROM tmp;
+# 96
+INSERT INTO tmp SELECT * FROM tmp;
+# 192 rows
+INSERT INTO tmp SELECT * FROM tmp;
+
+CREATE TABLE t
+(a VARCHAR(10),
+ b INT,
+ KEY a (a))
+ENGINE = MyISAM;
+
+--let $binlog_file=query_get_value(SHOW MASTER STATUS, File, 1)
+--let $binlog_start=query_get_value(SHOW MASTER STATUS, Position, 1)
+INSERT INTO t SELECT * FROM tmp;
+
+--sync_slave_with_master
+--connection slave
+--echo # on slave:
+SELECT COUNT(*) FROM t WHERE b > -1;
+--connection master
+--echo # on master:
+--source include/show_binlog_events.inc
+
+RENAME TABLE t to t_2;
+RENAME TABLE t_2 to t;
+
+--sync_slave_with_master
+--connection slave
+--echo # on slave:
+SELECT COUNT(*) FROM t WHERE b > -1;
+CHECK TABLE t;
+REPAIR TABLE t;
+SELECT COUNT(*) FROM t WHERE b > -1;
+--let $diff_tables= master:t, slave:t
+--source include/diff_tables.inc
+
+--connection master
+DROP TABLE t, tmp;
+--sync_slave_with_master
+
+--source include/rpl_end.inc

=== modified file 'storage/myisam/ha_myisam.cc'
--- a/storage/myisam/ha_myisam.cc	revid:alexander.barkov@stripped
+++ b/storage/myisam/ha_myisam.cc	revid:mattias.jonsson@stripped
@@ -1029,11 +1029,13 @@ int ha_myisam::repair(THD *thd, MI_CHECK
   int error=0;
   uint local_testflag=param.testflag;
   bool optimize_done= !do_optimize, statistics_done=0;
+  bool has_old_locks= thd->locked_tables_mode || file->lock_type != F_UNLCK;
   const char *old_proc_info=thd->proc_info;
   char fixed_name[FN_REFLEN];
   MYISAM_SHARE* share = file->s;
   ha_rows rows= file->state->records;
   DBUG_ENTER("ha_myisam::repair");
+  DBUG_ASSERT(file->lock_type != F_RDLCK);
 
   param.db_name=    table->s->db.str;
   param.table_name= table->alias;
@@ -1047,8 +1049,8 @@ int ha_myisam::repair(THD *thd, MI_CHECK
   // Release latches since this can take a long time
   ha_release_temporary_latches(thd);
 
-  // Don't lock tables if we have used LOCK TABLE
-  if (! thd->locked_tables_mode &&
+  // Don't lock tables if we have used LOCK TABLE or already locked.
+  if (!has_old_locks &&
       mi_lock_database(file, table->s->tmp_table ? F_EXTRA_LCK : F_WRLCK))
   {
     char errbuf[MYSYS_STRERROR_SIZE];
@@ -1176,7 +1178,7 @@ int ha_myisam::repair(THD *thd, MI_CHECK
     update_state_info(&param, file, 0);
   }
   thd_proc_info(thd, old_proc_info);
-  if (! thd->locked_tables_mode)
+  if (!has_old_locks)
     mi_lock_database(file,F_UNLCK);
   DBUG_RETURN(error ? HA_ADMIN_FAILED :
 	      !optimize_done ? HA_ADMIN_ALREADY_DONE : HA_ADMIN_OK);

No bundle (reason: useless for push emails).