On 13 Feb, 2008, at 00:43, Mats Kindahl wrote:
> 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?
If a current transaction is in progress, there is not a problem
because the counter does track the amount of logged events to
replicate should the transaction be committed.
>
> * 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.
No, because I cannot clear it for every statement or else multi-
statement transactions won't replicate properly. There should only be
2 ways a transaction can end: It can be committed or rolled back.
Regards,
Antony
>
> 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
>
> <mats.vcf>
