Re: bzr commit into mysql-5.1 branch (alfranio.correia:2669) Bug#38360

[Sven Sandberg]

Alfranio Correia wrote:
> #At file:///home/acorreia/workspace.sun/repository.mysql/mysql-5.1-rpl/
> 
>  2669 Alfranio Correia	2008-10-06
>       Created a new test case that aims at verifying that 
>       replication works with an intermediate slave using BLACKHOLE 
>       (BUG#38360) storage ngine.
> added:
>   mysql-test/include/hierarchical_rpl_for_4_hosts_init.inc
>   mysql-test/include/hierarchical_rpl_for_4_hosts_sync.inc
>   mysql-test/suite/rpl/r/rpl_hierarchical_for_4_hosts.result
>   mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.cnf
>   mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.test
> 
> per-file messages:
>   mysql-test/include/hierarchical_rpl_for_4_hosts_init.inc
>     This configures an hierarchical replication topology and starts the replication
> threads.
>   mysql-test/include/hierarchical_rpl_for_4_hosts_sync.inc
>     Verifies if all replicas are synchronized.
>   mysql-test/suite/rpl/r/rpl_hierarchical_for_4_hosts.result
>     This file has the expected results.
>   mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.cnf
>     This configuration file introduces two additional servers: slave1 --> replica
> c and slave2 --> replica d.
>   mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.test
>     This the test case aims at testing the BLACKHOLE (BUG#38360).
> === added file 'mysql-test/include/hierarchical_rpl_for_4_hosts_init.inc'
> --- a/mysql-test/include/hierarchical_rpl_for_4_hosts_init.inc	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/include/hierarchical_rpl_for_4_hosts_init.inc	2008-10-06 14:29:50
> +0000
> @@ -0,0 +1,90 @@
> +#############################################################
> +# Purpose: Setting up hierarchical replication as follows:
> +#                      A  
> +#                      |
> +#                      B  
> +#                    /  \
> +#                   C    D 
> +# 
> +# Notes:
> +# 1. --slave-num=3 must be added to *-master.opt file
> +# 2. Although the test uses new names for servers the file names 
> +#    used for logging are still old:
> +#    master_a -> master.[log|err]
> +#    master_b -> slave.[log|err]
> +#    master_c -> slave1.[log|err]
> +#    master_d -> slave2.[log|err]
> +#
> +#############################################################
> +# I am eliminating this because I think this is wrong.
> +#--source include/master-slave.inc
> +# Please check with Sven and Serge.

I agree, it can be removed.

But please do --source include/have_log_bin.inc.

> +
> +# Configure replica a
> +--echo ** Configuring replica a **
> +connect (master_a,127.0.0.1,root,,test,$MASTER_MYPORT,);
> +RESET MASTER;
> +--disable_warnings
> +STOP SLAVE;
> +--enable_warnings
> +RESET SLAVE;
> +let $_binlog_file= query_get_value(SHOW MASTER STATUS, File, 1);
> +--echo
> +
> +# Configure replica b
> +--echo ** Configuring replica b **
> +connect (master_b,127.0.0.1,root,,test,$SLAVE_MYPORT,);
> +RESET MASTER;
> +--disable_warnings
> +STOP SLAVE;
> +--enable_warnings
> +RESET SLAVE;
> +--replace_result $MASTER_MYPORT MASTER_A_PORT $_binlog_file MASTER_A_LOG_FILE
> +--eval CHANGE MASTER TO
> master_host='127.0.0.1',master_port=$MASTER_MYPORT,master_user='root',MASTER_LOG_FILE='$_binlog_file'
> +let $_binlog_file= query_get_value(SHOW MASTER STATUS, File, 1);
> +--echo
> +
> +# master c
> +--echo ** Configuring replica c **
> +connect (master_c,127.0.0.1,root,,test,$SLAVE_MYPORT1,);
> +RESET MASTER;
> +--disable_warnings
> +STOP SLAVE;
> +--enable_warnings
> +RESET SLAVE;
> +--replace_result $SLAVE_MYPORT MASTER_B_PORT $_binlog_file MASTER_B_LOG_FILE
> +--eval CHANGE MASTER TO
> master_host='127.0.0.1',master_port=$SLAVE_MYPORT,master_user='root',MASTER_LOG_FILE='$_binlog_file'
> +--echo
> +
> +# master d 
> +--echo ** Configuring replica d **
> +connect (master_d,127.0.0.1,root,,test,$SLAVE_MYPORT2,);
> +RESET MASTER;
> +--disable_warnings
> +STOP SLAVE;
> +--enable_warnings
> +RESET SLAVE;
> +--replace_result $SLAVE_MYPORT MASTER_B_PORT $_binlog_file MASTER_B_LOG_FILE
> +--eval CHANGE MASTER TO
> master_host='127.0.0.1',master_port=$SLAVE_MYPORT,master_user='root',MASTER_LOG_FILE='$_binlog_file'
> +--echo
> +
> +# Start hierarchical replication
> +--echo ** Starting hiearchical replication **
> +--connection master_b
> +connect(slave,127.0.0.1,root,,test,$SLAVE_MYPORT);
> +START SLAVE;
> +--source include/wait_for_slave_to_start.inc
> +--disconnect slave
> +
> +--connection master_c
> +connect(slave,127.0.0.1,root,,test,$SLAVE_MYPORT1);
> +START SLAVE;
> +--source include/wait_for_slave_to_start.inc
> +--disconnect slave
> +
> +--connection master_d
> +connect(slave,127.0.0.1,root,,test,$SLAVE_MYPORT2);
> +START SLAVE;
> +--source include/wait_for_slave_to_start.inc
> +--disconnect slave
> +--echo
> 
> === added file 'mysql-test/include/hierarchical_rpl_for_4_hosts_sync.inc'
> --- a/mysql-test/include/hierarchical_rpl_for_4_hosts_sync.inc	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/include/hierarchical_rpl_for_4_hosts_sync.inc	2008-10-06 14:29:50
> +0000
> @@ -0,0 +1,21 @@
> +#############################################################
> +# Purpose: Syncing all hosts for hierarchical replication 
> +# based on the following diagram:
> +#                      A
> +#                      |
> +#                      B  
> +#                    /  \
> +#                   C    D
> +# Notes: see include/hierarchical_rpl_for_4_hosts_init.inc
> +#  
> +#############################################################
> +
> +# Make the full traversal of sync
> +--connection master_a
> +--disable_query_log
> +--sync_slave_with_master master_b
> +--sync_slave_with_master master_c
> +--connection master_b
> +--sync_slave_with_master master_d
> +--connection master_a
> +--enable_query_log
> 
> === added file 'mysql-test/suite/rpl/r/rpl_hierarchical_for_4_hosts.result'
> --- a/mysql-test/suite/rpl/r/rpl_hierarchical_for_4_hosts.result	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_hierarchical_for_4_hosts.result	2008-10-06 14:29:50
> +0000
> @@ -0,0 +1,98 @@
> +*** Setting up hierarchical replication with four nodes ***
> +** Configuring replica a **
> +RESET MASTER;
> +STOP SLAVE;
> +RESET SLAVE;
> +
> +** Configuring replica b **
> +RESET MASTER;
> +STOP SLAVE;
> +RESET SLAVE;
> +CHANGE MASTER TO
> master_host='127.0.0.1',master_port=MASTER_A_PORT,master_user='root',MASTER_LOG_FILE='MASTER_A_LOG_FILE';
> +
> +** Configuring replica c **
> +RESET MASTER;
> +STOP SLAVE;
> +RESET SLAVE;
> +CHANGE MASTER TO
> master_host='127.0.0.1',master_port=MASTER_B_PORT,master_user='root',MASTER_LOG_FILE='MASTER_B_LOG_FILE';
> +
> +** Configuring replica d **
> +RESET MASTER;
> +STOP SLAVE;
> +RESET SLAVE;
> +CHANGE MASTER TO
> master_host='127.0.0.1',master_port=MASTER_B_PORT,master_user='root',MASTER_LOG_FILE='MASTER_B_LOG_FILE';
> +
> +** Starting hiearchical replication **
> +START SLAVE;
> +START SLAVE;
> +START SLAVE;
> +
> +
> +*** Preparing database schema ***
> +CREATE TABLE t1 (a INT NOT NULL, b VARCHAR(100), c INT NOT NULL);
> +CREATE TABLE t2 (a INT NOT NULL, b VARCHAR(100), c INT NOT NULL, PRIMARY KEY(a));
> +
> +*** Testing the hiearchical replication ***
> +INSERT INTO t1(a,b,c) VALUES(1,'A',1);
> +INSERT INTO t1(a,b,c) VALUES(2,'B',1);
> +INSERT INTO t1(a,b,c) VALUES(3,'C',1);
> +INSERT INTO t1(a,b,c) VALUES(4,'D',1);
> +DELETE FROM t1 where b = 'B';
> +INSERT INTO t2(a,b,c) VALUES(1,'A',1);
> +INSERT INTO t2(a,b,c) VALUES(2,'B',1);
> +INSERT INTO t2(a,b,c) VALUES(3,'C',1);
> +INSERT INTO t2(a,b,c) VALUES(4,'D',1);
> +DELETE FROM t2 where a = 1;
> +DELETE FROM t2 where a = 2;
> +DELETE FROM t2 where a = 3;
> +DELETE FROM t2 where a = 4;
> +INSERT INTO t2(a,b,c) VALUES(1,'A',1);
> +INSERT INTO t2(a,b,c) VALUES(2,'B',1);
> +INSERT INTO t2(a,b,c) VALUES(3,'C',1);
> +INSERT INTO t2(a,b,c) VALUES(4,'D',1);
> +UPDATE t2 SET b = 'AA' WHERE a =1;
> +UPDATE t2 SET b = 'BB' WHERE a =2;
> +UPDATE t2 SET b = 'CC' WHERE a =3;
> +UPDATE t2 SET b = 'DD' WHERE a =4;
> +SELECT * FROM t1 WHERE a =4;
> +a	b	c
> +SELECT * FROM t2 WHERE a =4;
> +a	b	c
> +
> +*** Verifying if servers are consistent as expected ***
> +*** Checking replicas a, c and d ***
> +SELECT COUNT(*) FROM t1;
> +COUNT(*)
> +3
> +SELECT COUNT(*) FROM t2;
> +COUNT(*)
> +4
> +SELECT COUNT(*) FROM t1;
> +COUNT(*)
> +3
> +SELECT COUNT(*) FROM t2;
> +COUNT(*)
> +4
> +SELECT COUNT(*) FROM t1;
> +COUNT(*)
> +3
> +SELECT COUNT(*) FROM t2;
> +COUNT(*)
> +4
> +*** Checking replica b ***
> +SELECT COUNT(*) FROM t1;
> +COUNT(*)
> +0
> +SELECT COUNT(*) FROM t2;
> +COUNT(*)
> +0
> +>>> Something was written to binary log <<<
> +
> +*** Cleaning up ***
> +DROP TABLE t1,t2;
> +STOP SLAVE;
> +RESET SLAVE;
> +STOP SLAVE;
> +RESET SLAVE;
> +STOP SLAVE;
> +RESET SLAVE;
> 
> === added file 'mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.cnf'
> --- a/mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.cnf	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.cnf	2008-10-06 14:29:50
> +0000
> @@ -0,0 +1,25 @@
> +!include ../my.cnf
> +
> +[mysqld.1]
> +log-slave-updates
> +default-storage_engine = innodb
> +
> +[mysqld.2]
> +log-slave-updates
> +default-storage_engine = blackhole
> +
> +[mysqld.3]
> +log-slave-updates
> +default-storage_engine = innodb
> +
> +[mysqld.4]
> +log-slave-updates
> +default-storage_engine = innodb
> +
> +[ENV]
> +SLAVE_MYPORT1=		@mysqld.3.port
> +SLAVE_MYSOCK1=		@mysqld.3.socket
> +
> +SLAVE_MYPORT2=		@mysqld.4.port
> +SLAVE_MYSOCK2=		@mysqld.4.socket
> +
> 
> === added file 'mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.test'
> --- a/mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.test	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_hierarchical_for_4_hosts.test	2008-10-06 14:29:50
> +0000
> @@ -0,0 +1,127 @@
> +#############################################################
> +# Purpose: Verifying that replication works with an 
> +# intermediate slave using BLACKHOLE (BUG#38360) storage 
> +# engine, using the replication topology of the following 
> +# diagram:
> +#                      A  (innodb)
> +#                      |
> +#                      B  (blackhole)
> +#                    /  \ 
> +#                   C    D (innodb)
> +#
> +# This test consists in the following steps:
> +# - Verifying requirements innodb and blackhole engine.

Sorry, what does that mean? Not sure what you are referring to...

> +# - Configureing the environment.

speling erorr: remove e from configureing

> +# - Creating the tables.
> +# - Changing database engine in server b.

I don't think you do this? You rely on the default engine specified in
the configuration file?

> +# - Executing several commands.
> +#       - crud statements.
> +#	- crud statements with transactions.

This is the important part of the test: please explain briefly what
types of queries are being done (i.e., each of INSERT, DELETE, UPDATE,
INSERT...SELECT, CREATE...SELECT; using no key, non-primary key, primary
key, etc).

Also, I don't think there are any transactions in the test?

> +# - Verifying that server a, c e d have the same data.

I think some Portuguese sneaked into that sentence :-)

e -> and

> +# - However, b does not have data but its bin log
> +# was updated.
> +#
> +#############################################################
> +--source include/have_innodb.inc
> +--source include/have_blackhole.inc
> +
> +# Set up a hierarchical replication
> +--echo *** Setting up hierarchical replication with four nodes ***
> +--source include/hierarchical_rpl_for_4_hosts_init.inc
> +--echo
> +
> +# Save information from the binlog in the server b
> +connection master_b;
> +let $before = query_get_value("SHOW MASTER STATUS", Position, 1);
> +
> +# Preparing data.
> +--echo *** Preparing database schema ***
> +--connection master_a
> +CREATE TABLE t1 (a INT NOT NULL, b VARCHAR(100), c INT NOT NULL);
> +CREATE TABLE t2 (a INT NOT NULL, b VARCHAR(100), c INT NOT NULL, PRIMARY KEY(a));
> +--source include/hierarchical_rpl_for_4_hosts_sync.inc
> +--echo
> +
> +#
> +# Test the hiearchical replication
> +#
> +--echo *** Testing the hiearchical replication ***
> +--connection master_a
> +INSERT INTO t1(a,b,c) VALUES(1,'A',1);
> +INSERT INTO t1(a,b,c) VALUES(2,'B',1);
> +INSERT INTO t1(a,b,c) VALUES(3,'C',1);
> +INSERT INTO t1(a,b,c) VALUES(4,'D',1);
> +
> +DELETE FROM t1 where b = 'B';

Please try UPDATE on t1 too.

As suggested by Mats on IRC, it would be useful to try "INSERT...SELECT
something FROM t1" and "CREATE...SELECT something FROM t1" too.

> +INSERT INTO t2(a,b,c) VALUES(1,'A',1);
> +INSERT INTO t2(a,b,c) VALUES(2,'B',1);
> +INSERT INTO t2(a,b,c) VALUES(3,'C',1);
> +INSERT INTO t2(a,b,c) VALUES(4,'D',1);
> +
> +DELETE FROM t2 where a = 1;
> +DELETE FROM t2 where a = 2;
> +DELETE FROM t2 where a = 3;
> +DELETE FROM t2 where a = 4;
> +
> +INSERT INTO t2(a,b,c) VALUES(1,'A',1);
> +INSERT INTO t2(a,b,c) VALUES(2,'B',1);
> +INSERT INTO t2(a,b,c) VALUES(3,'C',1);
> +INSERT INTO t2(a,b,c) VALUES(4,'D',1);
> +
> +UPDATE t2 SET b = 'AA' WHERE a =1;
> +UPDATE t2 SET b = 'BB' WHERE a =2;
> +UPDATE t2 SET b = 'CC' WHERE a =3;
> +UPDATE t2 SET b = 'DD' WHERE a =4;

I'd suggest an optimization: if update comes before delete, we don't
have to insert all the rows again. Also, I think it's enough to test
each scenario once (we don't need all 4 rows).

Please use INSERT...SELECT and CREATE...SELECT for t2 too.


As suggested by Mats on IRC, please also try
insert/delete/update/insert...select/create...select, using:

 (1) a multi-column primary key where only some of the columns are used;

 (2) a multi-column primary key where only some of the columns are used,
and also some columns that are not in the key;

 (3) use a key that is not primary;

 (4) like (1) and (2), but use a key that is not primary;

 (5) like (3) and (4), but make sure that more than one row match the key;


This means the test may become quite big, but it's quite an important
bug, and blackhole is not well tested. Hopefully you can cut and paste
most of it...


> +
> +--connection master_b
> +SELECT * FROM t1 WHERE a =4;
> +SELECT * FROM t2 WHERE a =4;
> +
> +--source include/hierarchical_rpl_for_4_hosts_sync.inc
> +--echo 
> +
> +# Verify if servers are consistent as expected
> +--echo *** Verifying if servers are consistent as expected ***
> +--echo *** Checking replicas a, c and d ***
> +--connection master_a
> +SELECT COUNT(*) FROM t1;
> +SELECT COUNT(*) FROM t2;
> +
> +--connection master_c
> +SELECT COUNT(*) FROM t1;
> +SELECT COUNT(*) FROM t2;
> +
> +--connection master_d
> +SELECT COUNT(*) FROM t1;
> +SELECT COUNT(*) FROM t2;
> +
> +--echo *** Checking replica b ***
> +--connection master_b
> +SELECT COUNT(*) FROM t1;
> +SELECT COUNT(*) FROM t2;
> +
> +let $after = query_get_value("SHOW MASTER STATUS", Position, 1);
> +let $something_written = `select $after - $before != 0`;
> +if ($something_written) {
> +  --echo >>> Something was written to binary log <<<
> +}
> +if (!$something_written) {
> +  --echo >>> Nothing was written to binary log <<<
> +}
> +--echo 

I have no strong opinion, but I think this can be removed (if nothing is
written to B's binlog, then we would detect that when comparing A and C
or when comparing A and D).

If you decide to keep it, I'd suggest shortening it a bit: we expect
that something is written, so there's no need to have a case for when
it's not written, the test can just fail:

let $after = query_get_value("SHOW MASTER STATUS", Position, 1);
let $something_written= `SELECT $after - $before != 0`;
--echo Something written to binlog (expect 1): $something_written

> +
> +# Clean up
> +--echo *** Cleaning up ***
> +--connection master_a
> +DROP TABLE t1,t2;
> +--source include/hierarchical_rpl_for_4_hosts_sync.inc
> +--connection master_b
> +STOP SLAVE;
> +RESET SLAVE;
> +--connection master_c
> +STOP SLAVE;
> +RESET SLAVE;
> +--connection master_d
> +STOP SLAVE;
> +RESET SLAVE;
> 
> 


-- 
Sven Sandberg, Software Engineer
MySQL AB, www.mysql.com