Hello, Dao-Gang.
Alfranio and I just have talked to identify the problem.
Taking your patch's regression test (t2 is of T type, t1 of func() is of N)
insert into t2 values (func(),1);
you can see that two Table-map events that are written straight to the stmt and
the trans caches by write_locked_table_maps() when the N-rows event of func() was
being logged through
binlog_log_row() <- handler::ha_write_row()
As the following insert into t2 dup-key errored out in the following,
show binlog events displays the first
table map accompanied by the N-rows event and the 2nd table map as "orphaned".
Evidently Table-map flushing logics needs refinement such that
Table-map(s) is(are) stored to the cache always with at least one Rows-event.
There are few approaches how that can be implemented but anyone
will require a mandatory change in THD::binlog_write_table_map()
to hold on with writing Table-maps into the caches.
I can't come up with a very detailed method by now, still it views to me
that both caches have to keep the created at write_locked_table_maps()
Table-maps marked by some pending state. The pending state is ceased and the maps
are flushed when a first Rows-event is being written to that cache.
The pending state for Table-maps should be cleared in case of an error as well.
You welcome to talk on #rep tomorrow on this matter.
cheers,
Andrei
----- Original Message -----
From: Dao-Gang.Qu@stripped
To: alfranio.correia@stripped
Cc: commits@stripped, Andrei.Elkin@stripped
Sent: Thursday, September 30, 2010 4:08:04 AM GMT +02:00 Athens, Beirut, Bucharest,
Istanbul
Subject: Re: bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3299) Bug#55869
2010-09-30 07:49, Alfranio Correia wrote:
> Hi Daogang,
>
> Apparently there is no problem with your patch.
> However, I prefer a different approach that will help us in the future
> to simply the "if"
> in binlog_rollback. Besides the approach is less risky.
>
> Add a flag to trx_cache in order to track if a non-transactional
> change was written
> to it. Something like set_changes_to_non_trans_temp_table();
>
> If you want we can discuss this tomorrow.
Sure. Thanks!
Best Regards,
Daogang
>
> Cheers.
>
>
> On 09/27/2010 05:04 AM, Dao-Gang.Qu@stripped wrote:
>> #At
>> file:///home/daogangqu/mysql/bzrwork1/bug55869/mysql-next-mr-bugfixing/
>> based on revid:mats.kindahl@stripped
>>
>> 3299 Dao-Gang.Qu@stripped 2010-09-27
>> Bug #55869 In mixed mode, the Table_map_event is binloged
>> without any rows_log_event.
>>
>> In mixed mode, the Table_map_event is binlogged without any
>> rows_log_event
>> when a unsafe single statement transaction corrupted. It's a
>> wrong behavior.
>> The reason is that the Table_map_event will be binlogged with
>> the "ROLLBACK"
>> Query_log_event together for a unsafe single stmt transaction,
>> which has
>> updated non-transactional table regardless of corruption.
>>
>> To fix the problem to not binlog the Table_map_event and
>> "ROLLBACK"
>> Query_log_event for a unsafe corrupted single stmt transaction,
>> although it has updated non-transactional table.
>> @ mysql-test/suite/binlog/r/binlog_trans_log.result
>> Test result for Bug #55869.
>> @ mysql-test/suite/binlog/t/binlog_trans_log.test
>> Added the test to verify if Table_map_event is binlogged
>> without
>> any rows_log_event for a unsafe corrupted single stmt
>> transaction,
>> although it has updated non-transactional table.
>>
>> added:
>> mysql-test/suite/binlog/r/binlog_trans_log.result
>> mysql-test/suite/binlog/t/binlog_trans_log.test
>> modified:
>> sql/binlog.cc
>> === added file 'mysql-test/suite/binlog/r/binlog_trans_log.result'
>> --- a/mysql-test/suite/binlog/r/binlog_trans_log.result 1970-01-01
>> 00:00:00 +0000
>> +++ b/mysql-test/suite/binlog/r/binlog_trans_log.result 2010-09-27
>> 04:04:43 +0000
>> @@ -0,0 +1,26 @@
>> +CREATE TABLE t1 (a int NOT NULL auto_increment primary key)
>> ENGINE=MyISAM|
>> +CREATE TABLE t2 (a int NOT NULL auto_increment, b int, PRIMARY KEY
>> (a)) ENGINE=InnoDB|
>> +insert into t2 values (1,3)|
>> +create function func()
>> +RETURNS int(11)
>> +DETERMINISTIC
>> +begin
>> +insert into t1 values (null);
>> +select count(*) from t1 into @a;
>> +return @a;
>> +end|
>> +reset master|
>> +insert into t2 values (func(),1)|
>> +ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
>> +# The following must show there are events after the query
>> +show binlog events from<binlog_start>|
>> +Log_name Pos Event_type Server_id End_log_pos Info
>> +master-bin.000001 # Query # # BEGIN
>> +master-bin.000001 # Table_map # # table_id: # (test.t1)
>> +master-bin.000001 # Write_rows # # table_id: # flags:
>> STMT_END_F
>> +master-bin.000001 # Query # # COMMIT
>> +select count(*),@a from t1 /* must be 1,1 */|
>> +count(*) @a
>> +1 1
>> +drop table t1,t2;
>> +drop function func;
>>
>> === added file 'mysql-test/suite/binlog/t/binlog_trans_log.test'
>> --- a/mysql-test/suite/binlog/t/binlog_trans_log.test 1970-01-01
>> 00:00:00 +0000
>> +++ b/mysql-test/suite/binlog/t/binlog_trans_log.test 2010-09-27
>> 04:04:43 +0000
>> @@ -0,0 +1,41 @@
>> +#
>> +# Bug #55869
>> +# This test verifies if Table_map_event is binlogged without any
>> +# rows_log_event for a unsafe corrupted single stmt transaction,
>> +# although it has updated non-transactional table.
>> +#
>> +
>> +--source include/have_innodb.inc
>> +--source include/have_binlog_format_mixed.inc
>> +
>> +delimiter |;
>> +
>> +CREATE TABLE t1 (a int NOT NULL auto_increment primary key)
>> ENGINE=MyISAM|
>> +CREATE TABLE t2 (a int NOT NULL auto_increment, b int, PRIMARY KEY
>> (a)) ENGINE=InnoDB|
>> +
>> +insert into t2 values (1,3)|
>> +
>> +create function func()
>> +RETURNS int(11)
>> +DETERMINISTIC
>> +begin
>> + insert into t1 values (null);
>> + select count(*) from t1 into @a;
>> + return @a;
>> +end|
>> +
>> +reset master|
>> +
>> +--error ER_DUP_ENTRY
>> +insert into t2 values (func(),1)|
>> +
>> +--echo # The following must show there are events after the query
>> +--source include/show_binlog_events.inc
>> +
>> +select count(*),@a from t1 /* must be 1,1 */|
>> +delimiter ;|
>> +
>> +# clean-up
>> +drop table t1,t2;
>> +drop function func;
>> +
>>
>> === modified file 'sql/binlog.cc'
>> --- a/sql/binlog.cc 2010-09-17 13:22:22 +0000
>> +++ b/sql/binlog.cc 2010-09-27 04:04:43 +0000
>> @@ -665,7 +665,8 @@ static int binlog_rollback(handlerton *h
>> thd->variables.binlog_format == BINLOG_FORMAT_MIXED) ||
>> (trans_has_updated_non_trans_table(thd)&&
>> ending_single_stmt_trans(thd,all)&&
>> - thd->variables.binlog_format == BINLOG_FORMAT_MIXED)))
>> + thd->variables.binlog_format == BINLOG_FORMAT_MIXED
>> +&& !(thd->is_error()&&
> thd->lex->is_stmt_unsafe()))))
>> {
>> Query_log_event qev(thd, STRING_WITH_LEN("ROLLBACK"), TRUE,
>> FALSE, TRUE, 0);
>> error= binlog_flush_trx_cache(thd, cache_mngr,&qev);
>>
>>
>>
>>
>>
