Hi Zhenxing,
Let me start by commenting on the behavior:
1. The BEGIN/COMMIT/ROLLBACK, will no longer be prepended with
"use ...; " since this is explicitly suppressed when building
the query log event.
2. DB filtering rules will not apply anymore to the
BEGIN/COMMIT/ROLLBACK statements.
Given 1. and 2. I started thinking on how this could impact the
existing users, and have not been able to come up with anything
against. I feel that, in a way, this is a difference in behavior
with respect to binlogging that has direct impact on filtering
rules.
Punch line: I agree with your fix.
However, there are some issues still with the proposed patch. I have
run mtr on the patched binary mysqld and there are some tests
failing because some result files are left unchanged. These are, as
expected, related to binlog. Please fix them:
* binlog
* mix_innodb_myisam_binlog
Please, find some comments inline also.
On Wed, 2009-05-20 at 14:00 +0000, 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-20
> 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 sql/log.cc
> M sql/log_event.cc
> === 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-20 13:59:57 +0000
> @@ -0,0 +1,77 @@
> +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=1029;
Maybe mask the master_log_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;
> +Warnings:
> +Warning 1196 Some non-transactional changed tables couldn't be rolled back
Maybe remove the warning from result file (disable_warnings before
query?).
> +start slave until master_log_file='master-bin.000001', master_log_pos=1559;
Mask out master_log_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-20 13:59:57 +0000
> @@ -0,0 +1 @@
> +--replicate-do-db=db1
> \ No newline at end of file
>
> === 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-20 13:59:57 +0000
> @@ -0,0 +1,95 @@
> +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();
> +ROLLBACK;
> +
> +connection slave;
> +
> +#replace_result $master_pos MASTER_POS;
I see you already intended to mask out the master_log_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-20 13:59:57 +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-20 13:59:57 +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))
> {
> thd->set_time((time_t)when);
> thd->query_length= q_len_arg;
>
Regards,
Luís
