List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:August 16 2009 1:18pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)
WL#2687
View as plain text  
Hi Jasonh,

Thank you for your comments.
I think I've addressed all your requests.

Cheers-

He Zhenxing wrote:
> Hi Alfranio,
>
> I think you're right, then basically I have not other comments, I think
> the patch is OK to push if nothing new comes up during our meeting
> today. Thank you for the great work!
>
> Alfranio Correia wrote:
>   
>> He Zhenxing wrote:
>>     
>>> Hi Alfranio,
>>>
>>> Great work! 
>>>
>>> I think the patch is almost OK, I have one proposal about M statement,
>>> currently we consider all statement that referenced both N-table and
>>> T-table as unsafe, I think there need a refinement, we should only
>>> consider tables that are modified within the transaction before this
>>> statement or will be modified by this statement, and ignore tables that
>>> are not modified within the transaction before and are not for update.
>>>
>>> Let's assume 't' as a trans table, and 'n' as a non-trans table, so the
>>> following INSERT .. SELECT statements will be considered an N or T
>>> statement, not an M-statement, and so should not generate unsafe errors
>>> nor switch to row format in mixed mode:
>>>  BEGIN;
>>>  INSERT INTO n SELECT * FROM t; /* This is an N statement, safe */
>>>  COMMIT;
>>>
>>>  BEGIN;
>>>  INSERT INTO t SELECT * FROM n; /* This is an T statemet, safe */
>>>  COMMIT;
>>>
>>> But this INSERT .. SELECT is considered as an M-statement:
>>>  BEGIN;
>>>  INSERT INTO t VALUES (1);
>>>  INSERT INTO n SELECT * FROM t; /* This is an M statement, unsafe */
>>>  COMMIT;
>>>
>>>  BEGIN;
>>>  INSERT INTO n VALUES (1);
>>>  INSERT INTO t SELECT * FROM n; /* Because the above statement will be logged
> separately, so this is a *T* statement, safe */
>>>  COMMIT;
>>>
>>> In nuts shell, if a table is not for update, and it has not been
>>> modified within the transaction before this statement, then this table
>>> will be ignored when deciding if the statement is N,T or M.
>>>
>>> Please see also comments below!
>>>   
>>>       
>> Hi Jasonh,
>>
>> I've handled some of the things you requested in your comments below,
>> but I will reply again as soon as I handle everything. I just want to
>> give you
>> a feedback on your proposal on how to classify a statement as unsafe.
>>
>> I put Andrei in CC and if you want we can discuss your idea tomorrow.
>>
>> First, recall that an unsafe statement will have rows written into the
>> binary log
>> and that the mixed mode should be equivalent to the row mode in the sense
>> that it should produce the same result under any circumstance.
>>
>> Said that, let's analyze the following case:
>>
>> T1, T2 are transactions executed on behalf of different connections.
>>
>> 1 - Populating t:
>>
>> CREATE TABLE t(a int, PRIMARY KEY(a));
>> T1: INSERT INTO t VALUES (1);
>> T1: INSERT INTO t VALUES (100);
>> T1: INSERT INTO t VALUES (200);
>> T1: INSERT INTO t VALUES (300);
>>
>>
>> T1: BEGIN
>> T1: INSERT INTO n SELECT * FROM t;
>> T2: INSERT INTO t VALUES (250);
>> T1: INSERT INTO t VALUES (50);
>> T1: COMMIT;
>>
>> The point is that the concurrent statements may be written into the
>> binary in way different from the execution.
>> For instance,
>>
>> BINARY LOG:
>>
>> INSERT INTO t VALUES (250);         
>> INSERT INTO n SELECT * FROM t;
>> ....
>>
>> or
>>
>> BINARY LOG:
>>
>> INSERT INTO n SELECT * FROM t;
>> INSERT INTO t VALUES (250);         
>> ....
>>
>> Is this possible right? If it is not, please, tell me why.
>> So, assuming that it is possible, the slave and the master may diverge
>> if we adopt your proposal.
>> By using the current implementation which classifies the  INSERT INTO n
>> SELECT * FROM t
>> as unsafe, we would be propagating rows and as such the ordering would
>> not matter.
>>
>> I also would like to hear from you if there would be any counterexample
>> where the mixed mode
>> would not be safe.
>>
>> Cheers.
>>
>>     
>>> Alfranio Correia wrote:
>>>   
>>>       
>>>> #At
> file:///home/acorreia/workspace.sun/repository.mysql/bzrwork/bug-40278/mysql-5.1-bugteam/
> based on revid:staale.smedseng@stripped
>>>>
>>>>  2911 Alfranio Correia	2009-06-06
>>>>       WL#2687
>>>>
>>>>     removed:
>>>>       mysql-test/suite/rpl/r/rpl_row_basic_11bugs-master.opt
>>>>       mysql-test/suite/rpl/r/rpl_row_basic_11bugs-slave.opt
>>>>     added:
>>>>       mysql-test/suite/rpl/include/rpl_mixed_engines.inc
>>>>       mysql-test/suite/rpl/r/rpl_mixing_mixed_engines.result
>>>>       mysql-test/suite/rpl/r/rpl_mixing_row_engines.result
>>>>       mysql-test/suite/rpl/r/rpl_mixing_stmt_engines.result
>>>>       mysql-test/suite/rpl/t/rpl_mixing_mixed_engines.test
>>>>       mysql-test/suite/rpl/t/rpl_mixing_row_engines.test
>>>>       mysql-test/suite/rpl/t/rpl_mixing_stmt_engines.test
>>>>         
--------
>>>>     
>>>>         
>>> I think the convention is to put the mode name (row,stm,mixed) right
>>> after the 'rpl_' prefix in the test names, so I'd suggest to name these
>>> as:
>>>   rpl_row_mixing_engines.test
>>>   rpl_stm_mixing_engines.test
>>>   rpl_mixed_mixing_engines.test
>>>       
--------
done.

>>>   
>>>       
>>>>     modified:
>>>>       mysql-test/extra/rpl_tests/rpl_flsh_tbls.test
>>>>       mysql-test/extra/rpl_tests/rpl_insert_delayed.test
>>>>       mysql-test/extra/rpl_tests/rpl_insert_ignore.test
>>>>       mysql-test/extra/rpl_tests/rpl_log.test
>>>>       mysql-test/r/commit_1innodb.result
>>>>       mysql-test/suite/binlog/r/binlog_database.result
>>>>       mysql-test/suite/binlog/r/binlog_multi_engine.result
>>>>       mysql-test/suite/binlog/r/binlog_row_binlog.result
>>>>       mysql-test/suite/binlog/r/binlog_row_ctype_ucs.result
>>>>       mysql-test/suite/binlog/r/binlog_row_insert_select.result
>>>>       mysql-test/suite/binlog/r/binlog_row_mix_innodb_myisam.result
>>>>       mysql-test/suite/binlog/r/binlog_stm_binlog.result
>>>>       mysql-test/suite/binlog/r/binlog_stm_mix_innodb_myisam.result
>>>>       mysql-test/suite/binlog/r/binlog_stm_row.result
>>>>       mysql-test/suite/binlog/t/binlog_incident.test
>>>>       mysql-test/suite/ndb/r/ndb_binlog_format.result
>>>>       mysql-test/suite/rpl/r/rpl_blackhole.result
>>>>       mysql-test/suite/rpl/r/rpl_extraCol_myisam.result
>>>>       mysql-test/suite/rpl/r/rpl_innodb.result
>>>>       mysql-test/suite/rpl/r/rpl_loaddata_fatal.result
>>>>       mysql-test/suite/rpl/r/rpl_log_pos.result
>>>>       mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result
>>>>       mysql-test/suite/rpl/r/rpl_row_basic_11bugs.result
>>>>       mysql-test/suite/rpl/r/rpl_row_conflicts.result
>>>>       mysql-test/suite/rpl/r/rpl_row_create_table.result
>>>>       mysql-test/suite/rpl/r/rpl_row_flsh_tbls.result
>>>>       mysql-test/suite/rpl/r/rpl_row_insert_delayed.result
>>>>       mysql-test/suite/rpl/r/rpl_row_log.result
>>>>       mysql-test/suite/rpl/r/rpl_row_log_innodb.result
>>>>       mysql-test/suite/rpl/r/rpl_sp.result
>>>>       mysql-test/suite/rpl/r/rpl_stm_flsh_tbls.result
>>>>       mysql-test/suite/rpl/r/rpl_stm_insert_delayed.result
>>>>       mysql-test/suite/rpl/r/rpl_stm_log.result
>>>>       mysql-test/suite/rpl/t/rpl_blackhole.test
>>>>       mysql-test/suite/rpl/t/rpl_empty_master_crash.test
>>>>       mysql-test/suite/rpl/t/rpl_innodb.test
>>>>       mysql-test/suite/rpl/t/rpl_invoked_features.test
>>>>       mysql-test/suite/rpl/t/rpl_loaddata_fatal.test
>>>>       mysql-test/suite/rpl/t/rpl_log_pos.test
>>>>       mysql-test/suite/rpl/t/rpl_read_only.test
>>>>       mysql-test/suite/rpl/t/rpl_row_basic_11bugs.test
>>>>       mysql-test/suite/rpl/t/rpl_row_flsh_tbls.test
>>>>       mysql-test/suite/rpl/t/rpl_row_until.test
>>>>       mysql-test/suite/rpl/t/rpl_stm_flsh_tbls.test
>>>>       mysql-test/suite/rpl/t/rpl_trigger.test
>>>>      
> mysql-test/suite/rpl_ndb/r/rpl_ndb_mixed_engines_transactions.result
>>>>       sql/ha_ndbcluster_binlog.cc
>>>>       sql/handler.cc
>>>>       sql/handler.h
>>>>       sql/log.cc
>>>>       sql/log.h
>>>>       sql/log_event.cc
>>>>       sql/log_event.h
>>>>       sql/log_event_old.cc
>>>>       sql/rpl_injector.cc
>>>>       sql/set_var.cc
>>>>       sql/sp.cc
>>>>       sql/sp_head.cc
>>>>       sql/sql_acl.cc
>>>>       sql/sql_base.cc
>>>>       sql/sql_class.cc
>>>>       sql/sql_class.h
>>>>       sql/sql_delete.cc
>>>>       sql/sql_insert.cc
>>>>       sql/sql_load.cc
>>>>       sql/sql_parse.cc
>>>>       sql/sql_table.cc
>>>>       sql/sql_update.cc
>>>>       sql/sql_view.cc
>>>> === modified file 'mysql-test/extra/rpl_tests/rpl_flsh_tbls.test'
>>>> --- a/mysql-test/extra/rpl_tests/rpl_flsh_tbls.test	2008-12-12 11:34:18
> +0000
>>>> +++ b/mysql-test/extra/rpl_tests/rpl_flsh_tbls.test	2009-06-06 17:51:51
> +0000
>>>> @@ -20,19 +20,13 @@ rename table t1 to t5, t2 to t1;
>>>>  # first don't write it to the binlog, to test the NO_WRITE_TO_BINLOG
> keyword.
>>>>  flush no_write_to_binlog tables;
>>>>  # Check that it's not in the binlog.
>>>> ---replace_result $SERVER_VERSION SERVER_VERSION
>>>> ---replace_column 2 # 5 #
>>>> ---replace_regex /table_id: [0-9]+/table_id: #/
>>>> -eval SHOW BINLOG EVENTS FROM $rename_event_pos ;
>>>> +source include/show_binlog_events2.inc;
>>>>         
--------
>>>>     
>>>>         
>>> I'd suggest to use:
>>>   let $pos= query_get_value("SHOW MASTER STATUS", Position, 1);
>>> to get the desired position.
>>>
>>> [snip]
>>>       
--------

Done.

--------
>>>   
>>>       
>>>> @@ -430,7 +404,7 @@ select
>>>>  @a not like "%#%error_code=%error_code=%";
>>>>  @a like "%#%error_code=0%ROLLBACK\n/*!*/;%ROLLBACK /* added by
> mysqlbinlog */;%" OR
>>>>  @a like "%#%error_code=0%ROLLBACK\r\n/*!*/;%ROLLBACK /* added by
> mysqlbinlog */;%"	@a not like "%#%error_code=%error_code=%"
>>>> -1	1
>>>> +0	1
>>>>     
>>>>         
>>> This looks like a test problem, below is some codes in the test case
>>> before the above output:
>>>
>>> if (`select @@binlog_format = 'ROW'`)                                        
>                                                                                
>>> {                                                                            
>                                                                                
>>>   --exec $MYSQL_BINLOG --start-position=524 $MYSQLD_DATADIR/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/mix_innodb_myisam_binlog.output                        
>>> }                                                                            
>                                                                                
>>>                                                                              
>                                                                                
>>> if (`select @@binlog_format = 'STATEMENT' || @@binlog_format = 'MIXED'`)     
>                                                                                
>>> {                                                                            
>                                                                                
>>>   --exec $MYSQL_BINLOG --start-position=555 $MYSQLD_DATADIR/master-bin.000001
> > $MYSQLTEST_VARDIR/tmp/mix_innodb_myisam_binlog.output                        
>>> }          
>>>
>>> this part of the test used absolute binlog positions, and that can be
>>> problematic, probably the binlog positions should be changed
>>> accordingly, please double check that the test case still works as
>>> expected.
>>>
>>> [snip]
>>>       
--------

Done.

>>>   
>>>       
>>>> === added file 'mysql-test/suite/rpl/include/rpl_mixed_engines.inc'
>>>> --- a/mysql-test/suite/rpl/include/rpl_mixed_engines.inc	1970-01-01
> 00:00:00 +0000
>>>> +++ b/mysql-test/suite/rpl/include/rpl_mixed_engines.inc	2009-06-06
> 17:51:51 +0000
>>>> @@ -0,0 +1,905 @@
>>>>
> +###################################################################################
>>>> +# This test checks if transactions that mixes transactional and
> non-transactional
>>>> +# tables are correctly handled. Thus to check the behavior provided by
> the current
>>>> +# code we devide the test as follows:
>>>> +#
>>>> +# 1 - CREATING TABLES through SELECT * FROM
>>>> +# 2 - MIXING TRANSACTIONAL and NON-TRANSACTIONAL TABLES
>>>> +# 3 - CONCURRENCY:
>>>> +#     2.1 - NON-TRANSACT TABLES -  SET AUTOCOMMIT = 0  | COMMIT
>>>> +#     2.2 - NON-TRANSACT TABLES -  SET AUTOCOMMIT = 1  | START - COMMIT
>>>> +# 4 - SAVE POINTS
>>>>
> +###################################################################################
>>>> + 
>>>> +--echo
> ###################################################################################
>>>> +--echo #                                   CONFIGURATION
>>>> +--echo
> ###################################################################################
>>>> +connection master;
>>>> +
>>>> +CREATE TABLE t1 (a text, b int PRIMARY KEY, c text) ENGINE = MyISAM;
>>>> +CREATE TABLE t2 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t3 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t4 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t5 (a text, b int PRIMARY KEY, c text) ENGINE = MyISAM;
>>>> +CREATE TABLE t6 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t7 (a text, b int PRIMARY KEY, c text) ENGINE = Innodb;
>>>> +CREATE TABLE t8 (a text, b int PRIMARY KEY, c text) ENGINE = MyISAM;
>>>> +CREATE TABLE t9 (a text, b int PRIMARY KEY, c text) ENGINE = MyISAM;
>>>>         
--------
>>>>     
>>>>         
>>> I'd suggest to name all trans-tables t1,t2... and non-trans tables n1,
>>> n2..., so that's easier to understand the test.
>>>       
--------
Done.

>>>   
>>>       
>>>> +
>>>> +INSERT INTO t1 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t2 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t3 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t4 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t5 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +
>>>> +DELIMITER |;
>>>> +
>>>> +CREATE FUNCTION f1 () RETURNS VARCHAR(64)
>>>> +BEGIN
>>>> +  RETURN "Testing...";
>>>> +END|
>>>> +
>>>> +CREATE FUNCTION f2 () RETURNS VARCHAR(64)
>>>> +BEGIN
>>>> +  RETURN f1();
>>>> +END|
>>>> +
>>>> +CREATE PROCEDURE pc6 (IN x INT, IN y VARCHAR(64))
>>>> +BEGIN
>>>> +  INSERT INTO t6 VALUES (y,x,x);
>>>> +END|
>>>> +
>>>> +CREATE TRIGGER tr6_i BEFORE INSERT ON t6 FOR EACH ROW
>>>> +BEGIN
>>>> +  INSERT INTO t7 VALUES (NEW.a, NEW.b, NEW.c);
>>>> +  INSERT INTO t8 VALUES (NEW.a, NEW.b, NEW.c);
>>>> +  INSERT INTO t9 VALUES (NEW.a, NEW.b, NEW.c);
>>>> +END|
>>>> +
>>>> +DELIMITER ;|
>>>> +
>>>> +INSERT INTO t6 VALUES("text 100", 100, "text 100"), ("text 200", 200,
> "text 200"), ("text 300", 300, "text 300");
>>>> +INSERT INTO t7 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t8 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +INSERT INTO t9 VALUES("text 1", 1, "text 1"), ("text 2", 2, "text 2"),
> ("text 3", 3, "text 3");
>>>> +
>>>> +--echo
> ###################################################################################
>>>> +--echo #                                1 - CREATING TABLES
>>>> +--echo
> ###################################################################################
>>>> +connection master;
>>>> +
>>>> +CREATE TABLE t10 SELECT * FROM t2;
>>>> +DROP TABLE t10;
>>>> +CREATE TABLE t11 SELECT * FROM t2;
>>>> +DROP TABLE t11;
>>>> +
>>>> +
>>>> +--echo
> ###################################################################################
>>>> +--echo #                 2 - MIXING TRANSACTIONAL and NON-TRANSACTIONAL
> TABLES
>>>> +--echo
> ###################################################################################
>>>> +connection master;
>>>> +
>>>> +# Caching  (C): (stmt-cache (s), trx-cache (t), ignored (i))
>>>> +# Flushing (F): (upon committing/rollingback a statement (d), upon
>>>> +#               committing/rolling back a transaction (c/r), ignored
> (i))
>>>> +
>>>> +#
>>>> +#                          STMT             ROW              MIXED
>>>> +#
>>>> +
>>>> +let $binlog_start= query_get_value("SHOW MASTER STATUS", Position, 1);
>>>> +--echo #                     C - T           C - T             C - T
>>>> +--echo #1.a) B T T C         F - C           F - C             F - C
>>>> +--echo #
>>>> +BEGIN;
>>>> +INSERT INTO t2 VALUES ("new text 5", 5, "new text 5");
>>>>         
--------
>>>>     
>>>>         
>>> Please also add show_binlog_events or other methods after statements
>>> within transaction to check that statement cache are correctly flushed..
>>>
>>> [snip]
>>>
>>>       
--------
I think you can check this by verifying if the events are outside a
transaction.
Introducing what you have requested you just clutter the test case.
>>>   
>>>       
>>>> === modified file 'mysql-test/suite/rpl/r/rpl_extraCol_myisam.result'
>>>> --- a/mysql-test/suite/rpl/r/rpl_extraCol_myisam.result	2008-11-04
> 07:43:21 +0000
>>>> +++ b/mysql-test/suite/rpl/r/rpl_extraCol_myisam.result	2009-06-06
> 17:51:51 +0000
>>>> @@ -434,7 +434,7 @@ Replicate_Ignore_Table	#
>>>>  Replicate_Wild_Do_Table	
>>>>  Replicate_Wild_Ignore_Table	
>>>>  Last_Errno	1364
>>>> -Last_Error	Could not execute Write_rows event on table test.t9; Field
> 'e' doesn't have a default value, Error_code: 1364; handler error HA_ERR_ROWS_EVENT_APPLY;
> the event's master log master-bin.000001, end_log_pos 330
>>>> +Last_Error	Could not execute Write_rows event on table test.t9; Field
> 'e' doesn't have a default value, Error_code: 1364; handler error HA_ERR_ROWS_EVENT_APPLY;
> the event's master log master-bin.000001, end_log_pos 262
>>>>         
--------
>>>>     
>>>>         
>>> Please mask out the log position.
>>>
>>>       
--------
Done.
>>>   
>>>       
>>>>  Skip_Counter	0
>>>>  Exec_Master_Log_Pos	#
>>>>  Relay_Log_Space	#
>>>> @@ -452,7 +452,7 @@ Master_SSL_Verify_Server_Cert	No
>>>>  Last_IO_Errno	#
>>>>  Last_IO_Error	#
>>>>  Last_SQL_Errno	1364
>>>> -Last_SQL_Error	Could not execute Write_rows event on table test.t9;
> Field 'e' doesn't have a default value, Error_code: 1364; handler error
> HA_ERR_ROWS_EVENT_APPLY; the event's master log master-bin.000001, end_log_pos 330
>>>> +Last_SQL_Error	Could not execute Write_rows event on table test.t9;
> Field 'e' doesn't have a default value, Error_code: 1364; handler error
> HA_ERR_ROWS_EVENT_APPLY; the event's master log master-bin.000001, end_log_pos 262
>>>>  SET GLOBAL SQL_SLAVE_SKIP_COUNTER=2;
>>>>  START SLAVE;
>>>>  *** Create t10 on slave  ***
>>>>     
>>>>         
>>> [snip]
>>>
>>>   
>>>       
>>>> === modified file 'mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result'
>>>> --- a/mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result	2008-07-16 09:17:10
> +0000
>>>> +++ b/mysql-test/suite/rpl/r/rpl_rbr_to_sbr.result	2009-06-06 17:51:51
> +0000
>>>> @@ -19,10 +19,8 @@ Log_name	Pos	Event_type	Server_id	End_lo
>>>>  master-bin.000001	#	Format_desc	1	#	Server ver: VERSION, Binlog ver: 4
>>>>  master-bin.000001	#	Query	1	#	use `test`; CREATE TABLE t1 (a INT, b
> LONG)
>>>>  master-bin.000001	#	Query	1	#	use `test`; INSERT INTO t1 VALUES (1,1),
> (2,2)
>>>> -master-bin.000001	#	Query	1	#	use `test`; BEGIN
>>>>  master-bin.000001	#	Table_map	1	#	table_id: # (test.t1)
>>>>  master-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
>>>> -master-bin.000001	#	Query	1	#	use `test`; COMMIT
>>>>  **** On Slave ****
>>>>  SHOW SLAVE STATUS;
>>>>  Slave_IO_State	#
>>>> @@ -31,7 +29,7 @@ Master_User	root
>>>>  Master_Port	MASTER_PORT
>>>>  Connect_Retry	1
>>>>  Master_Log_File	master-bin.000001
>>>> -Read_Master_Log_Pos	594
>>>> +Read_Master_Log_Pos	457
>>>>     
>>>>         
--------
>>> Mask out the position
>>>       
--------
Done
>>>   
>>>       
>>>>  Relay_Log_File	#
>>>>  Relay_Log_Pos	#
>>>>  Relay_Master_Log_File	master-bin.000001
>>>> @@ -46,7 +44,7 @@ Replicate_Wild_Ignore_Table	
>>>>  Last_Errno	0
>>>>  Last_Error	
>>>>  Skip_Counter	0
>>>> -Exec_Master_Log_Pos	594
>>>> +Exec_Master_Log_Pos	457
>>>>  Relay_Log_Space	#
>>>>  Until_Condition	None
>>>>  Until_Log_File	
>>>> @@ -68,9 +66,7 @@ Log_name	Pos	Event_type	Server_id	End_lo
>>>>  slave-bin.000001	#	Format_desc	2	#	Server ver: VERSION, Binlog ver: 4
>>>>  slave-bin.000001	#	Query	1	#	use `test`; CREATE TABLE t1 (a INT, b
> LONG)
>>>>  slave-bin.000001	#	Query	1	#	use `test`; INSERT INTO t1 VALUES (1,1),
> (2,2)
>>>> -slave-bin.000001	#	Query	1	#	use `test`; BEGIN
>>>>  slave-bin.000001	#	Table_map	1	#	table_id: # (test.t1)
>>>>  slave-bin.000001	#	Write_rows	1	#	table_id: # flags: STMT_END_F
>>>> -slave-bin.000001	#	Query	1	#	use `test`; COMMIT
>>>>  DROP TABLE IF EXISTS t1;
>>>>  SET @@global.binlog_format= @old_binlog_format;
>>>>     
>>>>         
>>> [snip]
>>>
>>>   
>>>       
>>>> === modified file 'mysql-test/suite/rpl/r/rpl_row_conflicts.result'
>>>> --- a/mysql-test/suite/rpl/r/rpl_row_conflicts.result	2009-01-09 14:12:31
> +0000
>>>> +++ b/mysql-test/suite/rpl/r/rpl_row_conflicts.result	2009-06-06 17:51:51
> +0000
>>>> @@ -24,7 +24,7 @@ a
>>>>  1
>>>>  [on slave]
>>>>  ---- Wait until slave stops with an error ----
>>>> -Last_SQL_Error = Could not execute Write_rows event on table test.t1;
> Duplicate entry '1' for key 'PRIMARY', Error_code: 1062; handler error
> HA_ERR_FOUND_DUPP_KEY; the event's master log master-bin.000001, end_log_pos 346 (expected
> "duplicate key" error)
>>>> +Last_SQL_Error = Could not execute Write_rows event on table test.t1;
> Duplicate entry '1' for key 'PRIMARY', Error_code: 1062; handler error
> HA_ERR_FOUND_DUPP_KEY; the event's master log master-bin.000001, end_log_pos 278 (expected
> "duplicate key" error)
>>>>         

>>>>     
>>>>         
>>> Mask out the position
>>>
>>>       
This does not exist in mysql-pe.
>>>   
>>>       
>>>>  SELECT * FROM t1;
>>>>  a
>>>>  1
>>>> @@ -50,7 +50,7 @@ SELECT * FROM t1;
>>>>  a
>>>>  [on slave]
>>>>  ---- Wait until slave stops with an error ----
>>>> -Last_SQL_Error = Could not execute Delete_rows event on table test.t1;
> Can't find record in 't1', Error_code: 1032; handler error HA_ERR_KEY_NOT_FOUND; the
> event's master log master-bin.000001, end_log_pos 982 (expected "can't find record"
> error)
>>>> +Last_SQL_Error = Could not execute Delete_rows event on table test.t1;
> Can't find record in 't1', Error_code: 1032; handler error HA_ERR_KEY_NOT_FOUND; the
> event's master log master-bin.000001, end_log_pos 503 (expected "can't find record"
> error)
>>>>  SELECT * FROM t1;
>>>>  a
>>>>  ---- Resolve the conflict on the slave and restart SQL thread ----
>>>>     
>>>>         
>>> [snip]
>>>
>>>   
>>>       
>>>> === modified file 'mysql-test/suite/rpl/t/rpl_blackhole.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_blackhole.test	2009-01-30 13:44:49
> +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_blackhole.test	2009-06-06 17:51:51
> +0000
>>>> @@ -45,9 +45,20 @@ source extra/rpl_tests/rpl_blackhole.tes
>>>>  let $statement = INSERT INTO t1 SELECT * FROM t2;
>>>>  source extra/rpl_tests/rpl_blackhole.test;
>>>>  
>>>> +# 
>>>> +# The MASTER has MyISAM as the engine for both tables. The SLAVE has
> Blackhole
>>>> +# on t1 (transactional engine) and MyISAM on t2 (non-transactional
> engine). 
>>>> +#
>>>> +# In MIXED MODE, the command below is logged as statement on the MASTER.
> On the
>>>> +# other hand, on the SLAVE, it is tagged as unsafe and then executed in
> ROW
>>>> +# MODE. In a nutshell, statements that mixes transactional and
> non-transactional
>>>> +# engines are tagged as unsafe. This behavior generates a problem as no
> rows are
>>>> +# returned and makes the # SLAVE diverge from the MASTER.
>>>> +#
>>>>  # Test INSERT-SELECT from Blackhole, no primary key
>>>> -let $statement = INSERT INTO t2 SELECT * FROM t1;
>>>> -source extra/rpl_tests/rpl_blackhole.test;
>>>> +#let $statement = INSERT INTO t2 SELECT * FROM t1;
>>>> +#source extra/rpl_tests/rpl_blackhole.test;
>>>> +#
>>>>  
>>>>         
--------
>>>>     
>>>>         
>>> If following the proposal at the beginning, then the above statement
>>> will not be considered unsafe, the the test will pass in mixed mode too.
>>>       
--------
I disregarded this as we are going to keep the current behavior.
>>>   
>>>       
>>>>  connection master;
>>>>  ALTER TABLE t1 ADD PRIMARY KEY pk_t1 (a,b);
>>>>
>>>> === modified file 'mysql-test/suite/rpl/t/rpl_empty_master_crash.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_empty_master_crash.test	2007-06-27
> 12:29:10 +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_empty_master_crash.test	2009-06-06
> 17:51:51 +0000
>>>> @@ -1,6 +1,6 @@
>>>>  source include/master-slave.inc;
>>>>  
>>>> -source include/show_slave_status.inc;
>>>> +source include/show_slave_status2.inc;
>>>>  
>>>>  #
>>>>  # Load table should not succeed on the master as this is not a slave
>>>>
>>>> === modified file 'mysql-test/suite/rpl/t/rpl_innodb.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_innodb.test	2008-10-29 13:25:03 +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_innodb.test	2009-06-06 17:51:51 +0000
>>>> @@ -111,7 +111,6 @@ connection slave;
>>>>  SHOW CREATE TABLE mysqltest1.tmp;
>>>>  --error ER_NO_SUCH_TABLE
>>>>  SHOW CREATE TABLE mysqltest1.tmp2;
>>>> -# t1 has two rows here: the transaction not rolled back since t1 uses
> MyISAM
>>>>     
>>>>         
>>> Please check this part of the test, I think it need to be rewritten (or
>>> removed).
>>>
>>>       
--------
>>>   
>>>       
>>>>  SELECT COUNT(*) FROM mysqltest1.t1;
>>>>  FLUSH LOGS;
>>>>  
>>>>     
>>>>         
>>> [snip]
>>>
>>>       
--------
Done. I've updated the comments because in ROW based replication the
behavior is different.
However, I think it is good to leave this part of the code just to
highlight the difference between the
replication modes.


>>>   
>>>       
>>>> === modified file 'mysql-test/suite/rpl/t/rpl_row_until.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_row_until.test	2009-02-19 20:29:12
> +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_row_until.test	2009-06-06 17:51:51
> +0000
>>>> @@ -14,18 +14,21 @@ INSERT INTO t1 VALUES (1),(2),(3),(4);
>>>>  DROP TABLE t1;
>>>>  # Save master log postion for query DROP TABLE t1
>>>>  save_master_pos;
>>>> -let $master_pos_drop_t1= query_get_value(SHOW BINLOG EVENTS, Pos, 7);
>>>> +let $master_pos_drop_t1= query_get_value(SHOW BINLOG EVENTS, Pos, 5);
>>>> +#show binlog events;
>>>>  
>>>>     
>>>>         
--------
>>> I'd suggest to use:
>>>   let $master_pos_drop_t1= query_get_value(SHOW MASTER STATUS, Position, 1);
>>>
>>> right before the DROP statement.
>>>
>>>       
--------

Done.

>>>   
>>>       
>>>>  CREATE TABLE t2(n INT NOT NULL AUTO_INCREMENT PRIMARY KEY);
>>>>  # Save master log postion for query CREATE TABLE t2
>>>>  save_master_pos;
>>>> -let $master_pos_create_t2= query_get_value(SHOW BINLOG EVENTS, Pos, 8);
>>>> +let $master_pos_create_t2= query_get_value(SHOW BINLOG EVENTS, Pos, 6);
>>>> +#show binlog events;
>>>>  
>>>>  INSERT INTO t2 VALUES (1),(2);
>>>>  save_master_pos;
>>>>  # Save master log postion for query INSERT INTO t2 VALUES (1),(2);
>>>> -let $master_pos_insert1_t2= query_get_value(SHOW BINLOG EVENTS,
> End_log_pos, 12);
>>>> +let $master_pos_insert1_t2= query_get_value(SHOW BINLOG EVENTS,
> End_log_pos, 8);
>>>>  sync_slave_with_master;
>>>> +#show binlog events;
>>>>  
>>>>  # Save relay log postion for query INSERT INTO t2 VALUES (1),(2);
>>>>  let $relay_pos_insert1_t2= query_get_value(show slave status,
> Relay_Log_Pos, 1);
>>>> @@ -34,8 +37,9 @@ connection master;
>>>>  INSERT INTO t2 VALUES (3),(4);
>>>>  DROP TABLE t2;
>>>>  # Save master log postion for query INSERT INTO t2 VALUES (1),(2);
>>>> -let $master_pos_drop_t2= query_get_value(SHOW BINLOG EVENTS,
> End_log_pos, 17);
>>>> +let $master_pos_drop_t2= query_get_value(SHOW BINLOG EVENTS,
> End_log_pos, 11);
>>>>  sync_slave_with_master;
>>>> +#show binlog events;
>>>>  
>>>>  --source include/stop_slave.inc
>>>>  # Reset slave.
>>>> @@ -45,6 +49,7 @@ eval CHANGE MASTER TO MASTER_USER='root'
>>>>  --enable_query_log
>>>>  
>>>>  # Try to replicate all queries until drop of t1
>>>> +
>>>>  connection slave;
>>>>  echo START SLAVE UNTIL MASTER_LOG_FILE='master-bin.000001',
> MASTER_LOG_POS=master_pos_drop_t1;
>>>>  --disable_query_log
>>>>     
>>>>         
>>> [snip]
>>>
>>>   
>>>       
>>>> === modified file 'sql/ha_ndbcluster_binlog.cc'
>>>> --- a/sql/ha_ndbcluster_binlog.cc	2008-04-09 16:42:05 +0000
>>>> +++ b/sql/ha_ndbcluster_binlog.cc	2009-06-06 17:51:51 +0000
>>>> @@ -257,6 +257,7 @@ static void run_query(THD *thd, char *bu
>>>>    thd->query= buf;
>>>>    thd->variables.pseudo_thread_id= thread_id;
>>>>    thd->transaction.stmt.modified_non_trans_table= FALSE;
>>>> +  thd->transaction.stmt.modified_trans_table= FALSE;
>>>>    if (disable_binlog)
>>>>      thd->options&= ~OPTION_BIN_LOG;
>>>>      
>>>> @@ -1857,7 +1858,8 @@ static void ndb_binlog_query(THD *thd, C
>>>>    thd->db= schema->db;
>>>>    thd->binlog_query(THD::STMT_QUERY_TYPE, schema->query,
>>>>                      schema->query_length, FALSE,
>>>> -                    schema->name[0] == 0 || thd->db[0] == 0);
>>>> +                    schema->name[0] == 0 || thd->db[0] == 0, 
>>>> +                    FALSE, THD::KILLED_NO_VALUE);
>>>>         
--------
>>>>     
>>>>         
>>> Can we add a new argument 'direct' to ndb_binlog_query and pass it down
>>> to THD::binlog_query? So that we can get ride of the DDL hack below.
>>>       
--------
Sure. I will try this.
>>>   
>>>       
>>>>    thd->server_id= thd_server_id_save;
>>>>    thd->db= thd_db_save;
>>>>  }
>>>>     
>>>>         
>>> [snip]
>>>
>>>   
>>>       
>>>>    /*
>>>>       In most cases this is only called if 'is_open()' is true; in fact
> this is
>>>> @@ -3936,7 +4008,6 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>>>>    */
>>>>    if (likely(is_open()))
>>>>    {
>>>> -    IO_CACHE *file= &log_file;
>>>>  #ifdef HAVE_REPLICATION
>>>>      /*
>>>>        In the future we need to add to the following if tests like
>>>> @@ -3946,48 +4017,50 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>>>>      const char *local_db= event_info->get_db();
>>>>      if ((thd && !(thd->options & OPTION_BIN_LOG)) ||
>>>>  	(!binlog_filter->db_ok(local_db)))
>>>> -    {
>>>> -      VOID(pthread_mutex_unlock(&LOCK_log));
>>>>        DBUG_RETURN(0);
>>>> -    }
>>>>  #endif /* HAVE_REPLICATION */
>>>> +    IO_CACHE *file= NULL;
>>>>  
>>>> -#if defined(USING_TRANSACTIONS) 
>>>>      /*
>>>> -      Should we write to the binlog cache or to the binlog on disk?
>>>> -      Write to the binlog cache if:
>>>> -      - it is already not empty (meaning we're in a transaction; note
> that the
>>>> -     present event could be about a non-transactional table, but still
> we need
>>>> -     to write to the binlog cache in that case to handle updates to
> mixed
>>>> -     trans/non-trans table types the best possible in binlogging)
>>>> -      - or if the event asks for it (cache_stmt == TRUE).
>>>> +      This is an ugly hacking that must be removed as soon as we fix a
> bug in
>>>> +      NDB. In a nutshell, this  means that we should use the binary log
> directly
>>>> +      if we are processing a DDL.
>>>>      */
>>>> -    if (opt_using_transactions && thd)
>>>> +
>>>> +    bool ddl=
>>>> +      (!(thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))
> &&
>>>> +       !thd->transaction.stmt.modified_non_trans_table &&
>>>> +       !thd->transaction.stmt.modified_trans_table &&
>>>> +       !thd->transaction.all.modified_non_trans_table &&
>>>> +       !thd->transaction.all.modified_trans_table &&
>>>> +       !event_info->is_trans_event());
>>>> +
>>>>         
--------
>>>>     
>>>>         
>>> Hmmm, this looks really really really ugly, why cannot we add change the
>>> interface of ndb_binlog_query to add a new argument 'direct', and pass
>>> it down to this function?
>>>       
--------

I will try this.
>>>   
>>>       
>>>> +    direct= direct || ddl;
>>>> +
>>>> +    if (direct)
>>>> +    {
>>>> +      file= &log_file;
>>>> +      pthread_mutex_lock(&LOCK_log);
>>>> +    }
>>>> +    else
>>>>      {
>>>> -      if (thd->binlog_setup_trx_data())
>>>> +      thd->binlog_setup_trx_data();
>>>>          goto err;
>>>>     
>>>>         
>>> Must be a mistake, should not remove the 'if'.
>>>
>>> [snip]
>>>       

Sure. Done.

Cheers.
Thread
bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687Alfranio Correia6 Jun
  • Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687Luís Soares17 Jun
    • Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687Alfranio Correia17 Jun
      • Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687Luís Soares18 Jun
        • Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687Alfranio Correia16 Aug
  • Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687He Zhenxing29 Jun
    • Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687Alfranio Correia12 Aug
      • Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687He Zhenxing13 Aug
        • Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687Alfranio Correia16 Aug
Re: bzr commit into mysql-5.1-bugteam branch (alfranio.correia:2911)WL#2687Alfranio Correia18 Jun