List:Commits« Previous MessageNext Message »
From:Serge Kozlov Date:April 18 2008 1:35pm
Subject:Re: bk commit into 5.1 tree (skozlov:1.2552)
View as plain text  
Hi, Zhenxing.

I agree with your notes. Do you approve that if I fix them?

He Zhenxing wrote:
> On 2008-04-18 Fri 14:02 +0400,Serge Kozlov wrote:
>> Hi, Zhenxing.
>>
>> Thanks for review. Look my notes started with 'skozlov' and say what do 
>> you think.
>>
>> He Zhenxing wrote:
>>> Hi Serge
>>>
>>> Thank you for your work! I have some comments, please explain or fix
>>> these, see below.
>>>
>>> On 2008-04-02 Wed 18:51 +0400,Serge Kozlov wrote:
>>>> Below is the list of changes that have just been committed into a local
>>>> 5.1 repository of skozlov.  When skozlov 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-04-02 18:51:14+04:00, skozlov@stripped +4 -0
>>>>   WL#3734, Slave group execution
>>>>
>>>>   mysql-test/suite/rpl/r/rpl_slave_grp_exec.result@stripped, 2008-04-02
> 18:51:11+04:00, skozlov@stripped +125 -0
>>>>     Result file
>>>>
>>>>   mysql-test/suite/rpl/r/rpl_slave_grp_exec.result@stripped, 2008-04-02
> 18:51:11+04:00, skozlov@stripped +0 -0
>>>>
>>>>   mysql-test/suite/rpl/t/rpl_slave_grp_exec-master.opt@stripped, 2008-04-02
> 18:51:11+04:00, skozlov@stripped +1 -0
>>>>     Option file for master
>>>>
>>>>   mysql-test/suite/rpl/t/rpl_slave_grp_exec-master.opt@stripped, 2008-04-02
> 18:51:11+04:00, skozlov@stripped +0 -0
>>>>
>>>>   mysql-test/suite/rpl/t/rpl_slave_grp_exec-slave.opt@stripped, 2008-04-02
> 18:51:11+04:00, skozlov@stripped +1 -0
>>>>     Option file for slave
>>>>
>>>>   mysql-test/suite/rpl/t/rpl_slave_grp_exec-slave.opt@stripped, 2008-04-02
> 18:51:11+04:00, skozlov@stripped +0 -0
>>>>
>>>>   mysql-test/suite/rpl/t/rpl_slave_grp_exec.test@stripped, 2008-04-02
> 18:51:11+04:00, skozlov@stripped +164 -0
>>>>     Test case
>>>>
>>>>   mysql-test/suite/rpl/t/rpl_slave_grp_exec.test@stripped, 2008-04-02
> 18:51:11+04:00, skozlov@stripped +0 -0
>>>>
>>>> diff -Nrup a/mysql-test/suite/rpl/r/rpl_slave_grp_exec.result
> b/mysql-test/suite/rpl/r/rpl_slave_grp_exec.result
>>>> --- /dev/null	Wed Dec 31 16:00:00 196900
>>>> +++ b/mysql-test/suite/rpl/r/rpl_slave_grp_exec.result	2008-04-02
> 18:51:11 +04:00
>>>> @@ -0,0 +1,125 @@
>>>> +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;
>>>> +
>>>> +*** Preparing data ***
>>>> +CREATE TABLE t1 (a INT NOT NULL, b VARCHAR(10)) ENGINE=MyISAM;
>>>> +CREATE TABLE t2 LIKE t1;
>>>> +CREATE TABLE t3 LIKE t1;
>>>> +CREATE TRIGGER tr1 BEFORE UPDATE ON t1
>>>> +FOR EACH ROW BEGIN
>>>> +UPDATE t2 SET b='Y' WHERE a=NEW.a;
>>>> +END|
>>>> +CREATE TRIGGER tr2 AFTER UPDATE ON t1
>>>> +FOR EACH ROW BEGIN
>>>> +UPDATE t3 SET b='Z' WHERE a=NEW.a;
>>>> +END|
>>>> +
>>>> +*** Test non-transactional group w/o PK ***
>>>> +INSERT INTO t3 VALUES(1, 'A');
>>>> +INSERT INTO t2 VALUES(1, 'A');
>>>> +INSERT INTO t1 VALUES(1, 'A');
>>>> +RENAME TABLE t3 TO t3_bak;
>>>> +UPDATE t1 SET b = 'X' WHERE a = 1;
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +a	b
>>>> +1	X
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +a	b
>>>> +1	Y
>>>> +SELECT * FROM t3 ORDER BY a;
>>>> +a	b
>>>> +1	Z
>>>> +SHOW TABLES LIKE 't%';
>>>> +Tables_in_test (t%)
>>>> +t1
>>>> +t2
>>>> +t3_bak
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +a	b
>>>> +1	A_for_row_or_X_for_stmt_mixed
>>>> +SELECT * FROM t2 ORDER BA_for_row_or_Y_for_stmt_mixed a;
>>>> +a	b
>>>> +1	A_for_row_or_Y_for_stmt_mixed
>>>> +STOP SLAVE;
>>>> +RENAME TABLE t3_bak TO t3;
>>>> +START SLAVE;
>>>> +TRUNCATE t1;
>>>> +TRUNCATE t2;
>>>> +TRUNCATE t3;
>>>> +
>>>> +*** Test non-transactional group w/ PK ***
>>>> +ALTER TABLE t1 ADD PRIMARY KEY (a);
>>>> +ALTER TABLE t2 ADD PRIMARY KEY (a);
>>>> +ALTER TABLE t3 ADD PRIMARY KEY (a);
>>>> +RENAME TABLE t3 TO t3_bak;
>>>> +INSERT INTO t3 VALUES(2, 'B');
>>>> +INSERT INTO t2 VALUES(2, 'B');
>>>> +INSERT INTO t1 VALUES(2, 'B');
>>>> +UPDATE t1 SET b = 'X' WHERE a = 2;
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +a	b
>>>> +2	X
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +a	b
>>>> +2	Y
>>>> +SELECT * FROM t3 ORDER BY a;
>>>> +a	b
>>>> +2	Z
>>>> +SHOW TABLES LIKE 't%';
>>>> +Tables_in_test (t%)
>>>> +t1
>>>> +t2
>>>> +t3_bak
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +a	b
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +a	b
>>>> +STOP SLAVE;
>>>> +RENAME TABLE t3_bak TO t3;
>>>> +START SLAVE;
>>>> +TRUNCATE t1;
>>>> +TRUNCATE t2;
>>>> +TRUNCATE t3;
>>>> +
>>>> +*** Test transactional group w/ PK ***
>>>> +ALTER TABLE t1 ENGINE=InnoDB;
>>>> +ALTER TABLE t2 ENGINE=InnoDB;
>>>> +ALTER TABLE t3 ENGINE=InnoDB;
>>>> +RENAME TABLE t3 TO t3_bak;
>>>> +INSERT INTO t3 VALUES (3, 'C'), (4, 'D');
>>>> +INSERT INTO t2 VALUES (3, 'C'), (4, 'D');
>>>> +INSERT INTO t1 VALUES (3, 'C'), (4, 'D');
>>>> +BEGIN;
>>>> +UPDATE t1 SET b = 'X' WHERE a = 3;
>>>> +UPDATE t1 SET b = 'X' WHERE a = 4;
>>>> +COMMIT;
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +a	b
>>>> +3	X
>>>> +4	X
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +a	b
>>>> +3	Y
>>>> +4	Y
>>>> +SELECT * FROM t3 ORDER BY a;
>>>> +a	b
>>>> +3	Z
>>>> +4	Z
>>>> +SHOW TABLES LIKE 't%';
>>>> +Tables_in_test (t%)
>>>> +t1
>>>> +t2
>>>> +t3_bak
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +a	b
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +a	b
>>>> +STOP SLAVE;
>>>> +RENAME TABLE t3_bak TO t3;
>>>> +START SLAVE;
>>>> +*** Clean up ***
>>>> +DROP TABLE t1,t2,t3;
>>>> diff -Nrup a/mysql-test/suite/rpl/t/rpl_slave_grp_exec-master.opt
> b/mysql-test/suite/rpl/t/rpl_slave_grp_exec-master.opt
>>>> --- /dev/null	Wed Dec 31 16:00:00 196900
>>>> +++ b/mysql-test/suite/rpl/t/rpl_slave_grp_exec-master.opt	2008-04-02
> 18:51:11 +04:00
>>>> @@ -0,0 +1 @@
>>>> +--innodb
>>>> diff -Nrup a/mysql-test/suite/rpl/t/rpl_slave_grp_exec-slave.opt
> b/mysql-test/suite/rpl/t/rpl_slave_grp_exec-slave.opt
>>>> --- /dev/null	Wed Dec 31 16:00:00 196900
>>>> +++ b/mysql-test/suite/rpl/t/rpl_slave_grp_exec-slave.opt	2008-04-02
> 18:51:11 +04:00
>>>> @@ -0,0 +1 @@
>>>> +--innodb
>>>> diff -Nrup a/mysql-test/suite/rpl/t/rpl_slave_grp_exec.test
> b/mysql-test/suite/rpl/t/rpl_slave_grp_exec.test
>>>> --- /dev/null	Wed Dec 31 16:00:00 196900
>>>> +++ b/mysql-test/suite/rpl/t/rpl_slave_grp_exec.test	2008-04-02 18:51:11
> +04:00
>>>> @@ -0,0 +1,164 @@
>>>> +#############################################################
>>>> +# Author: Serge Kozlov <skozlov@stripped>
>>>> +# Date:   03/21/2008
>>>> +# Purpose: Testing slave group execution: stop in middle
>>>> +# of a group (of events) should be immpossible on slave.
>>>> +# Group of events means set of statement between BEGIN/COMMIT
>>>> +# for transactional engines or set of events created by 
>>>> +# invoked features (like triggers, sp) for non-transactional 
>>>> +# engines.  
>>>> +#############################################################
>>> Add the WorkLog number in the comment for reference.
>>>
>>>> +--source include/have_innodb.inc
>>>> +--source include/master-slave.inc
>>>> +--echo
>>>> +
>>>> +# Create tables and data
>>>> +--echo *** Preparing data ***
>>>> +--connection master
>>>> +CREATE TABLE t1 (a INT NOT NULL, b VARCHAR(10)) ENGINE=MyISAM;
>>>> +CREATE TABLE t2 LIKE t1;
>>>> +CREATE TABLE t3 LIKE t1;
>>>> +
>>>> +DELIMITER |;
>>>> +CREATE TRIGGER tr1 BEFORE UPDATE ON t1
>>>> +  FOR EACH ROW BEGIN
>>>> +    UPDATE t2 SET b='Y' WHERE a=NEW.a;
>>>> +  END|
>>>> +CREATE TRIGGER tr2 AFTER UPDATE ON t1
>>>> +  FOR EACH ROW BEGIN
>>>> +    UPDATE t3 SET b='Z' WHERE a=NEW.a;
>>>> +  END|
>>>> +DELIMITER ;|
>>>> +--echo
>>>> +
>>>> +# Test non-transactional group with MyISAM tables w/o PK.
>>>> +# Data for t1,t2 should be replicated for SBR even t3 
>>>> +# doesn't exist on slave
>>>> +--echo *** Test non-transactional group w/o PK ***
>>>> +
>>>> +--connection master
>>>> +INSERT INTO t3 VALUES(1, 'A');
>>>> +INSERT INTO t2 VALUES(1, 'A');
>>>> +INSERT INTO t1 VALUES(1, 'A');
>>>> +--sync_slave_with_master
>>>> +
>>>> +RENAME TABLE t3 TO t3_bak;
>>>> +
>>>> +--connection master
>>>> +UPDATE t1 SET b = 'X' WHERE a = 1;
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +SELECT * FROM t3 ORDER BY a;
>>>> +
>>>> +--connection slave
>>>> +--source include/wait_for_slave_sql_to_stop.inc
>>>> +SHOW TABLES LIKE 't%';
>>>> +--replace_regex /(A|X)/A_for_row_or_X_for_stmt_mixed/
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +--replace_regex /(A|Y)/A_for_row_or_Y_for_stmt_mixed/
>>>> +SELECT * FROM t2 ORDER BY a;
>>> Here you replaced the 'A' or 'X' output of SELECT statement to
>>> 'A_for_row_...', but lets consider the following scenario:
>>>   - in Row format, SELECT statement outputs 'X' for t1 or 'Y' for t2
>>>   - in Statement or Mixed format, SELECT statement outputs 'A' for t1 or
>>> t2
>>> these two scenarios are all should be considered wrong, and hence fail
>>> the test, but with the replacement, the test will pass.
>>>
>>> So I suggest replace '(X|Y)' to the string in statement or mixed format,
>>> and replace 'A' to the string in row format.
>> skozlov: your suggestion means different result files (in fact two tests 
>> with same test code) for row and mixed/stmt but I would like to have one 
>> file for all formats. So I can try to find solution when 'X' appears for 
>> stmt/mixed and 'Y' for 'row' will produce an error. But it will still 
>> one test cases with one result file.
>>
> 
> No, I mean something like this:
> 
> if (`select $binlog_format=='ROW'`)
> {
>    let $val= A;
> }
> if (!`select $binlog_format=='ROW'`)
> {
>   let $val= (X|Y);
> }
> 
> --replace_regex /$val/OK/
> SELECT * from t1 ORDER BY a;

Agree with you. It is another good solution

> 
>>>> +
>>>> +STOP SLAVE;
>>>> +--source include/wait_for_slave_to_stop.inc
>>>> +RENAME TABLE t3_bak TO t3;
>>>> +START SLAVE;
>>>> +--source include/wait_for_slave_to_start.inc
>>>> +
>>>> +--connection master
>>>> +TRUNCATE t1;
>>>> +TRUNCATE t2;
>>>> +TRUNCATE t3;
>>>> +--sync_slave_with_master
>>>> +--echo
>>>> +
>>>> +
>>>> +# Test non-transactional group with MyISAM tables and PK.
>>>> +# No data replicated because update based on PK
>>>> +--echo *** Test non-transactional group w/ PK ***
>>>> +
>>>> +--connection master
>>>> +ALTER TABLE t1 ADD PRIMARY KEY (a);
>>>> +ALTER TABLE t2 ADD PRIMARY KEY (a);
>>>> +ALTER TABLE t3 ADD PRIMARY KEY (a);
>>>> +--sync_slave_with_master
>>>> +RENAME TABLE t3 TO t3_bak;
>>>> +
>>>> +--connection master
>>>> +INSERT INTO t3 VALUES(2, 'B');
>>>> +INSERT INTO t2 VALUES(2, 'B');
>>>> +INSERT INTO t1 VALUES(2, 'B');
>>>> +UPDATE t1 SET b = 'X' WHERE a = 2;
>>>> +
>>>> +--connection slave
>>>> +--source include/wait_for_slave_sql_to_stop.inc
>>>> +
>>>> +--connection master
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +SELECT * FROM t3 ORDER BY a;
>>>> +
>>>> +--connection slave
>>>> +SHOW TABLES LIKE 't%';
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +
>>>> +STOP SLAVE;
>>>> +--source include/wait_for_slave_to_stop.inc
>>>> +RENAME TABLE t3_bak TO t3;
>>>> +START SLAVE;
>>>> +--source include/wait_for_slave_to_start.inc
>>>> +
>>>> +--connection master
>>>> +TRUNCATE t1;
>>>> +TRUNCATE t2;
>>>> +TRUNCATE t3;
>>>> +--sync_slave_with_master
>>>> +--echo
>>>> +
>>>> +
>>>> +# Test transactional group with InnoDB tables with PK
>>>> +# No data replicated if errors happens inside transaction
>>>> +--echo *** Test transactional group w/ PK ***
>>>> +
>>>> +--connection master
>>>> +ALTER TABLE t1 ENGINE=InnoDB;
>>>> +ALTER TABLE t2 ENGINE=InnoDB;
>>>> +ALTER TABLE t3 ENGINE=InnoDB;
>>>> +
>>>> +--connection slave
>>>> +RENAME TABLE t3 TO t3_bak;
>>>> +
>>>> +--connection master
>>>> +INSERT INTO t3 VALUES (3, 'C'), (4, 'D');
>>>> +INSERT INTO t2 VALUES (3, 'C'), (4, 'D');
>>>> +INSERT INTO t1 VALUES (3, 'C'), (4, 'D');
>>>> +BEGIN;
>>>> +UPDATE t1 SET b = 'X' WHERE a = 3;
>>>> +UPDATE t1 SET b = 'X' WHERE a = 4;
>>>> +COMMIT;
>>> I think there is a problem here, the table t3 is named to t3_bak before
>>> the INSERT statements, and the first INSERT statement is for table t3,
>>> so when these statements are replicated to slave, the slave will stop on
>>> the first INSERT INTO t3 ... statement, and all the statements after
>>> this, include the UPDATE statements, will be ignored. And I don't think
>>> this is what you mean. I think the RENAME TABLE statement should be
>>> moved after all the INSERT statements.
>> No it's *transaction* engine (InnoDB) so any failures inside 
>> BEGIN/COMMIT should do rollback. It's correct behavior.
>>
> 
> But the first three inserts is not included in the transaction, and I
> think what the test want is rollback the transaction of the two UPDATEs
> statement. 

Yes, it's a mistake. Will fix.

> 
>>>> +
>>>> +--connection slave
>>>> +--source include/wait_for_slave_sql_to_stop.inc
>>>> +
>>>> +--connection master
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +SELECT * FROM t3 ORDER BY a;
>>>> +
>>>> +--connection slave
>>>> +SHOW TABLES LIKE 't%';
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +SELECT * FROM t2 ORDER BY a;
>>>> +
>>>> +STOP SLAVE;
>>>> +--source include/wait_for_slave_to_stop.inc
>>>> +RENAME TABLE t3_bak TO t3;
>>>> +START SLAVE;
>>>> +--source include/wait_for_slave_to_start.inc
>>>> +
>>>> +# Clean up
>>>> +--echo *** Clean up ***
>>>> +--connection master
>>>> +DROP TABLE t1,t2,t3;
>>>> +
>>>> +# End of 5.1 test
>>>
>>>
>>
>> -- 
>> Serge Kozlov, QA Developer
>> MySQL AB, Moscow, Russia, www.mysql.com
>> Office:
>>
>> Are you MySQL certified?  www.mysql.com/certification
>>
>> -- 
>> MySQL Code Commits Mailing List
>> For list archives: http://lists.mysql.com/commits
>> To unsubscribe:    http://lists.mysql.com/commits?unsub=1
> 
> 


-- 
Serge Kozlov, QA Developer
MySQL AB, Moscow, Russia, www.mysql.com
Office:

Are you MySQL certified?  www.mysql.com/certification
Thread
bk commit into 5.1 tree (skozlov:1.2552)Serge Kozlov2 Apr
  • Re: bk commit into 5.1 tree (skozlov:1.2552)Sven Sandberg3 Apr
    • Re: bk commit into 5.1 tree (skozlov:1.2552)Serge Kozlov3 Apr
      • Re: bk commit into 5.1 tree (skozlov:1.2552)Sven Sandberg9 Apr
  • Re: bk commit into 5.1 tree (skozlov:1.2552)He Zhenxing18 Apr
    • Re: bk commit into 5.1 tree (skozlov:1.2552)Serge Kozlov18 Apr
      • Re: bk commit into 5.1 tree (skozlov:1.2552)He Zhenxing18 Apr
        • Re: bk commit into 5.1 tree (skozlov:1.2552)Serge Kozlov18 Apr
          • Re: bk commit into 5.1 tree (skozlov:1.2552)He Zhenxing19 Apr