List:Commits« Previous MessageNext Message »
From:Daogang Qu Date:December 6 2010 10:13am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3524)
Bug#56662
View as plain text  
2010-12-06 17:29, He Zhenxing wrote:
> Hi Daogang,
>
> Nice work, please look for some comments!
>    
Thanks for your comments. See reply below.
> Dao-Gang.Qu@stripped wrote:
>    
>> #At file:///home/daogang/bzrwork/bug56662/mysql-5.1-bugteam/ based on
> revid:bjorn.munch@stripped
>>
>>   3524 Dao-Gang.Qu@stripped	2010-12-06
>>        Bug #56662  	Assertion failed: next_insert_id == 0, file .\handler.cc
>>
>>        In RBR, sometimes the new value of next_insert_id will be generated
>>        for current row when updating the auto_increment field of a table on
>>        slave side. Which causes the error. This is a incorrect behavior.
>>
>>        After the patch, we never generate a new value for next_insert_id
>>        for current row when updating the auto_increment field of a table
>>        on slave side in RBR.
>>       @ sql/log_event.cc
>>          Added code to not allow generation of auto_increment value when
>>          processing rows event by adding 'MODE_NO_AUTO_VALUE_ON_ZERO' to
>>          sql_mode.
>>
>>      modified:
>>        mysql-test/extra/rpl_tests/rpl_auto_increment.test
>>        mysql-test/suite/rpl/r/rpl_auto_increment.result
>>        sql/log_event.cc
>> === modified file 'mysql-test/extra/rpl_tests/rpl_auto_increment.test'
>> --- a/mysql-test/extra/rpl_tests/rpl_auto_increment.test	2009-09-10 10:05:53
> +0000
>> +++ b/mysql-test/extra/rpl_tests/rpl_auto_increment.test	2010-12-06 05:37:52
> +0000
>> @@ -241,3 +241,29 @@ DROP TABLE t1;
>>   DROP TABLE t2;
>>   SET SQL_MODE='';
>>   sync_slave_with_master;
>> +
>> +#
>> +# BUG#56662
>> +# The test verifies if the assertion of "next_insert_id == 0"
>> +# will fail in ha_external_lock() function.
>> +#
>> +connection master;
>> +CREATE TABLE t1 (id SMALLINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, data
> INT) ENGINE=innodb;
>> +
>> +BEGIN;
>> +--echo # Set sql_mode without NO_AUTO_VALUE_ON_ZERO for
>> +--echo # allowing zero to fill the auto_increment field.
>> +SET SQL_MODE=1400644414;
>>      
> REQUEST: Use SET SQL_MODE='no_auto_value_on_zero';
>
> This makes it clear that only this flag affects the test.
>    
Yes. Updated.
>    
>> +INSERT INTO t1(id,data) VALUES(0,2);
>> +--echo # Resetting sql_mode with NO_AUTO_VALUE_ON_ZERO to
>> +--echo # affect the execution of the transaction on slave.
>> +SET SQL_MODE=1997596415;
>>      
> REQUEST: SET SQL_MODE=0;
>    
Updated.
>    
>> +COMMIT;
>> +SELECT * FROM t1;
>> +sync_slave_with_master;
>> +SELECT * FROM t1;
>> +
>> +connection master;
>> +DROP TABLE t1;
>> +sync_slave_with_master;
>> +
>>
>> === modified file 'mysql-test/suite/rpl/r/rpl_auto_increment.result'
>> --- a/mysql-test/suite/rpl/r/rpl_auto_increment.result	2009-09-10 10:05:53 +0000
>> +++ b/mysql-test/suite/rpl/r/rpl_auto_increment.result	2010-12-06 05:37:52 +0000
>> @@ -312,3 +312,20 @@ Comparing tables master:test.t2 and slav
>>   DROP TABLE t1;
>>   DROP TABLE t2;
>>   SET SQL_MODE='';
>> +CREATE TABLE t1 (id SMALLINT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, data
> INT) ENGINE=innodb;
>> +BEGIN;
>> +# Set sql_mode without NO_AUTO_VALUE_ON_ZERO for
>> +# allowing zero to fill the auto_increment field.
>> +SET SQL_MODE=1400644414;
>> +INSERT INTO t1(id,data) VALUES(0,2);
>> +# Resetting sql_mode with NO_AUTO_VALUE_ON_ZERO to
>> +# affect the execution of the transaction on slave.
>> +SET SQL_MODE=1997596415;
>> +COMMIT;
>> +SELECT * FROM t1;
>> +id	data
>> +0	2
>> +SELECT * FROM t1;
>> +id	data
>> +0	2
>> +DROP TABLE t1;
>>
>> === modified file 'sql/log_event.cc'
>> --- a/sql/log_event.cc	2010-10-23 12:55:44 +0000
>> +++ b/sql/log_event.cc	2010-12-06 05:37:52 +0000
>> @@ -7569,6 +7569,14 @@ int Rows_log_event::do_apply_event(Relay
>>       // Do event specific preparations
>>       error= do_before_row_operations(rli);
>>
>> +    /*
>> +      Bug#56662 Assertion failed: next_insert_id == 0, file handler.cc
>> +      Don't allow generation of auto_increment value when processing
>> +      rows event by adding 'MODE_NO_AUTO_VALUE_ON_ZERO' to sql_mode.
>> +    */
>> +    ulong save_sql_mode= thd->variables.sql_mode;
>>      
> SUGGESTION: I think it not necessary to save and restore the old value.
>    
No. I don't think so. The sql_mode will affect its following transactions
and statements until it is updated. So I think we should save and restore
it so that it can keep consistency with master.

>    
>> +    thd->variables.sql_mode|= MODE_NO_AUTO_VALUE_ON_ZERO;
>> +
>>      
> SUGGESTION: I'd also suggest to use
> thd->variables.sql_mode= MODE_NO_AUTO_VALUE_ON_ZERO;
>
> To make it clear that we only need this flag set when applying row
> events.
>    
If that, it will have different behavior if the sql_mode is added with
'MODE_STRICT_TRANS_TABLES'  and 'MODE_STRICT_ALL_TABLES'.

Best Regards,

Daogang

>    
>>       // row processing loop
>>
>>       while (error == 0&&  m_curr_row<  m_rows_end)
>> @@ -7631,6 +7639,11 @@ int Rows_log_event::do_apply_event(Relay
>>             thd->transaction.stmt.modified_non_trans_table= TRUE;
>>       } // row processing loop
>>
>> +    /*
>> +      Restore the sql_mode after the rows event is processed.
>> +    */
>> +    thd->variables.sql_mode= save_sql_mode;
>> +
>>       DBUG_EXECUTE_IF("STOP_SLAVE_after_first_Rows_event",
>>                       const_cast<Relay_log_info*>(rli)->abort_slave=
> 1;);
>>
>>      
>
>
>
>    

Thread
bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3524) Bug#56662Dao-Gang.Qu6 Dec
  • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3524)Bug#56662He Zhenxing6 Dec
    • Re: bzr commit into mysql-5.1-bugteam branch (Dao-Gang.Qu:3524)Bug#56662Daogang Qu6 Dec