Hi Antony!
There is basically two items I'm wondering about:
* You are clearing the counter in one path in the rollback case, but
what about the other part where a statement inside a transaction
is rolled back? Isn't there a risk that it will trigger if you
have a duplicate key inside a transaction instead?
* Would it not be safer to clear the counter and the beginning of
the statement instead of at the end of the statement. That way, it
will not be possible to carry it over to the next statement even
if exiting by an unexpected path.
Just my few cents,
Mats Kindahl
antony@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 6.0 repository of antony. When antony 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, 2008-02-12 16:37:27-08:00, acurtis@stripped +5 -0
> Bug#33688
> "Falcon: replication fails on duplicate key error"
> I was able to reproduce this bug on InnoDB storage engine simply by
> removing table flag HA_BINLOG_STMT_CAPABLE.
> Bug affects any transactional storage engine which does not use statement
> based logs and is caused by thd->binlog_table_maps not being reset when
> rollback occurs.
>
> mysql-test/suite/falcon/r/rpl_bug_33688.result@stripped, 2008-02-12 16:37:11-08:00,
> acurtis@stripped +39 -0
> New BitKeeper file ``mysql-test/suite/falcon/r/rpl_bug_33688.result''
>
> mysql-test/suite/falcon/r/rpl_bug_33688.result@stripped, 2008-02-12 16:37:11-08:00,
> acurtis@stripped +0 -0
>
> mysql-test/suite/falcon/t/rpl_bug_33688-slave.opt@stripped, 2008-02-12 16:37:11-08:00,
> acurtis@stripped +1 -0
> New BitKeeper file ``mysql-test/suite/falcon/t/rpl_bug_33688-slave.opt''
>
> mysql-test/suite/falcon/t/rpl_bug_33688-slave.opt@stripped, 2008-02-12 16:37:11-08:00,
> acurtis@stripped +0 -0
>
> mysql-test/suite/falcon/t/rpl_bug_33688.test@stripped, 2008-02-12 16:37:11-08:00,
> acurtis@stripped +47 -0
> New BitKeeper file ``mysql-test/suite/falcon/t/rpl_bug_33688.test''
>
> mysql-test/suite/falcon/t/rpl_bug_33688.test@stripped, 2008-02-12 16:37:11-08:00,
> acurtis@stripped +0 -0
>
> sql/log.cc@stripped, 2008-02-12 16:37:10-08:00, acurtis@stripped +5 -0
> bug33688
> when resetting binlog data, don't forget to reset the
> thd->binlog_table_maps member.
>
> sql/sql_class.h@stripped, 2008-02-12 16:37:10-08:00, acurtis@stripped +3 -0
> bug33688
> new method to reset binlog_table_maps member.
>
> diff -Nrup a/mysql-test/suite/falcon/r/rpl_bug_33688.result
> b/mysql-test/suite/falcon/r/rpl_bug_33688.result
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/suite/falcon/r/rpl_bug_33688.result 2008-02-12 16:37:11 -08:00
> @@ -0,0 +1,39 @@
> +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;
> +#---- Bug 33688 ----
> +set storage_engine='Falcon';
> +create table t1 (a int primary key);
> +create table t4 (a int primary key);
> +insert into t1 values (1),(1);
> +Got one of the listed errors
> +insert into t4 values (1),(2);
> +show tables like 't1';
> +Tables_in_test (t1)
> +show tables like 't4';
> +Tables_in_test (t4)
> +t4
> +SELECT * FROM test.t4 ORDER BY a;
> +a
> +1
> +2
> +insert into t4 values (3);
> +SELECT * FROM test.t4 ORDER BY a;
> +a
> +1
> +2
> +3
> +insert into t1 values (1),(1);
> +Got one of the listed errors
> +insert into t4 values (6);
> +SELECT * FROM test.t4 ORDER BY a;
> +a
> +1
> +2
> +3
> +6
> +drop table t1;
> +drop table t4;
> diff -Nrup a/mysql-test/suite/falcon/t/rpl_bug_33688-slave.opt
> b/mysql-test/suite/falcon/t/rpl_bug_33688-slave.opt
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/suite/falcon/t/rpl_bug_33688-slave.opt 2008-02-12 16:37:11 -08:00
> @@ -0,0 +1 @@
> +--replicate-ignore-table=test.t1 --replicate-ignore-table=test.t2
> --replicate-ignore-table=test.t3
> diff -Nrup a/mysql-test/suite/falcon/t/rpl_bug_33688.test
> b/mysql-test/suite/falcon/t/rpl_bug_33688.test
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/suite/falcon/t/rpl_bug_33688.test 2008-02-12 16:37:11 -08:00
> @@ -0,0 +1,47 @@
> +--source include/have_falcon.inc
> +--source include/master-slave.inc
> +#
> +# Bug #33688: Falcon: replication fails on duplicate key error
> +#
> +--echo #---- Bug 33688 ----
> +connection master;
> +set storage_engine='Falcon';
> +create table t1 (a int primary key);
> +create table t4 (a int primary key);
> +# generate an error that goes to the binlog
> +--error 1022, ER_DUP_ENTRY
> +insert into t1 values (1),(1);
> +insert into t4 values (1),(2);
> +save_master_pos;
> +connection slave;
> +# as the t1 table is ignored on the slave, the slave should be able to sync
> +sync_with_master;
> +# check that the table has been ignored, because otherwise the test is nonsense
> +show tables like 't1';
> +show tables like 't4';
> +SELECT * FROM test.t4 ORDER BY a;
> +
> +connection master;
> +insert into t4 values (3);
> +save_master_pos;
> +
> +connection slave;
> +sync_with_master;
> +SELECT * FROM test.t4 ORDER BY a;
> +
> +connection master;
> +--error 1022, ER_DUP_ENTRY
> +insert into t1 values (1),(1);
> +insert into t4 values (6);
> +save_master_pos;
> +
> +connection slave;
> +sync_with_master;
> +SELECT * FROM test.t4 ORDER BY a;
> +
> +connection master;
> +drop table t1;
> +drop table t4;
> +save_master_pos;
> +connection slave;
> +sync_with_master;
> diff -Nrup a/sql/log.cc b/sql/log.cc
> --- a/sql/log.cc 2007-12-02 12:17:08 -08:00
> +++ b/sql/log.cc 2008-02-12 16:37:10 -08:00
> @@ -1376,7 +1376,12 @@ binlog_end_trans(THD *thd, binlog_trx_da
> transaction cache to remove the statement.
> */
> if (all || !(thd->options & (OPTION_BEGIN | OPTION_NOT_AUTOCOMMIT)))
> + {
> trx_data->reset();
> +
> + DBUG_ASSERT(!thd->binlog_get_pending_rows_event());
> + thd->clear_binlog_table_maps();
> + }
> else // ...statement
> trx_data->truncate(trx_data->before_stmt_pos);
>
> diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
> --- a/sql/sql_class.h 2007-12-18 09:26:46 -08:00
> +++ b/sql/sql_class.h 2008-02-12 16:37:10 -08:00
> @@ -1182,6 +1182,9 @@ public:
> uint get_binlog_table_maps() const {
> return binlog_table_maps;
> }
> + void clear_binlog_table_maps() {
> + binlog_table_maps= 0;
> + }
> #endif /* MYSQL_CLIENT */
>
> public:
>
>
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
