From: He Zhenxing Date: August 27 2009 9:01am Subject: Re: bzr commit into mysql-5.1 branch (alfranio.correia:3083) Bug#46864 List-Archive: http://lists.mysql.com/commits/81680 Message-Id: <1251363699.3942.20.camel@hezx> MIME-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7BIT Alfranio Correia wrote: > Hi all, > > He Zhenxing wrote: > > Hi Alfranio, > > > > Patch looks fine, approved! Thank you for the work! > > > > Just one question (do not need to handle it for this bug), do you think > > that we should also rollback the statement when the error is ignored by > > slave with --slave-skip-errors? Current behavior is that we do not > > rollback the statement if the error is ignored. In the document of > > option --slave-skip-errors, it only says that the error should not stop > > the execution of SQL thread, it does not say the the failed statement > > should be rollback or not. But I tend to think that we should rollback > > the statement too in this case. > > > I really don't know what to do. Maybe we should change the current > "if" by something like: > > if (expected_error && expected_error == actual_error) > rollback; > > What do you think? > I think just remove the 'if' will work if we agree to also rollback statement that the error is ignored. > Cheers. > > > Alfranio Correia wrote: > > > >> #At file:///home/acorreia/workspace.sun/repository.mysql/bzrwork/bug-46864/mysql-5.1-bugteam/ based on revid:alfranio.correia@stripped > >> > >> 3083 Alfranio Correia 2009-08-27 > >> BUG#46864 Incorrect update of InnoDB table on slave when using trigger with myisam table > >> > >> Slave does not correctly handle "expected errors" leading to inconsistencies > >> between the mater and slave. Specifically, when a statement changes both > >> transactional and non-transactional tables, the transactional changes are > >> automatically rolled back on the master but the slave ignores the error and > >> does not rollback them thus leading to inconsistencies. > >> > >> To fix the problem, we automatically rollback a statement that fails on > >> the slave but note that the transaction is not rolled back unless a "rollback" > >> command is in the relay log file. > >> @ mysql-test/extra/rpl_tests/rpl_mixing_engines.test > >> Enabled item 13.e which was disabled because of the bug fixed by the > >> current and removed item 14 which was introduced by mistake. > >> > >> modified: > >> mysql-test/extra/rpl_tests/rpl_mixing_engines.test > >> mysql-test/suite/rpl/r/rpl_stm_mixing_engines.result > >> sql/log_event.cc > >> === modified file 'mysql-test/extra/rpl_tests/rpl_mixing_engines.test' > >> --- a/mysql-test/extra/rpl_tests/rpl_mixing_engines.test 2009-08-26 23:13:03 +0000 > >> +++ b/mysql-test/extra/rpl_tests/rpl_mixing_engines.test 2009-08-27 00:09:31 +0000 > >> @@ -562,21 +562,21 @@ let $binlog_start= query_get_value("SHOW > >> --echo # There is a bug in the slave that needs to be fixed before enabling > >> --echo # this part of the test. A bug report will be filed referencing this > >> --echo # test case. > >> -# > >> -#BEGIN; > >> -#INSERT INTO nt_3 VALUES ("new text -28", -28, ''); > >> -#--error ER_DUP_ENTRY > >> -#INSERT INTO tt_3 VALUES ("new text -27", -27, ''), ("new text -28", -28, ''); > >> -#INSERT INTO tt_1 VALUES ("new text -27", -27, ''); > >> -#COMMIT; > >> -# > >> -#BEGIN; > >> -#INSERT INTO tt_4 VALUES ("new text -28", -28, ''); > >> -#--error ER_DUP_ENTRY > >> -#INSERT INTO nt_4 VALUES ("new text -27", -27, ''), ("new text -28", -28, ''); > >> -#INSERT INTO tt_1 VALUES ("new text -28", -28, ''); > >> -#COMMIT; > >> -# > >> + > >> +BEGIN; > >> +INSERT INTO nt_3 VALUES ("new text -28", -28, ''); > >> +--error ER_DUP_ENTRY > >> +INSERT INTO tt_3 VALUES ("new text -27", -27, ''), ("new text -28", -28, ''); > >> +INSERT INTO tt_1 VALUES ("new text -27", -27, ''); > >> +COMMIT; > >> + > >> +BEGIN; > >> +INSERT INTO tt_4 VALUES ("new text -28", -28, ''); > >> +--error ER_DUP_ENTRY > >> +INSERT INTO nt_4 VALUES ("new text -27", -27, ''), ("new text -28", -28, ''); > >> +INSERT INTO tt_1 VALUES ("new text -28", -28, ''); > >> +COMMIT; > >> + > >> --source include/show_binlog_events.inc > >> > >> --echo > >> @@ -683,23 +683,6 @@ ROLLBACK; > >> > >> --source include/show_binlog_events.inc > >> > >> ---echo > >> ---echo > >> ---echo > >> ---echo > >> -let $binlog_start= query_get_value("SHOW MASTER STATUS", Position, 1); > >> ---echo # > >> ---echo #14) "B M T R" with error in M generates in the binlog the "B M T R" entries. > >> ---echo # > >> - > >> -BEGIN; > >> -INSERT INTO tt_4 VALUES ("new text -32", -32, ''); > >> -TRUNCATE TABLE tt_4; > >> -INSERT INTO tt_4 VALUES ("new text -33", -33, ''); > >> -ROLLBACK; > >> - > >> ---source include/show_binlog_events.inc > >> - > >> connection master; > >> sync_slave_with_master; > >> > >> > >> === modified file 'mysql-test/suite/rpl/r/rpl_stm_mixing_engines.result' > >> --- a/mysql-test/suite/rpl/r/rpl_stm_mixing_engines.result 2009-08-26 23:13:03 +0000 > >> +++ b/mysql-test/suite/rpl/r/rpl_stm_mixing_engines.result 2009-08-27 00:09:31 +0000 > >> @@ -652,8 +652,30 @@ master-bin.000001 # Xid # # COMMIT /* XI > >> # There is a bug in the slave that needs to be fixed before enabling > >> # this part of the test. A bug report will be filed referencing this > >> # test case. > >> +BEGIN; > >> +INSERT INTO nt_3 VALUES ("new text -28", -28, ''); > >> +INSERT INTO tt_3 VALUES ("new text -27", -27, ''), ("new text -28", -28, ''); > >> +ERROR 23000: Duplicate entry '-28' for key 'PRIMARY' > >> +INSERT INTO tt_1 VALUES ("new text -27", -27, ''); > >> +COMMIT; > >> +BEGIN; > >> +INSERT INTO tt_4 VALUES ("new text -28", -28, ''); > >> +INSERT INTO nt_4 VALUES ("new text -27", -27, ''), ("new text -28", -28, ''); > >> +ERROR 23000: Duplicate entry '-28' for key 'PRIMARY' > >> +INSERT INTO tt_1 VALUES ("new text -28", -28, ''); > >> +COMMIT; > >> show binlog events from ; > >> Log_name Pos Event_type Server_id End_log_pos Info > >> +master-bin.000001 # Query # # use `test`; INSERT INTO nt_3 VALUES ("new text -28", -28, '') > >> +master-bin.000001 # Query # # BEGIN > >> +master-bin.000001 # Query # # use `test`; INSERT INTO tt_3 VALUES ("new text -27", -27, ''), ("new text -28", -28, '') > >> +master-bin.000001 # Query # # use `test`; INSERT INTO tt_1 VALUES ("new text -27", -27, '') > >> +master-bin.000001 # Xid # # COMMIT /* XID */ > >> +master-bin.000001 # Query # # BEGIN > >> +master-bin.000001 # Query # # use `test`; INSERT INTO tt_4 VALUES ("new text -28", -28, '') > >> +master-bin.000001 # Query # # use `test`; INSERT INTO nt_4 VALUES ("new text -27", -27, ''), ("new text -28", -28, '') > >> +master-bin.000001 # Query # # use `test`; INSERT INTO tt_1 VALUES ("new text -28", -28, '') > >> +master-bin.000001 # Xid # # COMMIT /* XID */ > >> > >> > >> > >> @@ -832,29 +854,6 @@ master-bin.000001 # Query # # use `test` > >> master-bin.000001 # Query # # use `test`; INSERT INTO nt_4 VALUES ("new text -29", -29, ''), ("new text -30", -30, '') > >> master-bin.000001 # Query # # use `test`; INSERT INTO tt_1 VALUES ("new text -31", -31, '') > >> master-bin.000001 # Query # # ROLLBACK > >> - > >> - > >> - > >> - > >> -# > >> -#14) "B M T R" with error in M generates in the binlog the "B M T R" entries. > >> -# > >> -BEGIN; > >> -INSERT INTO tt_4 VALUES ("new text -32", -32, ''); > >> -TRUNCATE TABLE tt_4; > >> -INSERT INTO tt_4 VALUES ("new text -33", -33, ''); > >> -ROLLBACK; > >> -show binlog events from ; > >> -Log_name Pos Event_type Server_id End_log_pos Info > >> -master-bin.000001 # Query # # BEGIN > >> -master-bin.000001 # Query # # use `test`; INSERT INTO tt_4 VALUES ("new text -32", -32, '') > >> -master-bin.000001 # Xid # # COMMIT /* XID */ > >> -master-bin.000001 # Query # # BEGIN > >> -master-bin.000001 # Query # # use `test`; TRUNCATE TABLE tt_4 > >> -master-bin.000001 # Xid # # COMMIT /* XID */ > >> -master-bin.000001 # Query # # BEGIN > >> -master-bin.000001 # Query # # use `test`; INSERT INTO tt_4 VALUES ("new text -33", -33, '') > >> -master-bin.000001 # Xid # # COMMIT /* XID */ > >> ################################################################################### > >> # CLEAN > >> ################################################################################### > >> > >> === modified file 'sql/log_event.cc' > >> --- a/sql/log_event.cc 2009-08-24 09:24:52 +0000 > >> +++ b/sql/log_event.cc 2009-08-27 00:09:31 +0000 > >> @@ -3202,6 +3202,15 @@ Default database: '%s'. Query: '%s'", > >> DBUG_PRINT("info",("error ignored")); > >> clear_all_errors(thd, const_cast(rli)); > >> thd->killed= THD::NOT_KILLED; > >> + /* > >> + When an error is expected and matches the actual error the > >> + slave does not report any error and by consequence changes > >> + on transactional tables are not rolled back in the function > >> + close_thread_tables(). For that reason, we explicitly roll > >> + them back here. > >> + */ > >> + if (expected_error) > >> + ha_autocommit_or_rollback(thd, TRUE); > >> } > >> /* > >> If we expected a non-zero error code and get nothing and, it is a concurrency > >> > >> > > > >