List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:April 19 2008 12:30pm
Subject:Re: bk commit into 5.1 tree (skozlov:1.2552)
View as plain text  
Hi Serge

Yes, approved after fix

On 2008-04-18 Fri 17:35 +0400,Serge Kozlov wrote:
> 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