Hi Jasonh,
Patch approved.
Find just a few comments in-line.
Cheers.
He Zhenxing wrote:
> #At file:///media/sdb2/hezx/work/mysql/bzrwork/b43263/5.0-bugteam/ based on
> revid:pstoev@stripped
>
> 2741 He Zhenxing 2009-05-25
> BUG#43263 BEGIN skipped in some replicate-do-db cases
>
> BEGIN/COMMIT/ROLLBACK was subject to replication db rules, and
> caused the boundary of a transaction not recognized correctly
> when these queries were ignored by the rules.
>
> Fixed the problem by skipping replication db rules for these
> statements.
> @ sql/log_event.cc
> Skip checking replication db rules for BEGIN/COMMIT/ROLLBACK statements
>
> A mysql-test/r/rpl_begin_commit_rollback.result
> A mysql-test/t/rpl_begin_commit_rollback-slave.opt
> A mysql-test/t/rpl_begin_commit_rollback.test
> M mysql-test/r/binlog.result
> M mysql-test/r/mix_innodb_myisam_binlog.result
> M sql/log.cc
> M sql/log_event.cc
> === modified file 'mysql-test/r/binlog.result'
> --- a/mysql-test/r/binlog.result 2009-03-25 16:48:10 +0000
> +++ b/mysql-test/r/binlog.result 2009-05-25 03:54:09 +0000
> @@ -12,10 +12,10 @@ show binlog events from 98;
> Log_name Pos Event_type Server_id End_log_pos Info
> master-bin.000001 # Query 1 # use `test`; create table t1 (a int) engine=innodb
> master-bin.000001 # Query 1 # use `test`; create table t2 (a int) engine=innodb
> -master-bin.000001 # Query 1 # use `test`; BEGIN
> +master-bin.000001 # Query 1 # BEGIN
> master-bin.000001 # Query 1 # use `test`; insert t1 values (5)
> master-bin.000001 # Xid 1 # COMMIT /* XID */
> -master-bin.000001 # Query 1 # use `test`; BEGIN
> +master-bin.000001 # Query 1 # BEGIN
> master-bin.000001 # Query 1 # use `test`; insert t2 values (5)
> master-bin.000001 # Xid 1 # COMMIT /* XID */
> drop table t1,t2;
> @@ -27,7 +27,7 @@ drop table t1;
> show binlog events in 'master-bin.000001' from 98;
> Log_name Pos Event_type Server_id End_log_pos Info
> master-bin.000001 # Query 1 # use `test`; create table t1 (n int) engine=innodb
> -master-bin.000001 # Query 1 # use `test`; BEGIN
> +master-bin.000001 # Query 1 # BEGIN
> master-bin.000001 # Query 1 # use `test`; insert into t1 values(100 + 4)
> master-bin.000001 # Query 1 # use `test`; insert into t1 values(99 + 4)
> master-bin.000001 # Query 1 # use `test`; insert into t1 values(98 + 4)
> @@ -147,7 +147,7 @@ show binlog events from 0;
> Log_name Pos Event_type Server_id End_log_pos Info
> master-bin.000001 4 Format_desc 1 98 Server version, Binlog ver: 4
> master-bin.000001 98 Query 1 197 use `test`; create table t1(n int) engine=innodb
> -master-bin.000001 197 Query 1 265 use `test`; BEGIN
> +master-bin.000001 197 Query 1 265 BEGIN
> master-bin.000001 265 Query 1 353 use `test`; insert into t1 values (1)
> master-bin.000001 353 Query 1 441 use `test`; insert into t1 values (2)
> master-bin.000001 441 Query 1 529 use `test`; insert into t1 values (3)
> @@ -161,7 +161,7 @@ show binlog events from 0;
> Log_name Pos Event_type Server_id End_log_pos Info
> master-bin.000001 4 Format_desc 1 98 Server version, Binlog ver: 4
> master-bin.000001 98 Query 1 198 use `test`; create table t1 (a int) engine=innodb
> -master-bin.000001 198 Query 1 266 use `test`; BEGIN
> +master-bin.000001 198 Query 1 266 BEGIN
> master-bin.000001 266 Query 1 357 use `test`; insert into t1 values( 400 )
> master-bin.000001 357 Query 1 448 use `test`; insert into t1 values( 399 )
> master-bin.000001 448 Query 1 539 use `test`; insert into t1 values( 398 )
>
> === modified file 'mysql-test/r/mix_innodb_myisam_binlog.result'
> --- a/mysql-test/r/mix_innodb_myisam_binlog.result 2008-03-27 05:42:34 +0000
> +++ b/mysql-test/r/mix_innodb_myisam_binlog.result 2009-05-25 03:54:09 +0000
> @@ -8,7 +8,7 @@ insert into t2 select * from t1;
> commit;
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(1)
> master-bin.000001 # Query # # use `test`; insert into t2 select * from t1
> master-bin.000001 # Xid # # COMMIT /* XID */
> @@ -23,10 +23,10 @@ Warnings:
> Warning 1196 Some non-transactional changed tables couldn't be rolled back
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(2)
> master-bin.000001 # Query # # use `test`; insert into t2 select * from t1
> -master-bin.000001 # Query # # use `test`; ROLLBACK
> +master-bin.000001 # Query # # ROLLBACK
> delete from t1;
> delete from t2;
> reset master;
> @@ -41,7 +41,7 @@ Warning 1196 Some non-transactional chan
> commit;
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(3)
> master-bin.000001 # Query # # use `test`; savepoint my_savepoint
> master-bin.000001 # Query # # use `test`; insert into t1 values(4)
> @@ -67,7 +67,7 @@ a
> 7
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(5)
> master-bin.000001 # Query # # use `test`; savepoint my_savepoint
> master-bin.000001 # Query # # use `test`; insert into t1 values(6)
> @@ -89,10 +89,10 @@ get_lock("a",10)
> 1
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(8)
> master-bin.000001 # Query # # use `test`; insert into t2 select * from t1
> -master-bin.000001 # Query # # use `test`; ROLLBACK
> +master-bin.000001 # Query # # ROLLBACK
> delete from t1;
> delete from t2;
> reset master;
> @@ -100,7 +100,7 @@ insert into t1 values(9);
> insert into t2 select * from t1;
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(9)
> master-bin.000001 # Xid # # COMMIT /* XID */
> master-bin.000001 # Query # # use `test`; insert into t2 select * from t1
> @@ -112,7 +112,7 @@ begin;
> insert into t2 select * from t1;
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(10)
> master-bin.000001 # Xid # # COMMIT /* XID */
> master-bin.000001 # Query # # use `test`; insert into t2 select * from t1
> @@ -120,11 +120,11 @@ insert into t1 values(11);
> commit;
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(10)
> master-bin.000001 # Xid # # COMMIT /* XID */
> master-bin.000001 # Query # # use `test`; insert into t2 select * from t1
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(11)
> master-bin.000001 # Xid # # COMMIT /* XID */
> alter table t2 engine=INNODB;
> @@ -137,7 +137,7 @@ insert into t2 select * from t1;
> commit;
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(12)
> master-bin.000001 # Query # # use `test`; insert into t2 select * from t1
> master-bin.000001 # Xid # # COMMIT /* XID */
> @@ -162,7 +162,7 @@ rollback to savepoint my_savepoint;
> commit;
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(14)
> master-bin.000001 # Xid # # COMMIT /* XID */
> delete from t1;
> @@ -182,7 +182,7 @@ a
> 18
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(16)
> master-bin.000001 # Query # # use `test`; insert into t1 values(18)
> master-bin.000001 # Xid # # COMMIT /* XID */
> @@ -234,24 +234,24 @@ get_lock("lock1",60)
> 1
> show binlog events from <binlog_start>;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values(16)
> master-bin.000001 # Query # # use `test`; insert into t1 values(18)
> master-bin.000001 # Xid # # COMMIT /* XID */
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; delete from t1
> master-bin.000001 # Xid # # COMMIT /* XID */
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; delete from t2
> master-bin.000001 # Xid # # COMMIT /* XID */
> master-bin.000001 # Query # # use `test`; alter table t2 type=MyISAM
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into t1 values (1)
> master-bin.000001 # Xid # # COMMIT /* XID */
> master-bin.000001 # Query # # use `test`; insert into t2 values (20)
> master-bin.000001 # Query # # use `test`; drop table t1,t2
> master-bin.000001 # Query # # use `test`; create temporary table ti (a int)
> engine=innodb
> -master-bin.000001 # Query # # use `test`; BEGIN
> +master-bin.000001 # Query # # BEGIN
> master-bin.000001 # Query # # use `test`; insert into ti values(1)
> master-bin.000001 # Xid # # COMMIT /* XID */
> master-bin.000001 # Query # # use `test`; create temporary table t1 (a int)
> engine=myisam
> @@ -310,11 +310,11 @@ File Position Binlog_Do_DB Binlog_Ignore
> master-bin.000001 507
> show binlog events from 98;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query 1 # use `test`; BEGIN
> +master-bin.000001 # Query 1 # BEGIN
> master-bin.000001 # Query 1 # use `test`; insert into ti values (1)
> master-bin.000001 # Query 1 # use `test`; insert into ti values (2)
> master-bin.000001 # Query 1 # use `test`; insert into tt select * from ti
> -master-bin.000001 # Query 1 # use `test`; ROLLBACK
> +master-bin.000001 # Query 1 # ROLLBACK
> select count(*) from ti /* zero */;
> count(*)
> 0
> @@ -342,11 +342,11 @@ File Position Binlog_Do_DB Binlog_Ignore
> master-bin.000001 581
> show binlog events from 98;
> Log_name Pos Event_type Server_id End_log_pos Info
> -master-bin.000001 # Query 1 # use `test`; BEGIN
> +master-bin.000001 # Query 1 # BEGIN
> master-bin.000001 # Query 1 # use `test`; insert into ti values (1)
> master-bin.000001 # Query 1 # use `test`; insert into ti values (2) /* to make the
> dup error in the following */
> master-bin.000001 # Query 1 # use `test`; insert into tt select * from ti /* one
> affected and error */
> -master-bin.000001 # Query 1 # use `test`; ROLLBACK
> +master-bin.000001 # Query 1 # ROLLBACK
> select count(*) from ti /* zero */;
> count(*)
> 0
>
> === added file 'mysql-test/r/rpl_begin_commit_rollback.result'
> --- a/mysql-test/r/rpl_begin_commit_rollback.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/r/rpl_begin_commit_rollback.result 2009-05-25 03:54:09 +0000
> @@ -0,0 +1,75 @@
> +stop slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +reset master;
> +reset slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +start slave;
> +DROP DATABASE IF EXISTS db1;
> +CREATE DATABASE db1;
> +use db1;
> +CREATE TABLE db1.t1 (a INT) ENGINE=InnoDB;
> +CREATE TABLE db1.t2 (a INT) ENGINE=MyISAM;
> +include/stop_slave.inc
> +CREATE PROCEDURE db1.p1 ()
> +BEGIN
> +INSERT INTO t1 VALUES (1);
> +INSERT INTO t1 VALUES (2);
> +INSERT INTO t1 VALUES (3);
> +INSERT INTO t1 VALUES (4);
> +INSERT INTO t1 VALUES (5);
> +END//
> +CREATE PROCEDURE db1.p2 ()
> +BEGIN
> +INSERT INTO t1 VALUES (6);
> +INSERT INTO t1 VALUES (7);
> +INSERT INTO t1 VALUES (8);
> +INSERT INTO t1 VALUES (9);
> +INSERT INTO t1 VALUES (10);
> +INSERT INTO t2 VALUES (1);
> +END//
> +use test;
> +BEGIN;
> +CALL db1.p1();
> +COMMIT;
> +SELECT * FROM db1.t1;
> +a
> +1
> +2
> +3
> +4
> +5
> +start slave until master_log_file='master-bin.000001', master_log_pos=MASTER_POS;
> +#
> +# If we got no rows here, then we're suffering BUG#43263
> +#
> +SELECT * FROM db1.t1;
> +a
> +1
> +2
> +3
> +4
> +5
> +BEGIN;
> +CALL db1.p2();
> +ROLLBACK;
> +start slave until master_log_file='master-bin.000001', master_log_pos=MASTER_POS;
> +#
> +# If we got no rows here, then we're suffering BUG#43263
> +#
> +SELECT * FROM db1.t1;
> +a
> +1
> +2
> +3
> +4
> +5
> +6
> +7
> +8
> +9
> +10
> +#
> +# Clean up
> +#
> +DROP DATABASE db1;
> +DROP DATABASE db1;
>
> === added file 'mysql-test/t/rpl_begin_commit_rollback-slave.opt'
> --- a/mysql-test/t/rpl_begin_commit_rollback-slave.opt 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/t/rpl_begin_commit_rollback-slave.opt 2009-05-25 03:54:09 +0000
> @@ -0,0 +1 @@
> +--replicate-do-db=db1
>
> === added file 'mysql-test/t/rpl_begin_commit_rollback.test'
> --- a/mysql-test/t/rpl_begin_commit_rollback.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/t/rpl_begin_commit_rollback.test 2009-05-25 03:54:09 +0000
> @@ -0,0 +1,97 @@
> +source include/master-slave.inc;
> +source include/have_innodb.inc;
> +
> +disable_warnings;
> +DROP DATABASE IF EXISTS db1;
> +enable_warnings;
> +
> +CREATE DATABASE db1;
> +
> +use db1;
> +
> +CREATE TABLE db1.t1 (a INT) ENGINE=InnoDB;
> +CREATE TABLE db1.t2 (a INT) ENGINE=MyISAM;
> +
> +sync_slave_with_master;
> +source include/stop_slave.inc;
> +connection master;
> +
> +DELIMITER //;
> +CREATE PROCEDURE db1.p1 ()
> +BEGIN
> + INSERT INTO t1 VALUES (1);
> + INSERT INTO t1 VALUES (2);
> + INSERT INTO t1 VALUES (3);
> + INSERT INTO t1 VALUES (4);
> + INSERT INTO t1 VALUES (5);
> +END//
> +
> +CREATE PROCEDURE db1.p2 ()
> +BEGIN
> + INSERT INTO t1 VALUES (6);
> + INSERT INTO t1 VALUES (7);
> + INSERT INTO t1 VALUES (8);
> + INSERT INTO t1 VALUES (9);
> + INSERT INTO t1 VALUES (10);
> + INSERT INTO t2 VALUES (1);
> +END//
> +DELIMITER ;//
> +
> +# Note: the master_log_pos is set to be the position of the BEGIN + 1,
> +# so before fix of BUG#43263 if the BEGIN is ignored, then all the
> +# INSERTS in p1 will be replicated in AUTOCOMMIT=1 mode and the slave
> +# SQL thread will stop right before the first INSERT. After fix of
> +# BUG#43263, BEGIN will not be ignored by the replication db rules,
> +# and then the whole transaction will be executed before slave SQL
> +# stop.
> +let $master_pos= query_get_value(SHOW MASTER STATUS, Position, 1);
> +let $master_pos= `SELECT $master_pos + 1`;
> +
> +use test;
> +BEGIN;
> +CALL db1.p1();
> +COMMIT;
> +
> +SELECT * FROM db1.t1;
> +
> +connection slave;
> +
> +replace_result $master_pos MASTER_POS;
> +eval start slave until master_log_file='master-bin.000001',
> master_log_pos=$master_pos;
> +source include/wait_for_slave_sql_to_stop.inc;
> +
> +--echo #
> +--echo # If we got no rows here, then we're suffering BUG#43263
> +--echo #
> +SELECT * FROM db1.t1;
> +
> +connection master;
> +
> +# See comments above.
> +let $master_pos= query_get_value(SHOW MASTER STATUS, Position, 1);
> +let $master_pos= `SELECT $master_pos + 1`;
> +
> +BEGIN;
> +CALL db1.p2();
> +disable_warnings;
> +ROLLBACK;
> +enable_warnings;
>
And just CALL db1.p1() and CALL db1.p1() without an explicit transaction?
> +
> +connection slave;
> +
> +replace_result $master_pos MASTER_POS;
> +eval start slave until master_log_file='master-bin.000001',
> master_log_pos=$master_pos;
> +source include/wait_for_slave_sql_to_stop.inc;
> +
> +--echo #
> +--echo # If we got no rows here, then we're suffering BUG#43263
> +--echo #
> +SELECT * FROM db1.t1;
> +
> +--echo #
> +--echo # Clean up
> +--echo #
> +connection master;
> +DROP DATABASE db1;
> +connection slave;
> +DROP DATABASE db1;
>
> === modified file 'sql/log.cc'
> --- a/sql/log.cc 2009-03-27 05:19:50 +0000
> +++ b/sql/log.cc 2009-05-25 03:54:09 +0000
> @@ -159,7 +159,7 @@ static int binlog_commit(THD *thd, bool
> if (all || !(thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)))
> {
> Query_log_event qev(thd, STRING_WITH_LEN("COMMIT"),
> - TRUE, FALSE, THD::KILLED_NO_VALUE);
> + TRUE, TRUE, THD::KILLED_NO_VALUE);
> qev.error_code= 0; // see comment in MYSQL_LOG::write(THD, IO_CACHE)
> DBUG_RETURN(binlog_end_trans(thd, trans_log, &qev));
> }
> @@ -204,7 +204,7 @@ static int binlog_rollback(THD *thd, boo
> if (unlikely(thd->transaction.all.modified_non_trans_table))
> {
> Query_log_event qev(thd, STRING_WITH_LEN("ROLLBACK"),
> - TRUE, FALSE, THD::KILLED_NO_VALUE);
> + TRUE, TRUE, THD::KILLED_NO_VALUE);
> qev.error_code= 0; // see comment in MYSQL_LOG::write(THD, IO_CACHE)
> error= binlog_end_trans(thd, trans_log, &qev);
> }
> @@ -2094,7 +2094,7 @@ bool MYSQL_LOG::write(THD *thd, IO_CACHE
> statement in autocommit mode.
> */
> Query_log_event qinfo(thd, STRING_WITH_LEN("BEGIN"),
> - TRUE, FALSE, THD::KILLED_NO_VALUE);
> + TRUE, TRUE, THD::KILLED_NO_VALUE);
> /*
> Imagine this is rollback due to net timeout, after all
> statements of the transaction succeeded. Then we want a
>
> === modified file 'sql/log_event.cc'
> --- a/sql/log_event.cc 2009-03-27 05:19:50 +0000
> +++ b/sql/log_event.cc 2009-05-25 03:54:09 +0000
> @@ -1953,7 +1953,10 @@ int Query_log_event::exec_event(struct s
> ::exec_event(), then the companion SET also have so we
> don't need to reset_one_shot_variables().
> */
> - if (db_ok(thd->db, replicate_do_db, replicate_ignore_db))
> + if (!strncmp(query_arg, "BEGIN", q_len_arg) ||
> + !strncmp(query_arg, "COMMIT", q_len_arg) ||
> + !strncmp(query_arg, "ROLLBACK", q_len_arg) ||
> + db_ok(thd->db, replicate_do_db, replicate_ignore_db))
>
Either the flag or the comparison.
It is up to you. I don't have a strong opinion.
> {
> thd->set_time((time_t)when);
> thd->query_length= q_len_arg;
>
>
> ------------------------------------------------------------------------
>
>
>
