MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Andrei Elkin Date:December 12 2007 9:38am
Subject:Re: bk commit into 5.1 tree (aelkin:1.2610) BUG#31552
View as plain text  
Mats, hej.


> Hi Andrei!
>
> Patch is OK to push, but please read my comments below.
>

> Just my few cents,
> Mats Kindahl
>
> Andrei Elkin wrote:
>> Below is the list of changes that have just been committed into a local
>> 5.1 repository of elkin. When elkin does a push these changes will
>> be propagated to the main repository and, within 24 hours after the
>> push, to the public repository.
>> For information on how to access the public repository
>> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>>
>> ChangeSet@stripped, 2007-12-07 14:46:05+02:00, aelkin@stripped +29 -0
>>   Bug#31552 Replication breaks when deleting rows from out-of-sync table
>>       without PK
>>   Bug#31609 Not all RBR slave errors reported as errors
>>   bug#32468 delete rows event on a table with foreign key constraint fails
>>     The first two bugs comprise idempotency issues.
>>   First, there was no error code reported under conditions of the bug
>>   description although the slave sql thread halted.
>>   Second, executions were different with and without presence of prim key in
>>   the table.
>>   Third, there was no way to instruct the slave whether to ignore an error
>>   and skip to the following event or to halt.
>>   Fourth, there are handler errors which might happen due to idempotent
>>   applying of binlog but those were not listed among the "idempotent" error
>>   list.
>>     All the named issues are addressed.
>>   Wrt to the 3rd, there is the new global system variable, changeble at run
>>   time, which controls the slave sql thread behaviour.
>>   The new variable allows further extensions to mimic the sql_mode
>>   session/global variable.
>>   To address the 4th, the new bug#32468 had to be fixed as it was staying
>>   in the way.
>>
>>   include/my_bitmap.h@stripped, 2007-12-07 14:45:54+02:00, aelkin@stripped
> +16 -0
>>     basic operations with bits of an integer type are added.
>>
>>   mysql-test/extra/rpl_tests/rpl_foreign_key.test@stripped, 2007-12-07 14:45:54+02:00,
> aelkin@stripped +31 -0
>>     regression test for bug#32468
>>
>>   mysql-test/extra/rpl_tests/rpl_row_basic.test@stripped, 2007-12-07 14:45:55+02:00,
> aelkin@stripped +14 -0
>>     changes due to bug#31552/31609 idempotency is not default any longer
>>
>>   mysql-test/extra/rpl_tests/rpl_row_tabledefs.test@stripped, 2007-12-07
> 14:45:55+02:00, aelkin@stripped +7 -0
>>     changes due to bug#31552/31609 idempotency is not default any longer
>>
>>   mysql-test/suite/rpl/r/rpl_foreign_key_innodb.result@stripped, 2007-12-07
> 14:45:55+02:00, aelkin@stripped +13 -0
>>     results changed
>>
>>   mysql-test/suite/rpl/r/rpl_idempotency.result@stripped, 2007-12-07 14:45:55+02:00,
> aelkin@stripped +155 -0
>>     results changed
>>
>>   mysql-test/suite/rpl/r/rpl_row_basic_11bugs.result@stripped, 2007-12-07
> 14:45:55+02:00, aelkin@stripped +2 -0
>>     results changed
>>
>>   mysql-test/suite/rpl/r/rpl_row_basic_2myisam.result@stripped, 2007-12-07
> 14:45:55+02:00, aelkin@stripped +4 -0
>>     results changed
>>
>>   mysql-test/suite/rpl/r/rpl_row_basic_3innodb.result@stripped, 2007-12-07
> 14:45:56+02:00, aelkin@stripped +4 -0
>>     results changed
>>
>>   mysql-test/suite/rpl/r/rpl_row_mystery22.result@stripped, 2007-12-07 14:45:56+02:00,
> aelkin@stripped +2 -0
>>     results changed
>>
>>   mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result@stripped, 2007-12-07
> 14:45:56+02:00, aelkin@stripped +4 -2
>>     results changed
>>
>>   mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result@stripped, 2007-12-07
> 14:45:56+02:00, aelkin@stripped +4 -2
>>     results changed
>>
>>   mysql-test/suite/rpl/r/rpl_temporary_errors.result@stripped, 2007-12-07
> 14:45:56+02:00, aelkin@stripped +2 -0
>>     results changed
>>
>>   mysql-test/suite/rpl/t/rpl_idempotency-master.opt@stripped, 2007-12-07
> 14:46:00+02:00, aelkin@stripped +2 -0
>>     innodb is necessary
>>
>>   mysql-test/suite/rpl/t/rpl_idempotency-master.opt@stripped, 2007-12-07
> 14:46:00+02:00, aelkin@stripped +0 -0
>>
>>   mysql-test/suite/rpl/t/rpl_idempotency-slave.opt@stripped, 2007-12-07
> 14:46:00+02:00, aelkin@stripped +2 -0
>>     innodb is necessary, as well as the tests start with non-default
>>     IDEMPOTENT slave execution mode.
>>     
>>
>>   mysql-test/suite/rpl/t/rpl_idempotency-slave.opt@stripped, 2007-12-07
> 14:46:00+02:00, aelkin@stripped +0 -0
>>
>>   mysql-test/suite/rpl/t/rpl_idempotency.test@stripped, 2007-12-07 14:45:56+02:00,
> aelkin@stripped +332 -0
>>     extenstions to the test providing testing of complements to the
>>     idempotent error set and checking how slave halts when it faces an error
>>     from the list when the mode is STRICT.
>>
>>   mysql-test/suite/rpl/t/rpl_row_basic_11bugs.test@stripped, 2007-12-07
> 14:45:57+02:00, aelkin@stripped +5 -0
>>     changes due to bug#31552/31609 idempotency is not default any longer.
>>
>>   mysql-test/suite/rpl/t/rpl_row_mystery22.test@stripped, 2007-12-07 14:45:57+02:00,
> aelkin@stripped +7 -0
>>     changes due to bug#31552/31609 idempotency is not default any longer
>>
>>   mysql-test/suite/rpl/t/rpl_temporary_errors.test@stripped, 2007-12-07
> 14:45:57+02:00, aelkin@stripped +4 -0
>>     changes due to bug#31552/31609 idempotency is not default any longer
>>
>>   mysql-test/suite/rpl_ndb/r/rpl_row_basic_7ndb.result@stripped, 2007-12-07
> 14:45:57+02:00, aelkin@stripped +4 -0
>>     results changed
>>
>>   sql/log_event.cc@stripped, 2007-12-07 14:45:57+02:00, aelkin@stripped +146
> -91
>>     the fix for bug#32468 delete rows event on a table with foreign key
> constraint fails
>>     ensures the flags are set at proper time so that their values will be caught
>>     by innodb.
>>     reseting the flags is done along the common error and errorless execution
>>     path.
>>     The list of idempotent error is extended with foreign keys related items.
>>     NDB engine write events are designed with the replace sematics in mind.
>>     Therefore the corrsponding ndb handler's flag are (re)set regardless of
>>     the slave's execution mode.
>>     Rows_log_event::write_row() starts using the bool replace argument as its
>>     caller sets it depending on the event's execution mode.
>>
>>   sql/log_event.h@stripped, 2007-12-07 14:45:58+02:00, aelkin@stripped +6
> -0
>>     adding a new member to hold the slave's mode during execution of the event.
>>
>>   sql/mysql_priv.h@stripped, 2007-12-07 14:45:58+02:00, aelkin@stripped +1
> -0
>>     changes to link the command line option with the new global sys var.
>>
>>   sql/mysqld.cc@stripped, 2007-12-07 14:45:58+02:00, aelkin@stripped +17 -2
>>     introduction of the new command line option.
>>     providing its initialization to a default.
>>     changes to link the command line option with the new global sys var.
>>
>>   sql/rpl_rli.cc@stripped, 2007-12-07 14:45:59+02:00, aelkin@stripped +5 -0
>>     rli post-event-execution cleanup restores the default bits.
>>
>>   sql/set_var.cc@stripped, 2007-12-07 14:45:59+02:00, aelkin@stripped +99
> -0
>>     The new "standard" sys_var_set class' and the new global system var related
>>     declarations and definitions.
>>     fix_slave_exec_mode() is used as with the update method of a new class so
>>     as at time of the command line arguments parsing.
>>
>>   sql/set_var.h@stripped, 2007-12-07 14:45:59+02:00, aelkin@stripped +39 -1
>>     new declarations. The class for the new global sys var is based on
>>     yet another new "standard" one.
>>
>>   sql/share/errmsg.txt@stripped, 2007-12-07 14:46:00+02:00, aelkin@stripped
> +2 -0
>>     slave_exec_mode setting error;
>>     slave inconsistency error which may be not an error when the intention
>>     is "idempotent". I.e consisting of row-based events binlog is being
>>     applied for the 2nd (more) time.
>>
>>   sql/sql_class.h@stripped, 2007-12-07 14:45:59+02:00, aelkin@stripped +3
> -0
>>     The names for the bits of the new sever slave_exec_mode_options.
>>
>> diff -Nrup a/include/my_bitmap.h b/include/my_bitmap.h
>> --- a/include/my_bitmap.h	2007-07-25 16:29:28 +03:00
>> +++ b/include/my_bitmap.h	2007-12-07 14:45:54 +02:00
>> @@ -159,6 +159,22 @@ static inline my_bool bitmap_cmp(const M
>>  #define bitmap_set_all(MAP) \
>>    (memset((MAP)->bitmap, 0xFF, 4*no_words_in_map((MAP))))
>>  +/**
>> +   check, set and clear a bit of interest of an integer.
>> +
>> +   If the bit is out of range @retval -1. Otherwise
>> +   bit_is_set   @return 0 or 1 reflecting the bit is set or not;
>> +   bit_do_set   @return 1 (bit is set 1)
>> +   bit_do_clear @return 0 (bit is cleared to 0)
>> +*/
>>   

>
> This comment is close to unreadable in Doxygen. Check the Doxygen
> output later, and fix the comment. Not necessary to do now, however.
>

Okay. I'd love to improve them but I really got a hard time trying to doxygenize.
Using intructions on https://inside.mysql.com/wiki/Doxygen
did not help me much.

Do you know by chance some other source of how to do it?


>> +
>> +#define bit_is_set(I,B)   (sizeof(I) * CHAR_BIT > (B) ?                 \
>> +                           (((I) & (ULL(1) << (B))) == 0 ? 0 : 1) :
> -1)
>> +#define bit_do_set(I,B)   (sizeof(I) * CHAR_BIT > (B) ?         \
>> +                           ((I) |= (ULL(1) << (B)), 1) : -1)
>> +#define bit_do_clear(I,B) (sizeof(I) * CHAR_BIT > (B) ?         \
>> +                           ((I) &= ~(ULL(1) << (B)), 0) : -1)
>> +
>>  #ifdef	__cplusplus
>>  }
>>  #endif
>> diff -Nrup a/mysql-test/extra/rpl_tests/rpl_foreign_key.test
> b/mysql-test/extra/rpl_tests/rpl_foreign_key.test
>> --- a/mysql-test/extra/rpl_tests/rpl_foreign_key.test	2007-06-06 20:48:52 +03:00
>> +++ b/mysql-test/extra/rpl_tests/rpl_foreign_key.test	2007-12-07 14:45:54 +02:00
>> @@ -32,3 +32,34 @@ SET FOREIGN_KEY_CHECKS=0;
>>  DROP TABLE IF EXISTS t1,t2,t3;
>>  SET FOREIGN_KEY_CHECKS=1;
>>  sync_slave_with_master;
>> +
>> +#
>> +# Bug #32468 delete rows event on a table with foreign key constraint fails
>> +#
>> +
>> +connection master;
>> +
>> +eval create table t1 (b int primary key) engine = $engine_type;
>> +eval create table t2 (a int primary key, b int, foreign key (b) references
> t1(b))
>> +       engine = $engine_type;
>> +
>> +insert into t1 set b=1;
>> +insert into t2 set a=1, b=1;
>> +
>> +set foreign_key_checks=0;
>> +set @@session.binlog_format=row;
>> +delete from t1;
>> +
>> +--echo must sync w/o a problem (could not with the buggy code)
>> +sync_slave_with_master;
>> +select count(*) from t1 /* must be zero */;
>> +
>> +
>> +# cleanup for bug#32468
>> +
>> +connection master;
>> +drop table t2,t1;
>> +
>> +sync_slave_with_master;
>> +
>> +
>> diff -Nrup a/mysql-test/extra/rpl_tests/rpl_row_basic.test
> b/mysql-test/extra/rpl_tests/rpl_row_basic.test
>> --- a/mysql-test/extra/rpl_tests/rpl_row_basic.test	2007-06-19 00:51:07 +03:00
>> +++ b/mysql-test/extra/rpl_tests/rpl_row_basic.test	2007-12-07 14:45:55 +02:00
>> @@ -174,11 +174,18 @@ sync_slave_with_master;
>>  INSERT INTO t7 VALUES (1,3), (2,6), (3,9);
>>  SELECT * FROM t7 ORDER BY C1;
>>  +# since bug#31552/31609 idempotency is not default any longer. In
>> order
>> +# the preceeding test INSERT INTO t7 to pass the mode is switched
>> +# temprorarily
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>> +
>>  connection master;
>>  --echo --- on master: new values inserted ---
>>  INSERT INTO t7 VALUES (1,2), (2,4), (3,6);
>>  SELECT * FROM t7 ORDER BY C1;
>>  sync_slave_with_master;
>> +
>> +set @@global.slave_exec_mode= default;
>>  --echo --- on slave: old values should be overwritten by replicated values ---
>>  SELECT * FROM t7 ORDER BY C1;
>>  @@ -206,12 +213,19 @@ SELECT * FROM t8 ORDER BY a;
>>  INSERT INTO t8 VALUES (1,2,3), (2,4,6), (3,6,9);
>>  SELECT * FROM t8 ORDER BY a;
>>  +# since bug#31552/31609 idempotency is not default any longer. In
>> order
>> +# the preceeding test INSERT INTO t8 to pass the mode is switched
>> +# temprorarily
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>> +
>>  connection master;
>>  --echo --- on master ---
>>  # We insert a row that will cause conflict on the primary key but not
>>  # on the other keys.
>>  INSERT INTO t8 VALUES (2,4,8);
>>  sync_slave_with_master;
>> +set @@global.slave_exec_mode= default;
>> +
>>  --echo --- on slave ---
>>  SELECT * FROM t8 ORDER BY a;
>>  diff -Nrup a/mysql-test/extra/rpl_tests/rpl_row_tabledefs.test
>> b/mysql-test/extra/rpl_tests/rpl_row_tabledefs.test
>> --- a/mysql-test/extra/rpl_tests/rpl_row_tabledefs.test	2007-08-21 15:32:48
> +03:00
>> +++ b/mysql-test/extra/rpl_tests/rpl_row_tabledefs.test	2007-12-07 14:45:55
> +02:00
>> @@ -69,6 +69,11 @@ ALTER TABLE t8 ADD e1 INT NOT NULL DEFAU
>>           # Insert some values for tables on slave side. These
>> should not be
>>  # modified when the row from the master is applied.
>> +# since bug#31552/31609 idempotency is not default any longer. In order
>> +# the following INSERTs to pass the mode is switched temprorarily
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>> +
>> +# so the inserts are going to be overriden
>>  INSERT INTO t1_int  VALUES (2, 4, 4711);
>>  INSERT INTO t1_char VALUES (2, 4, 'Foo is a bar');
>>  INSERT INTO t1_bit  VALUES (2, 4, b'101', b'11100', b'01');
>> @@ -86,6 +91,8 @@ SELECT * FROM t1_bit ORDER BY a;
>>  SELECT * FROM t1_char ORDER BY a;
>>  --echo **** On Slave ****
>>  sync_slave_with_master;
>> +set @@global.slave_exec_mode= default;
>> +
>>  SELECT a,b,x FROM t1_int ORDER BY a;
>>  SELECT a,b,HEX(x),HEX(y),HEX(z) FROM t1_bit ORDER BY a;
>>  SELECT a,b,x FROM t1_char ORDER BY a;
>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_foreign_key_innodb.result
> b/mysql-test/suite/rpl/r/rpl_foreign_key_innodb.result
>> --- a/mysql-test/suite/rpl/r/rpl_foreign_key_innodb.result	2007-06-27 15:27:28
> +03:00
>> +++ b/mysql-test/suite/rpl/r/rpl_foreign_key_innodb.result	2007-12-07 14:45:55
> +02:00
>> @@ -40,3 +40,16 @@ Got one of the listed errors
>>  SET FOREIGN_KEY_CHECKS=0;
>>  DROP TABLE IF EXISTS t1,t2,t3;
>>  SET FOREIGN_KEY_CHECKS=1;
>> +create table t1 (b int primary key) engine = INNODB;
>> +create table t2 (a int primary key, b int, foreign key (b) references t1(b))
>> +engine = INNODB;
>> +insert into t1 set b=1;
>> +insert into t2 set a=1, b=1;
>> +set foreign_key_checks=0;
>> +set @@session.binlog_format=row;
>> +delete from t1;
>> +must sync w/o a problem (could not with the buggy code)
>> +select count(*) from t1 /* must be zero */;
>> +count(*)
>> +0
>> +drop table t2,t1;
>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_idempotency.result
> b/mysql-test/suite/rpl/r/rpl_idempotency.result
>> --- a/mysql-test/suite/rpl/r/rpl_idempotency.result	2007-10-30 22:17:14 +02:00
>> +++ b/mysql-test/suite/rpl/r/rpl_idempotency.result	2007-12-07 14:45:55 +02:00
>> @@ -69,3 +69,158 @@ a
>>  Last_SQL_Error
>>  0
>>  DROP TABLE t1, t2;
>> +select @@global.slave_exec_mode /* must be IDEMPOTENT */;
>> +@@global.slave_exec_mode
>> +IDEMPOTENT
>> +create table ti1 (b int primary key) engine = innodb;
>> +create table ti2 (a int primary key, b int, foreign key (b) references ti1(b))
>> +engine = innodb;
>> +set foreign_key_checks=1 /* ensure the check */;
>> +insert into ti1 values (1),(2),(3);
>> +insert into ti2 set a=2, b=2;
>> +select * from ti1 order by b /* must be (1),(2),(3) */;
>> +b
>> +1
>> +2
>> +3
>> +insert into ti2 set a=1, b=1;
>> +select * from ti2 order by b /* must be (1,1) (2,2) */;
>> +a	b
>> +1	1
>> +2	2
>> +set @save_binlog_format= @@session.binlog_format;
>> +set @@session.binlog_format= row;
>> +delete from ti1 where b=1;
>> +select * from ti1 order by b /* must be (2),(3) */;
>> +b
>> +2
>> +3
>> +select * from ti1 order by b /* must stays as were on master (1),(2),(3) */;
>> +b
>> +1
>> +2
>> +3
>> +delete from ti1 where b=3;
>> +insert into ti2 set a=3, b=3;
>> +select * from ti2 order by b /* must be (1,1),(2,2) - not inserted */;
>> +a	b
>> +1	1
>> +2	2
>> +set global slave_exec_mode='IDEMPOTENT';
>> +set global slave_exec_mode='STRICT';
>> +set global slave_exec_mode='IDEMPOTENT,STRICT';
>> +ERROR HY000: Ambiguous slave modes combination. +select
>> @@global.slave_exec_mode /* must be STRICT */;
>> +@@global.slave_exec_mode
>> +STRICT
>> +*** foreign keys errors as above now forces to stop
>> +set foreign_key_checks=0;
>> +drop table ti2, ti1;
>> +create table ti1 (b int primary key) engine = innodb;
>> +create table ti2 (a int primary key, b int, foreign key (b) references ti1(b))
>> +engine = innodb;
>> +set foreign_key_checks=1 /* ensure the check */;
>> +insert into ti1 values (1),(2),(3);
>> +insert into ti2 set a=2, b=2;
>> +select * from ti1 order by b /* must be (1),(2),(3) */;
>> +b
>> +1
>> +2
>> +3
>> +*** conspire future problem
>> +insert into ti2 set a=1, b=1;
>> +select * from ti2 order by b /* must be (1,1) (2,2) */;
>> +a	b
>> +1	1
>> +2	2
>> +delete from ti1 where b=1 /* offending delete event */;
>> +select * from ti1 order by b /* must be (2),(3) */;
>> +b
>> +2
>> +3
>> +*** slave must stop
>> +Last_SQL_Error
>> +0
>> +select * from ti1 order by b /* must be (1),(2),(3) - not deleted */;
>> +b
>> +1
>> +2
>> +3
>> +set foreign_key_checks= 0;
>> +delete from ti2 where b=1;
>> +set foreign_key_checks= 1;
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +set global slave_exec_mode='STRICT';
>> +*** conspire the following insert failure
>> +*** conspire future problem
>> +delete from ti1 where b=3;
>> +insert into ti2 set a=3, b=3 /* offending write event */;
>> +*** slave must stop
>> +Last_SQL_Error
>> +1452
>> +select * from ti2 order by b /* must be (2,2) */;
>> +a	b
>> +2	2
>> +set foreign_key_checks= 0;
>> +insert into ti1 set b=3;
>> +set foreign_key_checks= 1;
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +set global slave_exec_mode='STRICT';
>> +select * from ti2 order by b /* must be (2,2),(3,3) */;
>> +a	b
>> +2	2
>> +3	3
>> +*** other errors
>> +*** conspiring query
>> +insert into ti1 set b=1;
>> +insert into ti1 set b=1 /* offending write event */;
>> +*** slave must stop
>> +Last_SQL_Error
>> +1062
>> +set foreign_key_checks= 0;
>> +delete from ti1 where b=1;
>> +set foreign_key_checks= 1;
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +set global slave_exec_mode='STRICT';
>> +CREATE TABLE t1 (a INT PRIMARY KEY);
>> +CREATE TABLE t2 (a INT);
>> +INSERT INTO t1 VALUES (-1),(-2),(-3);
>> +INSERT INTO t2 VALUES (-1),(-2),(-3);
>> +DELETE FROM t1 WHERE a = -2;
>> +DELETE FROM t2 WHERE a = -2;
>> +DELETE FROM t1 WHERE a = -2;
>> +*** slave must stop
>> +Last_SQL_Error
>> +1032
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +set global slave_exec_mode='STRICT';
>> +DELETE FROM t2 WHERE a = -2;
>> +*** slave must stop
>> +Last_SQL_Error
>> +0
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +set global slave_exec_mode='STRICT';
>> +UPDATE t1 SET a = 1 WHERE a = -1;
>> +UPDATE t2 SET a = 1 WHERE a = -1;
>> +UPDATE t1 SET a = 1 WHERE a = -1;
>> +*** slave must stop
>> +Last_SQL_Error
>> +1032
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +set global slave_exec_mode='STRICT';
>> +UPDATE t2 SET a = 1 WHERE a = -1;
>> +*** slave must stop
>> +Last_SQL_Error
>> +0
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +set global slave_exec_mode='STRICT';
>> +set @@session.binlog_format= @save_binlog_format;
>> +drop table t1,t2,ti2,ti1;
>> +*** end of tests
>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_row_basic_11bugs.result
> b/mysql-test/suite/rpl/r/rpl_row_basic_11bugs.result
>> --- a/mysql-test/suite/rpl/r/rpl_row_basic_11bugs.result	2007-10-24 17:02:31
> +03:00
>> +++ b/mysql-test/suite/rpl/r/rpl_row_basic_11bugs.result	2007-12-07 14:45:55
> +02:00
>> @@ -257,6 +257,7 @@ SELECT * FROM t1 ORDER BY a;
>>  a	b
>>  2	master,slave
>>  5	slave
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  **** On Master ****
>>  UPDATE t1 SET a = 5, b = 'master' WHERE a = 1;
>>  SELECT * FROM t1 ORDER BY a;
>> @@ -264,6 +265,7 @@ a	b
>>  2	master,slave
>>  5	master
>>  **** On Slave ****
>> +set @@global.slave_exec_mode= default;
>>  Last_SQL_Error
>>   SELECT * FROM t1 ORDER BY a;
>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_row_basic_2myisam.result
> b/mysql-test/suite/rpl/r/rpl_row_basic_2myisam.result
>> --- a/mysql-test/suite/rpl/r/rpl_row_basic_2myisam.result	2007-06-27 15:27:28
> +03:00
>> +++ b/mysql-test/suite/rpl/r/rpl_row_basic_2myisam.result	2007-12-07 14:45:55
> +02:00
>> @@ -370,6 +370,7 @@ C1	C2
>>  1	3
>>  2	6
>>  3	9
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  --- on master: new values inserted ---
>>  INSERT INTO t7 VALUES (1,2), (2,4), (3,6);
>>  SELECT * FROM t7 ORDER BY C1;
>> @@ -377,6 +378,7 @@ C1	C2
>>  1	2
>>  2	4
>>  3	6
>> +set @@global.slave_exec_mode= 'STRICT';
>>  --- on slave: old values should be overwritten by replicated values ---
>>  SELECT * FROM t7 ORDER BY C1;
>>  C1	C2
>> @@ -406,8 +408,10 @@ a	b	c
>>  2	4	6
>>  3	6	9
>>  99	99	99
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  --- on master ---
>>  INSERT INTO t8 VALUES (2,4,8);
>> +set @@global.slave_exec_mode= default;
>>  --- on slave ---
>>  SELECT * FROM t8 ORDER BY a;
>>  a	b	c
>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_row_basic_3innodb.result
> b/mysql-test/suite/rpl/r/rpl_row_basic_3innodb.result
>> --- a/mysql-test/suite/rpl/r/rpl_row_basic_3innodb.result	2007-06-27 15:27:32
> +03:00
>> +++ b/mysql-test/suite/rpl/r/rpl_row_basic_3innodb.result	2007-12-07 14:45:56
> +02:00
>> @@ -370,6 +370,7 @@ C1	C2
>>  1	3
>>  2	6
>>  3	9
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  --- on master: new values inserted ---
>>  INSERT INTO t7 VALUES (1,2), (2,4), (3,6);
>>  SELECT * FROM t7 ORDER BY C1;
>> @@ -377,6 +378,7 @@ C1	C2
>>  1	2
>>  2	4
>>  3	6
>> +set @@global.slave_exec_mode= 'STRICT';
>>  --- on slave: old values should be overwritten by replicated values ---
>>  SELECT * FROM t7 ORDER BY C1;
>>  C1	C2
>> @@ -406,8 +408,10 @@ a	b	c
>>  2	4	6
>>  3	6	9
>>  99	99	99
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  --- on master ---
>>  INSERT INTO t8 VALUES (2,4,8);
>> +set @@global.slave_exec_mode= default;
>>  --- on slave ---
>>  SELECT * FROM t8 ORDER BY a;
>>  a	b	c
>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_row_mystery22.result
> b/mysql-test/suite/rpl/r/rpl_row_mystery22.result
>> --- a/mysql-test/suite/rpl/r/rpl_row_mystery22.result	2007-06-27 15:27:32 +03:00
>> +++ b/mysql-test/suite/rpl/r/rpl_row_mystery22.result	2007-12-07 14:45:56 +02:00
>> @@ -5,6 +5,7 @@ reset slave;
>>  drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>  start slave;
>>  create table t1(n int auto_increment primary key, s char(10));
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  insert into t1 values (2,'old');
>>  insert into t1 values(NULL,'new');
>>  insert into t1 values(NULL,'new');
>> @@ -28,3 +29,4 @@ n	s
>>  1	new
>>  3	new
>>  drop table t1;
>> +set @@global.slave_exec_mode= default;
>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result
> b/mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result
>> --- a/mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result	2007-09-25 18:44:47
> +03:00
>> +++ b/mysql-test/suite/rpl/r/rpl_row_tabledefs_2myisam.result	2007-12-07 14:45:56
> +02:00
>> @@ -37,6 +37,7 @@ ALTER TABLE t8 ADD e1 INT NOT NULL DEFAU
>>  ADD e3 INT NOT NULL DEFAULT 0, ADD e4 INT NOT NULL DEFAULT 0,
>>  ADD e5 INT NOT NULL DEFAULT 0, ADD e6 INT NOT NULL DEFAULT 0,
>>  ADD e7 INT NOT NULL DEFAULT 0, ADD e8 INT NOT NULL DEFAULT 0;
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  INSERT INTO t1_int  VALUES (2, 4, 4711);
>>  INSERT INTO t1_char VALUES (2, 4, 'Foo is a bar');
>>  INSERT INTO t1_bit  VALUES (2, 4, b'101', b'11100', b'01');
>> @@ -60,6 +61,7 @@ a	b
>>  1	2
>>  2	5
>>  **** On Slave ****
>> +set @@global.slave_exec_mode= default;
>>  SELECT a,b,x FROM t1_int ORDER BY a;
>>  a	b	x
>>  1	2	42
>> @@ -123,7 +125,7 @@ Replicate_Ignore_Table	
>>  Replicate_Wild_Do_Table	
>>  Replicate_Wild_Ignore_Table	
>>  Last_Errno	1364
>> -Last_Error	Error in Write_rows event: error during transaction
>> execution on table test.t1_nodef. +Last_Error	Could not execute
>> Write_rows event on table test.t1_nodef; handler error <unknown>;
>> the event's master log and end_log_pos master-bin.000001 2674
>>  Skip_Counter	0
>>  Exec_Master_Log_Pos	#
>>  Relay_Log_Space	#
>> @@ -141,7 +143,7 @@ Master_SSL_Verify_Server_Cert	No
>>  Last_IO_Errno	0
>>  Last_IO_Error	
>>  Last_SQL_Errno	1364
>> -Last_SQL_Error	Error in Write_rows event: error during
>> transaction execution on table test.t1_nodef. +Last_SQL_Error	Could
>> not execute Write_rows event on table test.t1_nodef; handler error
>> <unknown>; the event's master log and end_log_pos master-bin.000001
>> 2674
>>   
> Just a suggestion for a more scrapable message (you know, applications actually try
> to make sense of the error messages).
>
>    "Could not execute Write_rows event on table test.t1_nodef; Handler error:
> <unknown>; Master log file: master-bin.000001; end_log_pos: 2674"
>

you have a point. Changed to this (although a comma as the separator
between log file and end_log_pos :).

>      
>
>>  SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
>>  START SLAVE;
>>  INSERT INTO t9 VALUES (2);
>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result
> b/mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result
>> --- a/mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result	2007-09-25 18:44:47
> +03:00
>> +++ b/mysql-test/suite/rpl/r/rpl_row_tabledefs_3innodb.result	2007-12-07 14:45:56
> +02:00
>> @@ -37,6 +37,7 @@ ALTER TABLE t8 ADD e1 INT NOT NULL DEFAU
>>  ADD e3 INT NOT NULL DEFAULT 0, ADD e4 INT NOT NULL DEFAULT 0,
>>  ADD e5 INT NOT NULL DEFAULT 0, ADD e6 INT NOT NULL DEFAULT 0,
>>  ADD e7 INT NOT NULL DEFAULT 0, ADD e8 INT NOT NULL DEFAULT 0;
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  INSERT INTO t1_int  VALUES (2, 4, 4711);
>>  INSERT INTO t1_char VALUES (2, 4, 'Foo is a bar');
>>  INSERT INTO t1_bit  VALUES (2, 4, b'101', b'11100', b'01');
>> @@ -60,6 +61,7 @@ a	b
>>  1	2
>>  2	5
>>  **** On Slave ****
>> +set @@global.slave_exec_mode= default;
>>  SELECT a,b,x FROM t1_int ORDER BY a;
>>  a	b	x
>>  1	2	42
>> @@ -123,7 +125,7 @@ Replicate_Ignore_Table	
>>  Replicate_Wild_Do_Table	
>>  Replicate_Wild_Ignore_Table	
>>  Last_Errno	1364
>> -Last_Error	Error in Write_rows event: error during transaction
>> execution on table test.t1_nodef. +Last_Error	Could not execute
>> Write_rows event on table test.t1_nodef; handler error <unknown>;
>> the event's master log and end_log_pos master-bin.000001 2944
>>  Skip_Counter	0
>>  Exec_Master_Log_Pos	#
>>  Relay_Log_Space	#
>> @@ -141,7 +143,7 @@ Master_SSL_Verify_Server_Cert	No
>>  Last_IO_Errno	0
>>  Last_IO_Error	
>>  Last_SQL_Errno	1364
>> -Last_SQL_Error	Error in Write_rows event: error during
>> transaction execution on table test.t1_nodef. +Last_SQL_Error	Could
>> not execute Write_rows event on table test.t1_nodef; handler error
>> <unknown>; the event's master log and end_log_pos master-bin.000001
>> 2944
>>  SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
>>  START SLAVE;
>>  INSERT INTO t9 VALUES (2);
>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_temporary_errors.result
> b/mysql-test/suite/rpl/r/rpl_temporary_errors.result
>> --- a/mysql-test/suite/rpl/r/rpl_temporary_errors.result	2007-10-24 17:02:32
> +03:00
>> +++ b/mysql-test/suite/rpl/r/rpl_temporary_errors.result	2007-12-07 14:45:56
> +02:00
>> @@ -12,6 +12,7 @@ INSERT INTO t1 VALUES (1,1), (2,2), (3,3
>>  SHOW STATUS LIKE 'Slave_retried_transactions';
>>  Variable_name	Value
>>  Slave_retried_transactions	0
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  UPDATE t1 SET a = 5, b = 47 WHERE a = 1;
>>  SELECT * FROM t1;
>>  a	b
>> @@ -28,6 +29,7 @@ a	b
>>  3	3
>>  4	4
>>  **** On Slave ****
>> +set @@global.slave_exec_mode= default;
>>  SHOW STATUS LIKE 'Slave_retried_transactions';
>>  Variable_name	Value
>>  Slave_retried_transactions	0
>> diff -Nrup a/mysql-test/suite/rpl/t/rpl_idempotency-master.opt
> b/mysql-test/suite/rpl/t/rpl_idempotency-master.opt
>> --- /dev/null	Wed Dec 31 16:00:00 196900
>> +++ b/mysql-test/suite/rpl/t/rpl_idempotency-master.opt	2007-12-07 14:46:00
> +02:00
>> @@ -0,0 +1,2 @@
>> +--innodb
>> +
>> diff -Nrup a/mysql-test/suite/rpl/t/rpl_idempotency-slave.opt
> b/mysql-test/suite/rpl/t/rpl_idempotency-slave.opt
>> --- /dev/null	Wed Dec 31 16:00:00 196900
>> +++ b/mysql-test/suite/rpl/t/rpl_idempotency-slave.opt	2007-12-07 14:46:00
> +02:00
>> @@ -0,0 +1,2 @@
>> +--slave-exec-mode=IDEMPOTENT --innodb
>> +
>> diff -Nrup a/mysql-test/suite/rpl/t/rpl_idempotency.test
> b/mysql-test/suite/rpl/t/rpl_idempotency.test
>> --- a/mysql-test/suite/rpl/t/rpl_idempotency.test	2007-10-30 22:17:14 +02:00
>> +++ b/mysql-test/suite/rpl/t/rpl_idempotency.test	2007-12-07 14:45:56 +02:00
>> @@ -77,3 +77,335 @@ enable_query_log;
>>  connection master;
>>  DROP TABLE t1, t2;
>>  sync_slave_with_master;
>> +
>> +# bug#31609 Not all RBR slave errors reported as errors
>> +# bug#31552 Replication breaks when deleting rows from out-of-sync table
>> +#           without PK
>> +
>> +#
>> +# Idempotent applying is not default any longer.
>> +# The default for slave-exec-mode option and server
>> +# variable slave_exec_mode  is 'STRICT'.
>> +# When 'STRICT' mode is set, the slave SQL thread will stop whenever
>> +# the row to change is not found. In 'IDEMPOTENT' mode, the SQL thread
>> +# will continue running and apply the row - replace if it's Write_rows event -
>> +# or skip to the next event.
>> +
>> +# the previous part of the tests was with IDEMPOTENT slave's mode.
>> +
>> +
>> +#
>> +# Other than above idempotent errors dealing with foreign keys constraint
>> +#
>> +
>> +select @@global.slave_exec_mode /* must be IDEMPOTENT */;
>> +
>> +connection master;
>> +
>> +create table ti1 (b int primary key) engine = innodb;
>> +create table ti2 (a int primary key, b int, foreign key (b) references ti1(b))
>> +     engine = innodb;
>> +set foreign_key_checks=1 /* ensure the check */;
>> +
>> +insert into ti1 values (1),(2),(3);
>> +insert into ti2 set a=2, b=2;
>> +
>> +sync_slave_with_master;
>> +
>> +#connection slave;
>> +select * from ti1 order by b /* must be (1),(2),(3) */;
>> +insert into ti2 set a=1, b=1;
>> +select * from ti2 order by b /* must be (1,1) (2,2) */;
>> +
>> +connection master;
>> +
>> +# from now on checking rbr specific idempotent errors
>> +set @save_binlog_format= @@session.binlog_format;
>> +set @@session.binlog_format= row;
>> +delete from ti1 where b=1;
>> +
>> +select * from ti1 order by b /* must be (2),(3) */;
>> +
>> +# slave must catch up (expect some warnings in error.log)
>> +sync_slave_with_master;
>> +
>> +#connection slave;
>> +select * from ti1 order by b /* must stays as were on master (1),(2),(3) */;
>> +
>> +delete from ti1 where b=3;
>> +
>> +connection master;
>> +insert into ti2 set a=3, b=3;
>> +
>> +# slave must catch up (expect some warnings in error.log)
>> +sync_slave_with_master;
>> +
>> +#connection slave;
>> +select * from ti2 order by b /* must be (1,1),(2,2) - not inserted */;
>> +
>> +
>> +#
>> +# Checking the new global sys variable
>> +#
>> +
>> +connection slave;
>> +
>> +set global slave_exec_mode='IDEMPOTENT';
>> +set global slave_exec_mode='STRICT';
>> +
>> +# checking mutual exclusion for the options
>> +--error ER_SLAVE_AMBIGOUS_EXEC_MODE
>> +set global slave_exec_mode='IDEMPOTENT,STRICT';
>> +
>> +select @@global.slave_exec_mode /* must be STRICT */;
>> +
>> +#
>> +# Checking stops.
>> +# In the following sections strict slave sql thread is going to
>> +# stop when faces an idempotent error. In order to proceed
>> +# the mode is temporarily switched to indempotent.
>> +#
>> +
>> +#
>> +--echo *** foreign keys errors as above now forces to stop
>> +#
>> +
>> +connection master;
>> +
>> +set foreign_key_checks=0;
>> +drop table ti2, ti1;
>> +
>> +create table ti1 (b int primary key) engine = innodb;
>> +create table ti2 (a int primary key, b int, foreign key (b) references ti1(b))
>> +     engine = innodb;
>> +set foreign_key_checks=1 /* ensure the check */;
>> +
>> +insert into ti1 values (1),(2),(3);
>> +insert into ti2 set a=2, b=2;
>> +
>> +sync_slave_with_master;
>> +
>> +#connection slave;
>> +select * from ti1 order by b /* must be (1),(2),(3) */;
>> +--echo *** conspire future problem
>> +insert into ti2 set a=1, b=1;
>> +select * from ti2 order by b /* must be (1,1) (2,2) */;
>> +
>> +connection master;
>> +
>> +delete from ti1 where b=1 /* offending delete event */;
>> +select * from ti1 order by b /* must be (2),(3) */;
>> +
>> +# foreign key: row is referenced
>> +
>> +--echo *** slave must stop
>> +source include/wait_for_slave_sql_to_stop.inc;
>> +
>> +connection slave;
>> +
>> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
>> +disable_query_log;
>> +eval SELECT "$last_error" AS Last_SQL_Error;
>> +enable_query_log;
>> +
>> +select * from ti1 order by b /* must be (1),(2),(3) - not deleted */;
>> +set foreign_key_checks= 0;
>> +delete from ti2 where b=1;
>> +set foreign_key_checks= 1;
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +connection master;
>> +sync_slave_with_master;
>> +#connection slave;
>> +set global slave_exec_mode='STRICT';
>> +
>> +connection master;
>> +
>> +sync_slave_with_master;
>> +
>> +#connection slave;
>> +--echo *** conspire the following insert failure
>> +# foreign key: no referenced row
>> +
>> +--echo *** conspire future problem
>> +delete from ti1 where b=3;
>> +
>> +connection master;
>> +insert into ti2 set a=3, b=3 /* offending write event */;
>> +--echo *** slave must stop
>> +
>> +source include/wait_for_slave_sql_to_stop.inc;
>> +
>> +connection slave;
>> +
>> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
>> +disable_query_log;
>> +eval SELECT "$last_error" AS Last_SQL_Error;
>> +enable_query_log;
>> +
>> +select * from ti2 order by b /* must be (2,2) */;
>> +set foreign_key_checks= 0;
>> +insert into ti1 set b=3;
>> +set foreign_key_checks= 1;
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +connection master;
>> +sync_slave_with_master;
>> +#connection slave;
>> +set global slave_exec_mode='STRICT';
>> +
>> +connection master;
>> +
>> +sync_slave_with_master;
>> +
>> +select * from ti2 order by b /* must be (2,2),(3,3) */;
>> +
>> +# +--echo *** other errors
>> +# +
>> +# dup key insert
>> +
>> +#connection slave;
>> +--echo *** conspiring query
>> +insert into ti1 set b=1;
>> +
>> +connection master;
>> +insert into ti1 set b=1 /* offending write event */;
>> +
>> +--echo *** slave must stop
>> +source include/wait_for_slave_sql_to_stop.inc;
>> +
>> +connection slave;
>> +
>> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
>> +disable_query_log;
>> +eval SELECT "$last_error" AS Last_SQL_Error;
>> +enable_query_log;
>> +
>> +set foreign_key_checks= 0;
>> +delete from ti1 where b=1;
>> +set foreign_key_checks= 1;
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +connection master;
>> +sync_slave_with_master;
>> +#connection slave;
>> +set global slave_exec_mode='STRICT';
>> +
>> +# key not found
>> +
>> +connection master;
>> +
>> +CREATE TABLE t1 (a INT PRIMARY KEY);
>> +CREATE TABLE t2 (a INT);
>> +INSERT INTO t1 VALUES (-1),(-2),(-3);
>> +INSERT INTO t2 VALUES (-1),(-2),(-3);
>> +sync_slave_with_master;
>> +
>> +#connection slave;
>> +DELETE FROM t1 WHERE a = -2;
>> +DELETE FROM t2 WHERE a = -2;
>> +connection master;
>> +DELETE FROM t1 WHERE a = -2;
>> +
>> +--echo *** slave must stop
>> +source include/wait_for_slave_sql_to_stop.inc;
>> +
>> +connection slave;
>> +
>> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
>> +disable_query_log;
>> +eval SELECT "$last_error" AS Last_SQL_Error;
>> +enable_query_log;
>> +
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +connection master;
>> +sync_slave_with_master;
>> +#connection slave;
>> +set global slave_exec_mode='STRICT';
>> +
>> +connection master;
>> +DELETE FROM t2 WHERE a = -2;
>> +--echo *** slave must stop
>> +source include/wait_for_slave_sql_to_stop.inc;
>> +
>> +connection slave;
>> +
>> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
>> +disable_query_log;
>> +eval SELECT "$last_error" AS Last_SQL_Error;
>> +enable_query_log;
>> +
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +connection master;
>> +sync_slave_with_master;
>> +#connection slave;
>> +set global slave_exec_mode='STRICT';
>> +
>> +UPDATE t1 SET a = 1 WHERE a = -1;
>> +UPDATE t2 SET a = 1 WHERE a = -1;
>> +
>> +connection master;
>> +UPDATE t1 SET a = 1 WHERE a = -1;
>> +
>> +--echo *** slave must stop
>> +source include/wait_for_slave_sql_to_stop.inc;
>> +
>> +connection slave;
>> +
>> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
>> +disable_query_log;
>> +eval SELECT "$last_error" AS Last_SQL_Error;
>> +enable_query_log;
>> +
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +connection master;
>> +sync_slave_with_master;
>> +#connection slave;
>> +set global slave_exec_mode='STRICT';
>> +
>> +
>> +connection master;
>> +UPDATE t2 SET a = 1 WHERE a = -1;
>> +
>> +--echo *** slave must stop
>> +source include/wait_for_slave_sql_to_stop.inc;
>> +
>> +connection slave;
>> +
>> +let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Errno, 1);
>> +disable_query_log;
>> +eval SELECT "$last_error" AS Last_SQL_Error;
>> +enable_query_log;
>> +
>> +set global slave_exec_mode='IDEMPOTENT';
>> +start slave sql_thread;
>> +connection master;
>> +sync_slave_with_master;
>> +#connection slave;
>> +set global slave_exec_mode='STRICT';
>> +
>> +
>> +# cleanup for bug#31609 tests
>> +
>> +connection master;
>> +set @@session.binlog_format= @save_binlog_format;
>> +drop table t1,t2,ti2,ti1;
>> +
>> +sync_slave_with_master;
>> +
>> +
>> +--echo *** end of tests
>> +
>> +
>> +
>> +
>> +
>> +
>> +
>> +
>> +
>> diff -Nrup a/mysql-test/suite/rpl/t/rpl_row_basic_11bugs.test
> b/mysql-test/suite/rpl/t/rpl_row_basic_11bugs.test
>> --- a/mysql-test/suite/rpl/t/rpl_row_basic_11bugs.test	2007-10-24 17:02:32
> +03:00
>> +++ b/mysql-test/suite/rpl/t/rpl_row_basic_11bugs.test	2007-12-07 14:45:57
> +02:00
>> @@ -242,12 +242,17 @@ INSERT INTO t1 VALUES (1,'master,slave')
>>  sync_slave_with_master;
>>  UPDATE t1 SET a = 5, b = 'slave' WHERE a = 1;
>>  SELECT * FROM t1 ORDER BY a;
>> +# since bug#31552/31609 idempotency is not default any longer. In
>> +# order the preceeding test UPDATE t1 to pass the mode is switched
>> +# temprorarily
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  --echo **** On Master ****
>>  connection master;
>>  UPDATE t1 SET a = 5, b = 'master' WHERE a = 1;
>>  SELECT * FROM t1 ORDER BY a;
>>  --echo **** On Slave ****
>>  sync_slave_with_master;
>> +set @@global.slave_exec_mode= default;
>>  let $last_error = query_get_value("SHOW SLAVE STATUS", Last_SQL_Error, 1);
>>  disable_query_log;
>>  eval SELECT "$last_error" AS Last_SQL_Error;
>> diff -Nrup a/mysql-test/suite/rpl/t/rpl_row_mystery22.test
> b/mysql-test/suite/rpl/t/rpl_row_mystery22.test
>> --- a/mysql-test/suite/rpl/t/rpl_row_mystery22.test	2007-06-27 15:27:25 +03:00
>> +++ b/mysql-test/suite/rpl/t/rpl_row_mystery22.test	2007-12-07 14:45:57 +02:00
>> @@ -9,6 +9,12 @@
>>  # first, cause a duplicate key problem on the slave
>>  create table t1(n int auto_increment primary key, s char(10));
>>  sync_slave_with_master;
>> +
>> +# bug#31552/31609 idempotency is not default any longer
>> +# so that the declared in heading comments aim of the test
>> +# should be backed up with explicit setting of the slave mode
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>> +
>>  insert into t1 values (2,'old');
>>  connection master;
>>  insert into t1 values(NULL,'new');
>> @@ -43,3 +49,4 @@ select * from t1 order by n;
>>  connection master;
>>  drop table t1;
>>  sync_slave_with_master;
>> +set @@global.slave_exec_mode= default;
>> diff -Nrup a/mysql-test/suite/rpl/t/rpl_temporary_errors.test
> b/mysql-test/suite/rpl/t/rpl_temporary_errors.test
>> --- a/mysql-test/suite/rpl/t/rpl_temporary_errors.test	2007-10-24 17:02:32
> +03:00
>> +++ b/mysql-test/suite/rpl/t/rpl_temporary_errors.test	2007-12-07 14:45:57
> +02:00
>> @@ -8,6 +8,9 @@ INSERT INTO t1 VALUES (1,1), (2,2), (3,3
>>  --echo **** On Slave ****
>>  sync_slave_with_master;
>>  SHOW STATUS LIKE 'Slave_retried_transactions';
>> +# since bug#31552/31609 idempotency is not default any longer. In order
>> +# the following UPDATE t1 to pass the mode is switched temprorarily
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  UPDATE t1 SET a = 5, b = 47 WHERE a = 1;
>>  SELECT * FROM t1;
>>  --echo **** On Master ****
>> @@ -17,6 +20,7 @@ SELECT * FROM t1;
>>  #SHOW BINLOG EVENTS;
>>  --echo **** On Slave ****
>>  sync_slave_with_master;
>> +set @@global.slave_exec_mode= default;
>>  SHOW STATUS LIKE 'Slave_retried_transactions';
>>  SELECT * FROM t1;
>>  source include/show_slave_status.inc;
>> diff -Nrup a/mysql-test/suite/rpl_ndb/r/rpl_row_basic_7ndb.result
> b/mysql-test/suite/rpl_ndb/r/rpl_row_basic_7ndb.result
>> --- a/mysql-test/suite/rpl_ndb/r/rpl_row_basic_7ndb.result	2007-06-27 15:27:31
> +03:00
>> +++ b/mysql-test/suite/rpl_ndb/r/rpl_row_basic_7ndb.result	2007-12-07 14:45:57
> +02:00
>> @@ -370,6 +370,7 @@ C1	C2
>>  1	3
>>  2	6
>>  3	9
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  --- on master: new values inserted ---
>>  INSERT INTO t7 VALUES (1,2), (2,4), (3,6);
>>  SELECT * FROM t7 ORDER BY C1;
>> @@ -377,6 +378,7 @@ C1	C2
>>  1	2
>>  2	4
>>  3	6
>> +set @@global.slave_exec_mode= 'STRICT';
>>  --- on slave: old values should be overwritten by replicated values ---
>>  SELECT * FROM t7 ORDER BY C1;
>>  C1	C2
>> @@ -406,8 +408,10 @@ a	b	c
>>  2	4	6
>>  3	6	9
>>  99	99	99
>> +set @@global.slave_exec_mode= 'IDEMPOTENT';
>>  --- on master ---
>>  INSERT INTO t8 VALUES (2,4,8);
>> +set @@global.slave_exec_mode= default;
>>  --- on slave ---
>>  SELECT * FROM t8 ORDER BY a;
>>  a	b	c
>> diff -Nrup a/sql/log_event.cc b/sql/log_event.cc
>> --- a/sql/log_event.cc	2007-10-30 22:23:39 +02:00
>> +++ b/sql/log_event.cc	2007-12-07 14:45:57 +02:00
>> @@ -90,6 +90,28 @@ static const char *HA_ERR(int i)
>>    case HA_ERR_LOGGING_IMPOSSIBLE: return "HA_ERR_LOGGING_IMPOSSIBLE";
>>    case HA_ERR_CORRUPT_EVENT: return "HA_ERR_CORRUPT_EVENT";
>>    }
>> +  return 0;
>> +}
>> +
>> +/**
>> +   macro to call from different branches of Rows_log_event::do_apply_event
>> +*/
>> +static void inline slave_rows_error_report(enum loglevel level, int ha_error,
>> +                                           Relay_log_info const *rli, THD *thd,
>> +                                           TABLE *table, const char * type,
>> +                                           const char *log_name, ulong pos)
>> +{
>> +  const char *handler_error=  HA_ERR(ha_error);
>> +  rli->report(level, thd->net.last_errno,
>> +              "Could not execute %s event on table %s.%s;"
>> +              "%s%s handler error %s; "
>> +              "the event's master log and end_log_pos %s %lu",
>> +              type, table->s->db.str,
>> +              table->s->table_name.str,
>> +              thd->net.last_error[0] != 0 ? thd->net.last_error : "",
>> +              thd->net.last_error[0] != 0 ? ";" : "",
>> +              handler_error == NULL? "<unknown>" : handler_error,
>> +              log_name, pos);
>>  }
>>  #endif
>>  @@ -6060,7 +6082,6 @@ int Rows_log_event::do_apply_event(Relay
>>  {
>>    DBUG_ENTER("Rows_log_event::do_apply_event(Relay_log_info*)");
>>    int error= 0;
>> -
>>    /*
>>      If m_table_id == ~0UL, then we have a dummy event that does not
>>      contain any data.  In that case, we just remove all tables in the
>> @@ -6106,6 +6127,24 @@ int Rows_log_event::do_apply_event(Relay
>>      */
>>      lex_start(thd);
>>  +    /*
>> +      There are a few flags that are replicated with each row event.
>> +      Make sure to set/clear them before executing the main body of
>> +      the event.
>> +    */
>> +    if (get_flags(NO_FOREIGN_KEY_CHECKS_F))
>> +        thd->options|= OPTION_NO_FOREIGN_KEY_CHECKS;
>> +    else
>> +        thd->options&= ~OPTION_NO_FOREIGN_KEY_CHECKS;
>> +
>> +    if (get_flags(RELAXED_UNIQUE_CHECKS_F))
>> +        thd->options|= OPTION_RELAXED_UNIQUE_CHECKS;
>> +    else
>> +        thd->options&= ~OPTION_RELAXED_UNIQUE_CHECKS;
>> +    /* A small test to verify that objects have consistent types */
>> +    DBUG_ASSERT(sizeof(thd->options) ==
> sizeof(OPTION_RELAXED_UNIQUE_CHECKS));
>> +
>> +
>>      while ((error= lock_tables(thd, rli->tables_to_lock,
>>                                 rli->tables_to_lock_count,
> &need_reopen)))
>>      {
>> @@ -6240,22 +6279,6 @@ int Rows_log_event::do_apply_event(Relay
>>        So we call set_time(), like in SBR. Presently it changes nothing.
>>      */
>>      thd->set_time((time_t)when);
>> -    /*
>> -      There are a few flags that are replicated with each row event.
>> -      Make sure to set/clear them before executing the main body of
>> -      the event.
>> -    */
>> -    if (get_flags(NO_FOREIGN_KEY_CHECKS_F))
>> -        thd->options|= OPTION_NO_FOREIGN_KEY_CHECKS;
>> -    else
>> -        thd->options&= ~OPTION_NO_FOREIGN_KEY_CHECKS;
>> -
>> -    if (get_flags(RELAXED_UNIQUE_CHECKS_F))
>> -        thd->options|= OPTION_RELAXED_UNIQUE_CHECKS;
>> -    else
>> -        thd->options&= ~OPTION_RELAXED_UNIQUE_CHECKS;
>> -    /* A small test to verify that objects have consistent types */
>> -    DBUG_ASSERT(sizeof(thd->options) ==
> sizeof(OPTION_RELAXED_UNIQUE_CHECKS));
>>       /*
>>        Now we are in a statement and will stay in a statement until we
>> @@ -6288,8 +6311,9 @@ int Rows_log_event::do_apply_event(Relay
>>      if (!get_flags(COMPLETE_ROWS_F))
>>        bitmap_intersect(table->write_set,&m_cols);
>>  +    this->slave_exec_mode= slave_exec_mode_options; // fix the
>> mode
>> +
>>   
>
> Is this really the right place to put that update? I suspect that it
> is better to pass that parameter as part of the execution context,
> i.e., either as a parameter or in Relay_log_info structure.
>
> In general, using member variables for parameter passing leads to very
> big objects, and also increases the size of the code as well as kills
> a lot of optimizations.

It's hard to disagree with the last statement.
However, the same would apply to rli object...

There were several reasons why Rows_log_event was choosen: 

 - avoiding messing with concurrency: 
   a rli instance is shared between many threads
 - the mode is to stay unchanged at least for the entire time of
   executing of the event so that its external altering won't affect
   the current event handling;
 - no hassles with signature change:
   rli is not always available in the Rows_log_event's methods;

>
>>      // Do event specific preparations -         error=
>> do_before_row_operations(rli);
>>       // row processing loop
>> @@ -6308,22 +6332,41 @@ int Rows_log_event::do_apply_event(Relay
>>        {
>>        case 0:
>>  	break;
>> +      /*
>> +        The following list of "idempotent" errors
>> +        means that an error from the list might happen
>> +        because of idempotent (more than once) +        applying of
>> a binlog file.
>> +        Notice, that binlog has a  ddl operation its
>> +        second applying may cause
>>  -      /* Some recoverable errors */
>> +        case HA_ERR_TABLE_DEF_CHANGED:
>> +        case HA_ERR_CANNOT_ADD_FOREIGN:
>> +        +        which are not included into to the list.
>> +      */
>>        case HA_ERR_RECORD_CHANGED:
>>        case HA_ERR_RECORD_DELETED:
>>        case HA_ERR_KEY_NOT_FOUND:
>>        case HA_ERR_END_OF_FILE:
>> -        /* Idempotency support: OK if tuple does not exist */
>> +      case HA_ERR_FOUND_DUPP_KEY:
>> +      case HA_ERR_FOUND_DUPP_UNIQUE:
>> +      case HA_ERR_FOREIGN_DUPLICATE_KEY:
>> +      case HA_ERR_NO_REFERENCED_ROW:
>> +      case HA_ERR_ROW_IS_REFERENCED:
>> +
>>          DBUG_PRINT("info", ("error: %s", HA_ERR(error)));
>> -        error= 0;
>> +        if (bit_is_set(slave_exec_mode, SLAVE_EXEC_MODE_IDEMPOTENT) == 1)
>> +        {
>> +          if (global_system_variables.log_warnings)
>> +            slave_rows_error_report(WARNING_LEVEL, error, rli, thd, table,
>> +                                    get_type_str(),
>> +                                    RPL_LOG_NAME, (ulong) log_pos);
>> +          error= 0;
>> +        }
>>          break;
>> -
>> +               default:
>> -	rli->report(ERROR_LEVEL, thd->net.last_errno,
>> -                    "Error in %s event: row application failed. %s",
>> -                    get_type_str(),
>> -                    thd->net.last_error ? thd->net.last_error : "");
>>  	thd->query_error= 1;
>>  	break;
>>        }
>> @@ -6367,16 +6410,14 @@ int Rows_log_event::do_apply_event(Relay
>>    */
>>    if (rli->tables_to_lock && get_flags(STMT_END_F))
>>      const_cast<Relay_log_info*>(rli)->clear_tables_to_lock();
>> -
>> +     if (error)
>>    {                     /* error has occured during the transaction */
>> -    rli->report(ERROR_LEVEL, thd->net.last_errno,
>> -                "Error in %s event: error during transaction execution "
>> -                "on table %s.%s. %s",
>> -                get_type_str(), table->s->db.str,
>> -                table->s->table_name.str,
>> -                thd->net.last_error ? thd->net.last_error : "");
>> -
>> +    slave_rows_error_report(ERROR_LEVEL, error, rli, thd, table,
>> +                            get_type_str(), RPL_LOG_NAME, (ulong) log_pos);
>> +  }
>> +  if (error)
>> +  {
>>      /*
>>        If one day we honour --skip-slave-errors in row-based replication, and
>>        the error should be skipped, then we would clear mappings, rollback,
>> @@ -6488,6 +6529,7 @@ Rows_log_event::do_update_pos(Relay_log_
>>      */
>>       thd->reset_current_stmt_binlog_row_based();
>> +
>>      rli->cleanup_context(thd, 0);
>>      if (error == 0)
>>      {
>> @@ -7170,43 +7212,50 @@ Write_rows_log_event::do_before_row_oper
>>  {
>>    int error= 0;
>>  -  /*
>> -    We are using REPLACE semantics and not INSERT IGNORE semantics
>> -    when writing rows, that is: new rows replace old rows.  We need to
>> -    inform the storage engine that it should use this behaviour.
>> +  /**
>> +     todo: to introduce a property for the event (handler?) which forces
>> +     applying the event in the replace (idempotent) fashion.
>>    */
>> +  if (bit_is_set(slave_exec_mode, SLAVE_EXEC_MODE_IDEMPOTENT) == 1 ||
>> +      m_table->s->db_type()->db_type == DB_TYPE_NDBCLUSTER)
>> +  {
>> +    /*
>> +      We are using REPLACE semantics and not INSERT IGNORE semantics
>> +      when writing rows, that is: new rows replace old rows.  We need to
>> +      inform the storage engine that it should use this behaviour.
>> +    */
>> +    +    /* Tell the storage engine that we are using REPLACE
>> semantics. */
>> +    thd->lex->duplicates= DUP_REPLACE;
>> +    +    /*
>> +      Pretend we're executing a REPLACE command: this is needed for
>> +      InnoDB and NDB Cluster since they are not (properly) checking the
>> +      lex->duplicates flag.
>> +    */
>> +    thd->lex->sql_command= SQLCOM_REPLACE;
>> +    /* +       Do not raise the error flag in case of hitting to an
>> unique attribute
>> +    */
>> +    m_table->file->extra(HA_EXTRA_IGNORE_DUP_KEY);
>> +    /* +       NDB specific: update from ndb master wrapped as
>> Write_rows
>> +       so that the event should be applied to replace slave's row
>> +    */
>> +    m_table->file->extra(HA_EXTRA_WRITE_CAN_REPLACE);
>> +    /* +       NDB specific: if update from ndb master wrapped as
>> Write_rows
>> +       does not find the row it's assumed idempotent binlog applying
>> +       is taking place; don't raise the error.
>> +    */
>> +    m_table->file->extra(HA_EXTRA_IGNORE_NO_KEY);
>> +    /*
>> +      TODO: the cluster team (Tomas?) says that it's better if the engine knows
>> +      how many rows are going to be inserted, then it can allocate needed
> memory
>> +      from the start.
>> +    */
>> +  }
>>  -  /* Tell the storage engine that we are using REPLACE
>> semantics. */
>> -  thd->lex->duplicates= DUP_REPLACE;
>> -
>> -  /*
>> -    Pretend we're executing a REPLACE command: this is needed for
>> -    InnoDB and NDB Cluster since they are not (properly) checking the
>> -    lex->duplicates flag.
>> -  */
>> -  thd->lex->sql_command= SQLCOM_REPLACE;
>> -  /* -     Do not raise the error flag in case of hitting to an
>> unique attribute
>> -  */
>> -  m_table->file->extra(HA_EXTRA_IGNORE_DUP_KEY);
>> -  /* -     NDB specific: update from ndb master wrapped as
>> Write_rows
>> -  */
>> -  /*
>> -    so that the event should be applied to replace slave's row
>> -  */
>> -  m_table->file->extra(HA_EXTRA_WRITE_CAN_REPLACE);
>> -  /* -     NDB specific: if update from ndb master wrapped as
>> Write_rows
>> -     does not find the row it's assumed idempotent binlog applying
>> -     is taking place; don't raise the error.
>> -  */
>> -  m_table->file->extra(HA_EXTRA_IGNORE_NO_KEY);
>> -  /*
>> -    TODO: the cluster team (Tomas?) says that it's better if the engine knows
>> -    how many rows are going to be inserted, then it can allocate needed memory
>> -    from the start.
>> -  */
>>    m_table->file->ha_start_bulk_insert(0);
>>    /*
>>      We need TIMESTAMP_NO_AUTO_SET otherwise ha_write_row() will not use fill
>> @@ -7228,18 +7277,23 @@ Write_rows_log_event::do_before_row_oper
>>  }
>>   int -Write_rows_log_event::do_after_row_operations(const
>> Slave_reporting_capability *const,
>> +Write_rows_log_event::do_after_row_operations(const
>> Slave_reporting_capability *const,
>>                                                int error)
>>  {
>>    int local_error= 0;
>> -  m_table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
>> -  m_table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE);
>> -  /*
>> -    reseting the extra with -
>> table->file->extra(HA_EXTRA_NO_IGNORE_NO_KEY); -    fires bug#27077
>> -    todo: explain or fix
>> -  */
>> +  if (bit_is_set(slave_exec_mode, SLAVE_EXEC_MODE_IDEMPOTENT) == 1 ||
>> +      m_table->s->db_type()->db_type == DB_TYPE_NDBCLUSTER)
>> +  {
>> +    m_table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
>> +    m_table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE);
>> +    /*
>> +      resetting the extra with +
>> table->file->extra(HA_EXTRA_NO_IGNORE_NO_KEY); +      fires
>> bug#27077
>> +      explanation: file->reset() performs this duty
>> +      ultimately. Still todo: fix
>> +    */
>> +  }
>>    if ((local_error= m_table->file->ha_end_bulk_insert()))
>>    {
>>      m_table->file->print_error(local_error, MYF(0));
>> @@ -7358,23 +7412,22 @@ Rows_log_event::write_row(const Relay_lo
>>     while ((error= table->file->ha_write_row(table->record[0])))
>>    {
>> -    if (error == HA_ERR_LOCK_DEADLOCK || error == HA_ERR_LOCK_WAIT_TIMEOUT)
>> -    {
>> -      table->file->print_error(error, MYF(0)); /* to check at
> exec_relay_log_event */
>> -      DBUG_RETURN(error);
>> -    }
>> -    if ((keynum= table->file->get_dup_key(error)) < 0)
>> +    if (error == HA_ERR_LOCK_DEADLOCK ||
>> +        error == HA_ERR_LOCK_WAIT_TIMEOUT ||
>> +        (keynum= table->file->get_dup_key(error)) < 0 ||
>> +        !overwrite)
>>   
>
> Wouldn't it be better to check the errors for which get_dup_key()
> behaves correctly?

You meant that if the error in

  while ((error= table->file->ha_write_row()

is set to  HA_ERR_FOUND_DUPP_UNIQUE or HA_ERR_FOUND_DUPP_KEY are the
only warrants to go into the body of the loop, did not you?

I think that will make sense esp if yet another handler error will be issued.

> As it stands now, this could would have to be
> changed if a HA_ERR_LOCK_LIVELOCK was introduced.

What is the reason to have that?

>
>>      {
>> -      DBUG_PRINT("info",("Can't locate duplicate key (get_dup_key returns
> %d)",keynum));
>> -      table->file->print_error(error, MYF(0));
>> +      DBUG_PRINT("info",("get_dup_key returns %d)", keynum));
>>        /*
>> -        We failed to retrieve the duplicate key
>> +        Deadlock, waiting for lock or just an error from the handler
>> +        such as HA_ERR_FOUND_DUPP_KEY when overwrite is false.
>> +        Retrieval of the duplicate key number may fail
>>          - either because the error was not "duplicate key" error
>>          - or because the information which key is not available
>>        */
>> +      table->file->print_error(error, MYF(0));
>>        DBUG_RETURN(error);
>>      }
>> -
>>      /*
>>         We need to retrieve the old row into record[1] to be able to
>>         either update or delete the offending record.  We either:
>> @@ -7512,11 +7565,13 @@ int  Write_rows_log_event::do_exec_row(const
>> Relay_log_info *const rli)
>>  {
>>    DBUG_ASSERT(m_table != NULL);
>> -  int error= write_row(rli, TRUE /* overwrite */);
>> -  
>> +  int error=
>> +    write_row(rli,        /* if 1 then overwrite */
>> +              bit_is_set(slave_exec_mode, SLAVE_EXEC_MODE_IDEMPOTENT) == 1);
>> +       if (error && !thd->net.last_errno)
>>      thd->net.last_errno= error;
>> -      +     return error;  }
>>  diff -Nrup a/sql/log_event.h b/sql/log_event.h
>> --- a/sql/log_event.h	2007-10-30 10:51:08 +02:00
>> +++ b/sql/log_event.h	2007-12-07 14:45:58 +02:00
>> @@ -809,6 +809,12 @@ public:
>>     bool cache_stmt;
>>  +  /**
>> +    A storage to cache the global system variable's value.
>> +    Handling of a separate event will be governed its member.
>> +  */
>> +  ulong slave_exec_mode;
>> +
>>  #ifndef MYSQL_CLIENT
>>    THD* thd;
>>  diff -Nrup a/sql/mysql_priv.h b/sql/mysql_priv.h
>> --- a/sql/mysql_priv.h	2007-10-23 18:10:26 +03:00
>> +++ b/sql/mysql_priv.h	2007-12-07 14:45:58 +02:00
>> @@ -1813,6 +1813,7 @@ extern uint volatile thread_count, threa
>>  extern my_bool opt_sql_bin_update, opt_safe_user_create, opt_no_mix_types;
>>  extern my_bool opt_safe_show_db, opt_local_infile, opt_myisam_use_mmap;
>>  extern my_bool opt_slave_compressed_protocol, use_temp_pool;
>> +extern ulong slave_exec_mode_options;
>>  extern my_bool opt_readonly, lower_case_file_system;
>>  extern my_bool opt_enable_named_pipe, opt_sync_frm, opt_allow_suspicious_udfs;
>>  extern my_bool opt_secure_auth;
>> diff -Nrup a/sql/mysqld.cc b/sql/mysqld.cc
>> --- a/sql/mysqld.cc	2007-10-30 10:51:08 +02:00
>> +++ b/sql/mysqld.cc	2007-12-07 14:45:58 +02:00
>> @@ -445,6 +445,8 @@ ulong thread_stack, what_to_log;
>>  ulong query_buff_size, slow_launch_time, slave_open_temp_tables;
>>  ulong open_files_limit, max_binlog_size, max_relay_log_size;
>>  ulong slave_net_timeout, slave_trans_retries;
>> +ulong slave_exec_mode_options;
>> +const char *slave_exec_mode_str= "STRICT";
>>  ulong thread_cache_size=0, thread_pool_size= 0;
>>  ulong binlog_cache_size=0, max_binlog_cache_size=0;
>>  ulong query_cache_size=0;
>> @@ -5101,7 +5103,8 @@ enum options_mysqld
>>    OPT_SECURE_FILE_PRIV,
>>    OPT_MIN_EXAMINED_ROW_LIMIT,
>>    OPT_LOG_SLOW_SLAVE_STATEMENTS,
>> -  OPT_OLD_MODE
>> +  OPT_OLD_MODE,
>> +  OPT_SLAVE_EXEC_MODE
>>  };
>>   @@ -5799,8 +5802,11 @@ replicating a LOAD DATA INFILE command."
>>     (uchar**) &slave_load_tmpdir, (uchar**) &slave_load_tmpdir, 0,
> GET_STR_ALLOC,
>>     REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
>>    {"slave-skip-errors", OPT_SLAVE_SKIP_ERRORS,
>> -   "Tells the slave thread to continue replication when a query returns an error
> from the provided list.",
>> +   "Tells the slave thread to continue replication when a query event returns an
> error from the provided list.",
>>     0, 0, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
>> +  {"slave-exec-mode", OPT_SLAVE_EXEC_MODE,
>> +   "Modes for how replication events should be executed.  Legal values are
> STRICT (default) and IDEMPOTENT. In IDEMPOTENT mode, replication will not stop for
> operations that are idempotent. In STRICT mode, replication will stop on any unexpected
> difference between the master and the slave.",
>> +   (uchar**) &slave_exec_mode_str, (uchar**) &slave_exec_mode_str, 0,
> GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
>>  #endif
>>    {"slow-query-log", OPT_SLOW_LOG,
>>     "Enable|disable slow query log", (uchar**) &opt_slow_log,
>> @@ -7109,6 +7115,9 @@ static void mysql_init_variables(void)
>>     /* Things with default values that are not zero */
>>    delay_key_write_options= (uint) DELAY_KEY_WRITE_ON;
>> +  slave_exec_mode_options= 0;
>> +  slave_exec_mode_options= (uint)
>> +    find_bit_type_or_exit(slave_exec_mode_str, &slave_exec_mode_typelib,
> NULL);
>>    opt_specialflag= SPECIAL_ENGLISH;
>>    unix_sock= ip_sock= INVALID_SOCKET;
>>    mysql_home_ptr= mysql_home;
>> @@ -7318,6 +7327,10 @@ mysqld_get_one_option(int optid,
>>    case OPT_SLAVE_SKIP_ERRORS:
>>      init_slave_skip_errors(argument);
>>      break;
>> +  case OPT_SLAVE_EXEC_MODE:
>> +    slave_exec_mode_options= (uint)
>> +      find_bit_type_or_exit(argument, &slave_exec_mode_typelib, "");
>> +    break;
>>  #endif
>>    case OPT_SAFEMALLOC_MEM_LIMIT:
>>  #if !defined(DBUG_OFF) && defined(SAFEMALLOC)
>> @@ -7863,6 +7876,8 @@ static void get_options(int *argc,char *
>>    }
>>    /* Set global MyISAM variables from delay_key_write_options */
>>    fix_delay_key_write((THD*) 0, OPT_GLOBAL);
>> +  /* Set global slave_exec_mode from its option */
>> +  fix_slave_exec_mode(OPT_GLOBAL);
>>   #ifndef EMBEDDED_LIBRARY
>>    if (mysqld_chroot)
>> diff -Nrup a/sql/rpl_rli.cc b/sql/rpl_rli.cc
>> --- a/sql/rpl_rli.cc	2007-10-13 23:12:47 +03:00
>> +++ b/sql/rpl_rli.cc	2007-12-07 14:45:59 +02:00
>> @@ -1160,6 +1160,11 @@ void Relay_log_info::cleanup_context(THD
>>    close_thread_tables(thd);
>>    clear_tables_to_lock();
>>    clear_flag(IN_STMT);
>> +  /*
>> +    Cleanup for the flags that have been set at do_apply_event.
>> +  */
>> +  thd->options&= ~OPTION_NO_FOREIGN_KEY_CHECKS;
>> +  thd->options&= ~OPTION_RELAXED_UNIQUE_CHECKS;
>>    last_event_start_time= 0;
>>    DBUG_VOID_RETURN;
>>  }
>> diff -Nrup a/sql/set_var.cc b/sql/set_var.cc
>> --- a/sql/set_var.cc	2007-10-23 18:10:27 +03:00
>> +++ b/sql/set_var.cc	2007-12-07 14:45:59 +02:00
>> @@ -88,6 +88,16 @@ TYPELIB delay_key_write_typelib=
>>    delay_key_write_type_names, NULL
>>  };
>>  +const char *slave_exec_mode_names[]=
>> +{ "STRICT", "IDEMPOTENT", NullS };
>> +static const unsigned int slave_exec_mode_names_len[]=
>> +{ sizeof("STRICT") - 1, sizeof("IDEMPOTENT") - 1, 0 };
>> +TYPELIB slave_exec_mode_typelib=
>> +{
>> +  array_elements(slave_exec_mode_names)-1, "",
>> +  slave_exec_mode_names, (unsigned int *) slave_exec_mode_names_len
>> +};
>> +
>>  static int  sys_check_ftb_syntax(THD *thd,  set_var *var);
>>  static bool sys_update_ftb_syntax(THD *thd, set_var * var);
>>  static void sys_default_ftb_syntax(THD *thd, enum_var_type type);
>> @@ -408,6 +418,11 @@ static sys_var_const_str_ptr sys_secure_
>>  static sys_var_long_ptr	sys_server_id(&vars, "server_id", &server_id,
> fix_server_id);
>>  static sys_var_bool_ptr	sys_slave_compressed_protocol(&vars,
> "slave_compressed_protocol",
>>  						      &opt_slave_compressed_protocol);
>> +static sys_var_set_slave_mode slave_exec_mode(&vars,
>> +                                              "slave_exec_mode",
>> +                                              &slave_exec_mode_options,
>> +                                              &slave_exec_mode_typelib,
>> +                                              0);
>>  static sys_var_long_ptr	sys_slow_launch_time(&vars, "slow_launch_time",
>>  					     &slow_launch_time);
>>  static sys_var_thd_ulong	sys_sort_buffer(&vars, "sort_buffer_size",
>> @@ -981,6 +996,79 @@ extern void fix_delay_key_write(THD *thd
>>    }
>>  }
>>  +bool sys_var_set::update(THD *thd, set_var *var)
>> +{
>> +  *value= var->save_result.ulong_value;
>> +  return 0;
>> +};
>> +
>> +uchar *sys_var_set::value_ptr(THD *thd, enum_var_type type,
>> +                              LEX_STRING *base)
>> +{
>> +  char buff[256];
>> +  String tmp(buff, sizeof(buff), &my_charset_latin1);
>> +  ulong length;
>> +  ulong val= *value;
>> +
>> +  tmp.length(0);
>> +  for (uint i= 0; val; val>>= 1, i++)
>> +  {
>> +    if (val & 1)
>> +    {
>> +      tmp.append(enum_names->type_names[i],
>> +                 enum_names->type_lengths[i]);
>> +      tmp.append(',');
>> +    }
>> +  }
>> +
>> +  if ((length= tmp.length()))
>> +    length--;
>> +  return (uchar*) thd->strmake(tmp.ptr(), length);
>> +}
>> +
>> +void sys_var_set_slave_mode::set_default(THD *thd, enum_var_type type)
>> +{
>> +  slave_exec_mode_options= 0;
>> +  bit_do_set(slave_exec_mode_options, SLAVE_EXEC_MODE_STRICT);
>> +}
>> +
>> +bool sys_var_set_slave_mode::check(THD *thd, set_var *var)
>> +{
>> +  bool rc=  sys_var_set::check(thd, var);
>> +  if (!rc &&
>> +      bit_is_set(var->save_result.ulong_value, SLAVE_EXEC_MODE_STRICT) == 1
> &&
>> +      bit_is_set(var->save_result.ulong_value, SLAVE_EXEC_MODE_IDEMPOTENT) ==
> 1)
>> +  {
>> +    rc= true;
>> +    my_error(ER_SLAVE_AMBIGOUS_EXEC_MODE, MYF(0), "");
>> +  }
>> +  return rc;
>> +}
>> +
>> +bool sys_var_set_slave_mode::update(THD *thd, set_var *var)
>> +{
>> +  bool rc;
>> +  pthread_mutex_lock(&LOCK_global_system_variables);
>> +  rc= sys_var_set::update(thd, var);
>> +  pthread_mutex_unlock(&LOCK_global_system_variables);
>> +  return rc;
>> +}
>> +
>> +void fix_slave_exec_mode(enum_var_type type)
>> +{
>> +  DBUG_ENTER("fix_slave_exec_mode");
>> +  compile_time_assert(sizeof(slave_exec_mode_options) * CHAR_BIT
>> +                      > SLAVE_EXEC_MODE_LAST_BIT - 1);
>> +  if (bit_is_set(slave_exec_mode_options, SLAVE_EXEC_MODE_STRICT) == 1
> &&
>> +      bit_is_set(slave_exec_mode_options, SLAVE_EXEC_MODE_IDEMPOTENT) == 1)
>> +  {
>> +    sql_print_error("Ambiguous slave modes combination."
>> +                    " STRICT will be used");
>> +    bit_do_clear(slave_exec_mode_options, SLAVE_EXEC_MODE_IDEMPOTENT);
>> +  }
>> +  if (bit_is_set(slave_exec_mode_options, SLAVE_EXEC_MODE_IDEMPOTENT) == 0)
>> +    bit_do_set(slave_exec_mode_options, SLAVE_EXEC_MODE_STRICT);
>> +}
>>   bool sys_var_thd_binlog_format::is_readonly() const
>>  {
>> @@ -3165,7 +3253,18 @@ int set_var::light_check(THD *thd)
>>    return 0;
>>  }
>>  +/**
>> +  Update variable
>>  +  @param   thd    thread handler
>> +  @returns 0|1    ok or	ERROR
>> +
>> +  @note ERROR can be only due to abnormal operations involving
>> +  the server's execution evironment such as
>> +  out of memory, hard disk failure or the computer blows up.
>> +  Consider set_var::check() method if there is a need to return
>> +  an error due to logics.
>> +*/
>>  int set_var::update(THD *thd)
>>  {
>>    if (!value)
>> diff -Nrup a/sql/set_var.h b/sql/set_var.h
>> --- a/sql/set_var.h	2007-08-13 16:11:12 +03:00
>> +++ b/sql/set_var.h	2007-12-07 14:45:59 +02:00
>> @@ -30,7 +30,8 @@ class sys_var_pluginvar; /* opaque */
>>  typedef struct system_variables SV;
>>  typedef struct my_locale_st MY_LOCALE;
>>  -extern TYPELIB bool_typelib, delay_key_write_typelib,
>> sql_mode_typelib;
>> +extern TYPELIB bool_typelib, delay_key_write_typelib, sql_mode_typelib,
>> +  slave_exec_mode_typelib;
>>   
>
> It would have been better to just add a line with the new variable,
> leaving the old line untouched. Any change of this form just make
> merges more complicated. Usually not a problem, but avoiding
> unnecessary rewrites does have a value.
>>   typedef int (*sys_check_func)(THD *,  set_var *);
>>  typedef bool (*sys_update_func)(THD *, set_var *);
>> @@ -771,6 +772,42 @@ public:
>>  };
>>   +class sys_var_set :public sys_var
>> +{
>> +protected:
>> +  ulong *value;
>> +  TYPELIB *enum_names;
>> +public:
>> +  sys_var_set(sys_var_chain *chain, const char *name_arg, ulong *value_arg,
>> +              TYPELIB *typelib, sys_after_update_func func)
>> +    :sys_var(name_arg, func), value(value_arg), enum_names(typelib)
>> +  { chain_sys_var(chain); }
>> +  virtual bool check(THD *thd, set_var *var)
>> +  {
>> +    return check_set(thd, var, enum_names);
>> +  }
>> +  virtual void set_default(THD *thd, enum_var_type type)
>> +  {  +    *value= 0;
>> +  }
>> +  bool update(THD *thd, set_var *var);
>> +  uchar *value_ptr(THD *thd, enum_var_type type, LEX_STRING *base);
>> +  bool check_update_type(Item_result type) { return 0; }
>> +  SHOW_TYPE show_type() { return SHOW_CHAR; }
>> +};
>> +
>> +class sys_var_set_slave_mode :public sys_var_set
>> +{
>> +public:
>> +  sys_var_set_slave_mode(sys_var_chain *chain, const char *name_arg,
>> +                         ulong *value_arg,
>> +                         TYPELIB *typelib, sys_after_update_func func) :
>> +    sys_var_set(chain, name_arg, value_arg, typelib, func) {}
>> +  void set_default(THD *thd, enum_var_type type);
>> +  bool check(THD *thd, set_var *var);
>> +  bool update(THD *thd, set_var *var);
>> +};
>> +
>>  class sys_var_log_output :public sys_var
>>  {
>>    ulong *value;
>> @@ -1189,6 +1226,7 @@ sys_var *find_sys_var(THD *thd, const ch
>>  int sql_set_variables(THD *thd, List<set_var_base> *var_list);
>>  bool not_all_support_one_shot(List<set_var_base> *var_list);
>>  void fix_delay_key_write(THD *thd, enum_var_type type);
>> +void fix_slave_exec_mode(enum_var_type type);
>>  ulong fix_sql_mode(ulong sql_mode);
>>  extern sys_var_const_str sys_charset_system;
>>  extern sys_var_str sys_init_connect;
>> diff -Nrup a/sql/share/errmsg.txt b/sql/share/errmsg.txt
>> --- a/sql/share/errmsg.txt	2007-10-17 11:13:54 +03:00
>> +++ b/sql/share/errmsg.txt	2007-12-07 14:46:00 +02:00
>> @@ -6112,3 +6112,5 @@ ER_TRG_CANT_OPEN_TABLE
>>   ER_CANT_CREATE_SROUTINE
>>    eng "Cannot create stored routine `%-.64s`. Check warnings"
>> +ER_SLAVE_AMBIGOUS_EXEC_MODE
>> +  eng "Ambiguous slave modes combination. %s"
>> diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
>> --- a/sql/sql_class.h	2007-11-06 14:03:54 +02:00
>> +++ b/sql/sql_class.h	2007-12-07 14:45:59 +02:00
>> @@ -38,6 +38,9 @@ enum enum_ha_read_modes { RFIRST, RNEXT,
>>  enum enum_duplicates { DUP_ERROR, DUP_REPLACE, DUP_UPDATE };
>>  enum enum_delay_key_write { DELAY_KEY_WRITE_NONE, DELAY_KEY_WRITE_ON,
>>  			    DELAY_KEY_WRITE_ALL };
>> +enum enum_slave_exec_mode { SLAVE_EXEC_MODE_STRICT,
>> +                            SLAVE_EXEC_MODE_IDEMPOTENT,
>> +                            SLAVE_EXEC_MODE_LAST_BIT};
>>  enum enum_mark_columns
>>  { MARK_COLUMNS_NONE, MARK_COLUMNS_READ, MARK_COLUMNS_WRITE};
>>  
>>
>>   
>


Thanks for your insightful review!

cheers,

Andrei
Thread
bk commit into 5.1 tree (aelkin:1.2610) BUG#31552Andrei Elkin7 Dec
  • Re: bk commit into 5.1 tree (aelkin:1.2610) BUG#31552Mats Kindahl7 Dec
    • Re: bk commit into 5.1 tree (aelkin:1.2610) BUG#31552Andrei Elkin12 Dec
      • Re: bk commit into 5.1 tree (aelkin:1.2610) BUG#31552Mats Kindahl12 Dec
Re: bk commit into 5.1 tree (aelkin:1.2610) BUG#31552Andrei Elkin7 Dec