Hi Sven,
Here are some review comments.
Please check it.
>
[snip]
>
>
> === modified file 'mysql-test/extra/rpl_tests/rpl_loaddata.test'
> --- a/mysql-test/extra/rpl_tests/rpl_loaddata.test 2010-02-04 11:26:36 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_loaddata.test 2010-07-26 13:31:58 +0000
>
> @@ -165,6 +152,8 @@ terminated by ',' optionally enclosed by
> connection slave;
> --source include/wait_for_slave_sql_to_stop.inc
> drop table t1, t2;
> +--source include/stop_slave_io.inc
> +RESET SLAVE;
We don't need to reset slave here, as the following test already adds
'source include/rpl_reset.inc'.
> connection master;
> drop table t1, t2;
>
> @@ -178,7 +167,8 @@ DROP TABLE IF EXISTS t1;
>
> # BUG#48297: Schema name is ignored when LOAD DATA is written into binlog,
> # replication aborts
> --- source include/master-slave-reset.inc
> +-- let $rpl_only_running_threads= 1
> +-- source include/rpl_reset.inc
see above.
>
> -- let $db1= b48297_db1
> -- let $db2= b42897_db2
> @@ -251,7 +241,7 @@ source include/diff_tables.inc;
> -- sync_slave_with_master
>
> # BUG#49479: LOAD DATA INFILE is binlogged without escaping field names
> --- source include/master-slave-reset.inc
> +-- source include/rpl_reset.inc
> -- connection master
> use test;
> CREATE TABLE t1 (`key` TEXT, `text` TEXT);
>
[snip]
>
> === modified file 'mysql-test/extra/rpl_tests/rpl_reset_slave.test'
> --- a/mysql-test/extra/rpl_tests/rpl_reset_slave.test 2010-05-24 13:54:08 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_reset_slave.test 2010-07-26 13:31:58 +0000
> @@ -54,9 +54,9 @@ source include/check_slave_no_error.inc;
> change master to master_user='impossible_user_name';
> start slave;
> let $slave_io_errno= 1045;
> -source include/wait_for_slave_io_error.inc;
> +--source include/wait_for_slave_io_error_and_stop.inc
> +--source include/stop_slave_sql.inc
>
> -source include/stop_slave.inc;
> change master to master_user='root';
> source include/start_slave.inc;
> source include/check_slave_no_error.inc;
> @@ -71,6 +71,19 @@ start slave;
> let $slave_io_errno= 1045;
> source include/wait_for_slave_io_error.inc;
>
> -source include/stop_slave.inc;
> +# Now, Slave_IO_Error is set and Slave_IO_Running=NO, but the thread
> +# is still trying to connect, i.e.,
> +# Master_info::slave_running==MYSQL_RUN_NOT_CONNECT. In order to run
> +# CHANGE MASTER, we must stop the IO thread completely. Hence we do
> +# STOP SLAVE, and then wait for the Slave_IO_State to become empty.
> +STOP SLAVE;
It should stop completely, as 1045 is a fatal error.
so using stop_slave_sql.inc is enough.
> +--source include/wait_for_slave_sql_to_stop.inc
> +--let $slave_param= Slave_IO_State
> +--let $slave_param_value=
> +--source include/wait_for_slave_param.inc
> +
> reset slave;
> source include/check_slave_no_error.inc;
> +
> +--let $rpl_only_running_threads= 1
> +--source include/rpl_end.inc
>
[snip]
> === modified file 'mysql-test/extra/rpl_tests/rpl_stm_EE_err2.test'
> --- a/mysql-test/extra/rpl_tests/rpl_stm_EE_err2.test 2008-11-13 19:19:00 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_stm_EE_err2.test 2010-07-26 13:31:58 +0000
> @@ -30,6 +30,10 @@ let $errno= query_get_value(SHOW SLAVE S
> --echo Error: "$error" (expected different error codes on master and slave)
> --echo Errno: "$errno" (expected 0)
> drop table t1;
> +--source include/stop_slave.inc
> +RESET SLAVE;
It aims to clear the error message, right?
Please add a comment to explain it.
>
> # End of 4.1 tests
>
> +--let $rpl_only_running_threads= 1
> +--source include/rpl_end.inc
>
> === added file 'mysql-test/extra/rpl_tests/rpl_test_framework.inc'
> --- a/mysql-test/extra/rpl_tests/rpl_test_framework.inc 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/extra/rpl_tests/rpl_test_framework.inc 2010-07-26 13:31:58 +0000
> @@ -0,0 +1,74 @@
> +# ==== Purpose ====
> +#
> +# Auxiliary file used by suite/rpl/t/rpl_test_framework.test
> +#
> +# The purpose is to check that the sync chain generated in
> +# rpl_change_topology.inc (invoked from rpl_init.inc) is correct. This
> +# is done in two ways:
> +# (1) Print the sync chain.
> +# (2) Execute a statement and verify that it replicates to all slaves.
> +#
> +#
> +# ==== Implementation ====
> +#
> +# Does this:
> +# (1) Set up a given replication topology (rpl_init.inc)
> +# (2) Print $rpl_sync_chain
> +# (3) Execute "DELETE FROM t1" and then "INSERT INTO t1" on the master
> +# (4) Sync and compare all servers.
> +# (5) Clean up the replication topology (rpl_end.inc)
> +#
> +# (Technical detail: Since DELETE FROM t1 is not executed at the end,
> +# some servers may have rows left in t1 from a previous invokation of
typo: invocation
> +# rpl_test_framework.inc. Therefore, this file will only work in
> +# statement mode where "DELETE FROM t1" removes rows that exist on
> +# slave but not on master.)
> +#
> +#
> +# ==== Usage ====
> +#
> +# --let $rpl_server_count= <number>
> +# --let $rpl_topology= <topology specification>
> +# --let $masters= <list of masters>
> +# [--let $rpl_diff_servers= <list of servers>]
> +# --source extra/rpl_tests/rpl_test_framework.inc
> +#
> +# Parameters:
> +# $rpl_server_count, $rpl_topology:
> +# See include/rpl_init.inc
> +#
> +# $masters
> +# This should be a list of numbers, each identifying a server.
> +# The DELETE and INSERT statements will be executed on all servers
> +# in the list.
> +#
> +# $rpl_diff_servers
> +# See include/rpl_diff.inc
> +
> +--source include/rpl_init.inc
> +--echo rpl_sync_chain= '$rpl_sync_chain'
> +
> +--inc $next_number
> +
> +if ($masters)
> +{
> + # Iterate over masters
> + while ($masters)
> + {
> + --let $master_i= `SELECT SUBSTRING_INDEX('$masters', ',', 1)`
> + --let $masters= `SELECT SUBSTRING('$masters', LENGTH('$master_i') + 2)`
> +
> + # Connect to master and execute statement
> + --let $rpl_connection_name= server_$master_i
> + --source include/rpl_connection.inc
> + DELETE FROM t1;
> + --eval INSERT INTO t1 VALUES ($next_number)
> + }
> +
> + # Compare all servers.
> + --let $rpl_diff_statement= SELECT * FROM t1
> + --source include/rpl_diff.inc
> +}
> +
> +--let $rpl_diff_servers=
> +--source include/rpl_end.inc
>
> === modified file 'mysql-test/include/analyze-sync_with_master.test'
> --- a/mysql-test/include/analyze-sync_with_master.test 2008-08-04 19:54:44 +0000
> +++ b/mysql-test/include/analyze-sync_with_master.test 2010-07-26 13:31:58 +0000
What is this file for?
> @@ -1,6 +1,2 @@
> -SHOW PROCESSLIST;
> -
> -let $binlog_name= query_get_value("SHOW MASTER STATUS", File, 1);
> -eval SHOW BINLOG EVENTS IN '$binlog_name';
> -
> -exit;
> \ No newline at end of file
> +--let $rpl_only_current_connection= 1
> +--source include/show_rpl_debug_info.inc
>
> === modified file 'mysql-test/include/check-testcase.test'
> --- a/mysql-test/include/check-testcase.test 2008-12-13 19:42:12 +0000
> +++ b/mysql-test/include/check-testcase.test 2010-07-26 13:31:58 +0000
> @@ -8,8 +8,10 @@
> # any unwanted side affects.
> #
> --disable_query_log
> +--replace_column 5 # 6 # 7 # 8 # 9 # 10 # 22 # 23 # 24 # 25 # 26 #
> +query_vertical
> +SHOW SLAVE STATUS;
> +
Can't we use show_slave_status.inc here?
> call mtr.check_testcase();
> --enable_query_log
>
> -
> -
>
[snip]
> === modified file 'mysql-test/include/have_innodb.inc'
> --- a/mysql-test/include/have_innodb.inc 2010-03-31 13:04:40 +0000
> +++ b/mysql-test/include/have_innodb.inc 2010-07-26 13:31:58 +0000
> @@ -1,4 +1,4 @@
> -disable_query_log;
> ---require r/true.require
> -select (support = 'YES' or support = 'DEFAULT' or support = 'ENABLED') as `TRUE`
> from information_schema.engines where engine = 'innodb';
> -enable_query_log;
> +if (`SELECT COUNT(*) = 0 FROM INFORMATION_SCHEMA.ENGINES WHERE engine = 'innodb' AND
> (support = 'YES' OR support = 'DEFAULT' OR support = 'ENABLED')`)
how about 'AND support IN('YES', 'DEFAULT', 'ENABLED')`)'
> +{
> + --skip Test requires InnoDB.
> +}
>
>
>
[snip]
>
> === added file 'mysql-test/include/rpl_begin_include_file.inc'
> --- a/mysql-test/include/rpl_begin_include_file.inc 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/include/rpl_begin_include_file.inc 2010-07-26 13:31:58 +0000
> @@ -0,0 +1,120 @@
> +# ==== Purpose ====
> +#
> +# This is an auxiliary file that facilitates writing include/*.inc
> +# files that invoke each other and reset the correct state afterwards.
> +# Assume that both rpl_a.inc and rpl_b.inc execute 'source
> +# include/rpl_begin_include_file.inc' at the beginning and 'source
> +# include/rpl_end_include_file.inc' at the end, and assume further
> +# that rpl_a.inc needs to source rpl_b.inc. Then:
> +#
> +# - When a user sources include/rpl_b.inc directly, the text
> +# 'include/rpl_b.inc' is written to the query log. When a user
> +# sources rpl_a.inc, the text 'include/rpl_a.inc' is written to the
> +# query log (but the text include/rpl_b.inc is not written; i.e., the
> +# internal workings of include/rpl_a.inc is not exposed to the
> +# query log).
> +#
> +# - If rpl_a.inc and/or rpl_b.inc needs to disable the query log,
> +# then they can specify this to rpl_begin_include_file.inc and
> +# rpl_end_include_file.inc. If the user invokes rpl_b.inc or
> +# rpl_a.inc, then rpl_b.inc will enable the query log at the end of
> +# rpl_b.inc. However, if rpl_a.inc needs the query log to be off,
> +# then rpl_b.inc will not turn on the query log when rpl_a.inc
> +# sources rpl_b.inc.
Could you please describe it in another way, I could not get the idea.
> +#
> +# - Warnings can be disabled similarly to how the query log is
> +# disabled.
> +#
> +# - If the mysqltest variable $rpl_debug is set, then
> +# rpl_begin_include_file.inc will print
> +# '==== BEGIN include/<filename> ====' and rpl_end_include_file.inc
> +# will print '==== END include/<filename> ===='.
> +#
> +#
> +#
>
> === added file 'mysql-test/include/rpl_change_topology.inc'
> --- a/mysql-test/include/rpl_change_topology.inc 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/include/rpl_change_topology.inc 2010-07-26 13:31:58 +0000
> @@ -0,0 +1,351 @@
> +# ==== Purpose ====
> +#
> +# Changes replication topology. This file is normally sourced from
> +# include/rpl_init.inc, but test cases can also source it if they
> +# need to change topology after they have sourced include/rpl_init.inc
> +#
> +# This file sets up variables needed by include/rpl_sync.inc and many
> +# other replication scripts in the include/ directory. It also issues
> +# CHANGE MASTER on all servers where the configuration changes from
> +# what it was before. It does not issue START SLAVE (use
> +# include/rpl_start_slaves.inc for that).
> +#
> +# Note: it is not currently possible to change the number of servers
> +# after the rpl_init.inc, without first calling rpl_end.inc. So the
> +# test has to set $rpl_server_count to the total number of servers
> +# that the test uses, before it sources include/rpl_init.inc. After
> +# that, $rpl_server_count must not change until after next time the
> +# test sources include/rpl_end.inc.
> +#
> +# Note: Since this script issues CHANGE MASTER, the test case must
> +# ensure that all slaves where the configuration changes have stopped
> +# both the IO thread and the SQL thread before this script is sourced.
> +#
> +#
> +# ==== Usage ====
> +#
> +# --let $rpl_server_count= 6
hmm, I think rpl_server_count can get from rpl_topology.
We can use the max number in rpl_topology.
> +# --let $rpl_topology= 1->2->3->1->4, 2->5, 6->7
> +# [--let $rpl_skip_change_master= 1]
> +# [--let $rpl_master_log_file= 1:master-bin.000001,3:master-bin.000003]
> +# [--let $rpl_master_log_pos= 1:4711,3:107]
> +# [--let $rpl_debug= 1]
> +# --source include/rpl_change_topology.inc
> +#
> +# Parameters:
> +# $rpl_master_log_file
> +# By default, CHANGE MASTER is executed with MASTER_LOG_FILE set
> +# to the name of the last binlog file on the master (retrieved by
> +# executing SHOW MASTER STATUS). This variable can be set to
> +# specify another filename. This variable should be a
> +# comma-separated list of the following form:
> +#
> +# SERVER_NUMBER_1:FILE_NAME_1,SERVER_NUMBER_2:FILE_NAME_2,...
> +#
> +# Before CHANGE MASTER is executed on server N, this script checks
> +# if $rpl_master_log_file contains the text N:FILE_NAME. If it
> +# does, then MASTER_LOG_FILE is set to FILE_NAME. Otherwise,
> +# MASTER_LOG_FILE is set to the last binlog on the master. For
> +# example, to specify that server_1 should start replicate from
> +# master-bin.000007 and server_5 should start replicate from
> +# master-bin.012345, do:
> +# --let $rpl_master_log_file= 1:master-bin.000007,5:master-bin.012345
> +#
> +# $rpl_master_log_pos
> +# By default, CHANGE MASTER is executed without specifying the
> +# MASTER_LOG_POS parameter. This variable can be set to set a
> +# specific position. It has the same form as $rpl_master_log_file
> +# (see above). For example, to specify that server_3 should start
> +# replicate from position 4711 of its master, do:
> +# --let $rpl_master_log_pos= 3:4711
> +#
> +# $rpl_server_count, $rpl_topology, $rpl_debug, $rpl_skip_change_master
> +# See include/rpl_init.inc
> +#
> +#
> +# ==== Side effects ====
> +#
> +# Turns on enable_query_log.
> +# Does not change the status of enable_warnings.
> +# Does not change current connection.
> +#
> +#
> +# ==== Internal variables configured by this file ====
> +#
> +# This file sets up the following variables, which are used by other
> +# low-level replication files such as:
> +# include/rpl_sync.inc
> +# include/rpl_start_slaves.inc
> +# include/rpl_stop_slaves.inc
> +# include/rpl_end.inc
> +#
> +# $rpl_server_count_length:
> +# Set to LENGTH($rpl_server_count). So if $rpl_server_count < 10,
> +# then $rpl_server_count_length = 1; if 10 <= $rpl_server_count <
> +# 100, then $rpl_server_count_length = 2, etc.
> +#
> +# $rpl_master_list
> +# Set to a string consisting of $rpl_server_count numbers, each one
> +# whitespace-padded to $rpl_server_count_length characters. If
> +# server N is a slave, then the N'th number is the master of server
> +# N. If server N is not a slave, then the N'th number is just spaces
> +# (so in fact it is not a number). For example, if $rpl_topology is
> +# '1->2,2->3,3->1,2->4,5->6', then $rpl_master_list is '3122 6'.
> +#
> +# $rpl_sync_chain
> +# Set to a string that specifies in what order servers should be
> +# synchronized in include/rpl_sync.inc. This has the form of a
> +# sequence of "chains". Each chain begins with
> +# $rpl_server_count_length space characters, followed by a sequence
> +# of numbers, each number whitespace-padded to
> +# $rpl_server_count_length characters. Each number in the sequence
> +# denotes a server, and the N'th server is a master of the (N+1)'th
> +# server. For example, if $rpl_topology is
> +# '1->2,2->3,3->1,2->4,5->6', then $rpl_sync_chain is ' 56
> 123124'.
> +
> +
> +# Remove whitespace from $rpl_topology
> +--let $rpl_topology= `SELECT REPLACE('$rpl_topology', ' ', '')`
> +
> +--let $rpl_include_filename= rpl_change_topology.inc [new topology=$rpl_topology]
> +--let $rpl_disable_query_log= 1
> +--source include/rpl_begin_include_file.inc
> +
> +
> +--let $_rpl_change_topology_old_connection= $CURRENT_CONNECTION
> +
> +
> +if ($rpl_debug)
> +{
> + --echo ---- Check input ----
> +}
> +
> +
> +if (`SELECT '$rpl_topology' = '' OR '$rpl_server_count' = ''`)
> +{
> + --die You must set $rpl_topology and $rpl_server_count before you source
> rpl_change_topology.inc. If you really want to change to the empty topology, set
> $rpl_topology= none
Could you please add the description into rpl_init.inc?
"If you really want to change to the empty topology, set $rpl_topology=
none"
> +}
[snip]
> +--let $_rpl_topology= $rpl_topology
> +if (`SELECT '$_rpl_topology' = 'none'`)
> +{
> + --let $_rpl_topology=
> +}
It is not need. 'none' will be treated as 0.
> +if (`SELECT '!$rpl_master_list!' = '!!'`)
> +{
> + --die You must source include/rpl_init.inc before you source
> include/rpl_change_topology.inc
> +}
> +--let $_rpl_old_master_list= $rpl_master_list
> +
> +if ($rpl_debug)
> +{
> + --echo \$rpl_server_count='$rpl_server_count'
> + --echo \$rpl_server_count_length='$rpl_server_count_length'
> + --echo new \$rpl_topology='$_rpl_topology'
> + --echo old \$rpl_master_list='$rpl_master_list'
> + --echo old \$rpl_sync_chain='$rpl_sync_chain'
> +}
> +
> +
> +if ($rpl_debug)
> +{
> + --echo ---- Generate \$rpl_server_count_length and \$rpl_master_list ----
> +}
> +
> +--let $rpl_server_count_length= `SELECT LENGTH('$rpl_server_count')`
> +--let $rpl_master_list=
> +--let $_rpl_no_server= `SELECT REPEAT(' ', $rpl_server_count_length)`
> +--let $rpl_master_list= `SELECT REPEAT('$_rpl_no_server', $rpl_server_count)`
> +while ($_rpl_topology)
> +{
> + # Get 's1->s2' from 's1->s2->s3->...' or from
> 's1->s2,s3->s4,...'
Please use '1->2' to replace 's1->s2', etc.
> + --let $_rpl_master_slave= `SELECT
> SUBSTRING_INDEX(SUBSTRING_INDEX('$_rpl_topology', ',', 1), '->', 2)`
> + # Remove 's1->s2,' from 's1->s2,s3->s4,...' or remove
> + # 's1->' from 's1->s2->...'
> + --let $_rpl_topology= `SELECT SUBSTR('$_rpl_topology', IF(SUBSTR('$_rpl_topology',
> LENGTH('$_rpl_master_slave') + 1, 1) = ',', LENGTH('$_rpl_master_slave'), LOCATE('->',
> '$_rpl_master_slave')) + 2)`
> + # Get 's1' from 's1->s2'
> + --let $_rpl_master= `SELECT SUBSTRING_INDEX('$_rpl_master_slave', '->', 1)`
> + # Get 's2' from 's1->s2'
> + --let $_rpl_slave= `SELECT SUBSTRING('$_rpl_master_slave', LENGTH('$_rpl_master')
> + 3)`
> + # Check that s2 does not have another master.
> + if (`SELECT LOCATE('->$_rpl_slave', '$_rpl_topology')`)
> + {
> + --echo ERROR IN TEST: Server $_rpl_slave has more than one master in topology
> '$rpl_topology'
> + --die ERROR IN TEST: found a server with more than one master in the
> $rpl_topology variable
> + }
> + # Save 's1' at position 's2' in $rpl_master_list
> + --let $rpl_master_list= `SELECT INSERT('$rpl_master_list', 1 + ($_rpl_slave - 1) *
> $rpl_server_count_length, $rpl_server_count_length, RPAD('$_rpl_master',
> $rpl_server_count_length, ' '))`
> +}
> +
> +
>
[snip]
> === added file 'mysql-test/include/rpl_connect.inc'
> --- a/mysql-test/include/rpl_connect.inc 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/include/rpl_connect.inc 2010-07-26 13:31:58 +0000
> @@ -0,0 +1,76 @@
> +# ==== Purpose ====
> +#
> +# Create a connection to a given numbered server.
> +#
> +# This script is normally used internally by rpl_init.inc and
> +# master-slave.inc, but it can also be used in test cases that need to
> +# create more connections or re-create connections after disconnect.
> +#
> +#
> +# ==== Usage ====
> +#
> +# --let $rpl_connection_name= <connection_name>
> +# [--let $rpl_server_number= <server_number>]
> +# [--let $rpl_debug= 1]
> +# --source include/rpl_connect.inc
> +#
> +# Parameters:
> +# $rpl_connection_name
> +# The name of the connection to create.
> +#
> +# $rpl_server_number
> +# By default, $rpl_connection_name will be parsed to get the
> +# server number. The following values for $rpl_connection_name are
> +# recognized:
> +#
> +# server_1, server_1_1, master, master1 - 1st server
> +# server_2, server_2_1, slave, slave1 - 2nd server
> +# server_3, server_3_1 - 3rd server
> +# server_4, server_4_1 - 4th server
> +# ...
> +# server_N, server_N_1 - Nth server
> +#
> +# Here, N = $rpl_server_count, as specified before rpl_init.inc
> +# was sourced.
> +#
> +# However, if $rpl_server_number is set, then $rpl_connection_name
> +# is not parsed and $rpl_server_number is used instead.
> +#
> +# $rpl_debug
> +# See include/rpl_init.inc
> +
> +
> +--let $rpl_include_filename= rpl_connect.inc [creating $rpl_connection_name]
> +--source include/rpl_begin_include_file.inc
> +
> +
> +# Get server number for this connection.
> +if (!$rpl_server_number)
> +{
> + --let $rpl_server_number= `SELECT IF('$rpl_connection_name' = 'master' OR
> '$rpl_connection_name' = 'master1', 1, IF('$rpl_connection_name' = 'slave' OR
> '$rpl_connection_name' = 'slave1', 2, IF('$rpl_connection_name' REGEXP '^server_[0-9]+',
> SUBSTRING_INDEX(SUBSTR('$rpl_connection_name', 8), '_', 1), '')))`
I think the caller have to set rpl_server_number and we don't need to
get server's number from the name.
> + if (!$rpl_server_number)
> + {
> + --echo Bug in test case: \$rpl_connection_name = '$rpl_connection_name' is not a
> known connection
> + --die Bug in test case: $rpl_connection_name is not a known connection
> + }
> +}
> +
> +# Get port number
> +--let $_rpl_port= \$SERVER_MYPORT_$rpl_server_number
> +if (`SELECT '$_rpl_port' = ''`)
> +{
> + --echo Bug in test case: '\$SERVER_MYPORT_$rpl_server_number' not initialized.
> Check the test's .cfg file.
> + --die Not all SERVER_MYPORT_* environment variables are setup correctly.
> +}
> +
> +# Create connection.
> +if ($rpl_debug)
> +{
> + --echo connect ($rpl_connection_name,127.0.0.1,root,,test,$_rpl_port,)
> +}
> +--connect ($rpl_connection_name,127.0.0.1,root,,test,$_rpl_port,)
> +
> +--let $rpl_server_number=
> +
> +--let $rpl_include_filename= rpl_connect.inc
> +--source include/rpl_end_include_file.inc
>
> === added file 'mysql-test/include/rpl_connection.inc'
> --- a/mysql-test/include/rpl_connection.inc 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/include/rpl_connection.inc 2010-07-26 13:31:58 +0000
> @@ -0,0 +1,47 @@
> +# ==== Purpose ====
> +#
> +# The same as 'connection $rpl_connection_name', but it can also
> +# prints the connection name. The connection is printed if $rpl_debug
> +# is set, or if rpl_connection.inc is not called between two
> +# invokations of rpl_begin_include_file.inc/rpl_end_include_file.inc.
typo: invocation.
> +# Otherwise the connection name is not printed.
> +#
> +#
> +
>
> === renamed file 'mysql-test/include/diff_master_slave.inc' =>
> 'mysql-test/include/rpl_diff.inc'
> --- a/mysql-test/include/diff_master_slave.inc 2009-03-27 05:19:50 +0000
> +++ b/mysql-test/include/rpl_diff.inc 2010-07-26 13:31:58 +0000
How about expanding diff_tables.inc to support a diff_statement?
It is easy to maintain.
>
> ---echo source include/diff_master_slave.inc;
> -disable_query_log;
> -disable_result_log;
> + # Execute statement
> + --let $_rpl_diff_file=
> $MYSQLTEST_VARDIR/tmp/_rpl_diff_server-$_rpl_diff_server_i.tmp
> + --exec $MYSQL --defaults-group-suffix=.$_rpl_diff_server_i $_rpl_diff_database
> < $_rpl_diff_statement_file > $_rpl_diff_file
Please make sure it can work well on Windows platforms.
>
>
[snip]
>
> === renamed file 'mysql-test/include/master-slave-end.inc' =>
> 'mysql-test/include/rpl_end.inc'
> --- a/mysql-test/include/master-slave-end.inc 2006-04-12 13:56:29 +0000
> +++ b/mysql-test/include/rpl_end.inc 2010-07-26 13:31:58 +0000
> @@ -1,6 +1,90 @@
> ---connection master
> ---sync_slave_with_master
> ---connection slave
> ---disable_query_log
> -STOP SLAVE;
> ---enable_query_log
> +# ==== Purpose ====
> +#
> +#
> +
> +--source include/rpl_sync.inc
> +--source include/rpl_stop_slaves.inc
> +--let $rpl_topology= 1->2
> +--source include/rpl_change_topology.inc
> +
Why do you change the rpl topology?
> +
> +--connection default
> +--let $_rpl_server= $rpl_server_count
> +--let $_rpl_one= _1
> +while ($_rpl_server)
> +{
> + --disconnect server_$_rpl_server
> + --disconnect server_$_rpl_server$_rpl_one
> + --dec $_rpl_server
> +}
> +
> +
> +--let $rpl_include_filename= rpl_end.inc
> +--source include/rpl_end_include_file.inc
>
[snip]
>
> === renamed file 'mysql-test/include/circular_rpl_for_4_hosts_init.inc' =>
> 'mysql-test/include/rpl_init.inc'
> --- a/mysql-test/include/circular_rpl_for_4_hosts_init.inc 2008-04-25 16:54:42 +0000
> +++ b/mysql-test/include/rpl_init.inc 2010-07-26 13:31:58 +0000
> @@ -1,130 +1,229 @@
> +# Check that $rpl_server_count is set
> +if (`SELECT '$rpl_server_count' NOT REGEXP '^[0-9]+\$'`)
> +{
> + --die You must set the mysqltest variable $rpl_server_count to a number before you
> source include/rpl_init.inc
> +}
Could you please check SERVER_MYPORT_N is not null(n is
1-$rpl_server_count)?
> +
> +
> +
>
[snip]
>
> === renamed file 'mysql-test/include/circular_rpl_for_4_hosts_sync.inc' =>
> 'mysql-test/include/rpl_sync.inc'
> --- a/mysql-test/include/circular_rpl_for_4_hosts_sync.inc 2008-04-24 20:41:04 +0000
> +++ b/mysql-test/include/rpl_sync.inc 2010-07-26 13:31:58 +0000
> +--let $_rpl_i= 1
> +--let $_rpl_connect= 0
> +while ($_rpl_i) {
> + --let $_rpl_server= `SELECT TRIM(SUBSTR('$rpl_sync_chain', 1 + ($_rpl_i - 1) *
> $rpl_server_count_length, $rpl_server_count_length))`
> +
Please add comment to explain that rpl_sync_chain always starts with
' ', so the first $_rpl_server is always null.
> + if ($_rpl_server)
> + {
> + if ($rpl_debug)
> + {
> + --echo [sync server_$_rpl_prev_server -> server_$_rpl_server]
> + }
> + if ($rpl_only_running_threads)
> + {
> + --connection server_$_rpl_server
> + --let $_rpl_slave_io_running= query_get_value(SHOW SLAVE STATUS,
> Slave_IO_Running, 1)
> + --let $_rpl_slave_sql_running= query_get_value(SHOW SLAVE STATUS,
> Slave_SQL_Running, 1)
> + if ($rpl_debug)
> + {
> + --echo Sync IO: $_rpl_slave_io_running; Sync SQL: $_rpl_slave_sql_running
> + }
> + --let $_rpl_slave_io_running= `SELECT IF('$_rpl_slave_io_running' = 'Yes', 1,
> '')`
> + --let $_rpl_slave_sql_running= `SELECT IF('$_rpl_slave_sql_running' = 'Yes',
> 1, '')`
> + if ($_rpl_slave_io_running)
> + {
> + --connection server_$_rpl_prev_server
> + if ($_rpl_slave_sql_running)
> + {
> + if ($rpl_debug)
> + {
> + --let $_rpl_master_file= query_get_value("SHOW MASTER STATUS", File, 1)
> + --let $_rpl_master_pos= query_get_value("SHOW MASTER STATUS", Position,
> 1)
> + --echo syncing master_file='$_rpl_master_file'
> master_pos='$_rpl_master_pos'
> + }
> + --sync_slave_with_master server_$_rpl_server
> + }
> + if (!$_rpl_slave_sql_running)
> + {
> + --source include/sync_slave_io_with_master.inc
> + }
> + }
> + if (!$_rpl_slave_io_running)
> + {
> + if ($_rpl_slave_sql_running)
> + {
> + --source include/sync_slave_sql_with_io.inc
> + }
> + }
> + }
> + if (!$rpl_only_running_threads)
> + {
> + --connection server_$_rpl_prev_server
> + if ($rpl_debug)
> + {
> + --let $_rpl_master_file= query_get_value("SHOW MASTER STATUS", File, 1)
> + --let $_rpl_master_pos= query_get_value("SHOW MASTER STATUS", Position, 1)
> + --echo syncing master_file='$_rpl_master_file'
> master_pos='$_rpl_master_pos'
> + }
> + --sync_slave_with_master server_$_rpl_server
> + }
> + }
> +
[snip]
> + # This happens at the beginning of a new sync subchain and at the
> + # end of the full sync chain.
> + if (!$_rpl_server)
> + {
> + --inc $_rpl_i
> + --let $_rpl_server= `SELECT TRIM(SUBSTR('$rpl_sync_chain', 1 + ($_rpl_i - 1) *
> $rpl_server_count_length, $rpl_server_count_length))`
> +
> + if (!$_rpl_server)
> + {
> + # terminate loop
> + --let $_rpl_i= -1
> + }
> + }
> +
Could you please put this snip before 'if ($_rpl_server)', as it handles
the first $_rpl_server which is null. It is more clear if we put it at
the begin for the loop.
> + --let $_rpl_prev_server= $_rpl_server
> + --inc $_rpl_i
> +}
> +
> +
> +--let $rpl_connection_name= $_rpl_sync_old_connection
> +--source include/rpl_connection.inc
> +
> +
> +--let $rpl_include_filename= rpl_sync.inc
> +--source include/rpl_end_include_file.inc
>
>
[snip]
> === modified file 'mysql-test/include/show_rpl_debug_info.inc'
> --- a/mysql-test/include/show_rpl_debug_info.inc 2010-05-24 13:54:08 +0000
> +++ b/mysql-test/include/show_rpl_debug_info.inc 2010-07-26 13:31:58 +0000
> +
> +
> +--enable_query_log
> +--enable_result_log
> +--enable_warnings
> +--disable_abort_on_error
> +--horizontal_results
> +
> +
> +--let $_rpl_old_con= $CURRENT_CONNECTION
> +--let $_rpl_is_first_server= 1
$_rpl_is_first_server can be removed.
see below code.
> +--let $_rpl_server= $rpl_server_count
> +--inc $_rpl_server
'inc $_rpl_server' should be removed.
> +
>
> -let $_master_con= $master_connection;
> -if (`SELECT '$_master_con' = ''`)
> +while ($_rpl_server)
> {
> - if (`SELECT '$_con' = 'slave'`)
> + if (!$_rpl_is_first_server)
Replaced by
+ if (!$rpl_only_current_connection)
> {
> - let $_master_con= master;
> + --connection server_$_rpl_server
> }
[snip]
> +
> + --let $_rpl_is_first_server= 0
Remove it.
> + --dec $_rpl_server
> + # Don't use same connection twice.
[snip]
> + if (`SELECT 'server_$_rpl_server' = '$_rpl_old_con'`)
> {
> - eval SHOW BINLOG EVENTS IN '$master_binlog_name_io';
> + --dec $_rpl_server
> + if ($rpl_only_current_connection)
> + {
> + --let $_rpl_server= 0
> + }
> }
Replaced by
+ if ($rpl_only_current_connection)
+ {
+ --let $_rpl_server= 0
+ }
>
> === modified file 'mysql-test/include/show_slave_status.inc'
> --- a/mysql-test/include/show_slave_status.inc 2010-05-24 13:54:08 +0000
> +++ b/mysql-test/include/show_slave_status.inc 2010-07-26 13:31:58 +0000
> +#
> +# When none of the above applies, you may use this script instead.
> +# However, take care so that the test never contains explicit binlog
> +# coordinates. Usually you can read the binlog coordinate into a
> +# variable and compare it to some other coordinate.
typo: coordinates.
>
>
> === modified file 'mysql-test/include/stop_slave.inc'
> --- a/mysql-test/include/stop_slave.inc 2009-01-09 14:12:31 +0000
> +++ b/mysql-test/include/stop_slave.inc 2010-07-26 13:31:58 +0000
> @@ -3,19 +3,89 @@
> +
> +if ($rpl_only_running_threads)
> +{
> + --let $_slave_sql_running= query_get_value(SHOW SLAVE STATUS, Slave_SQL_Running,
> 1)
> + --let $_slave_io_running= query_get_value(SHOW SLAVE STATUS, Slave_IO_Running, 1)
> + if ($rpl_debug)
> + {
> + --echo Stop SQL: $_slave_sql_running; Stop IO: $_slave_io_running
> + }
> +
[snip]
> + --let $_slave_running_bits= `SELECT IF('$_slave_io_running' = 'Yes', 1, 0) +
> IF('$_slave_sql_running' = 'Yes', 2, 0)`
> + if ($_slave_running_bits)
> + {
> + --dec $_slave_running_bits
> + # $_slave_running_bits=1: io thread running
> + if (!$_slave_running_bits)
> + {
> + --source include/stop_slave_io.inc
> + }
> + --dec $_slave_running_bits
> + # $_slave_running_bits=2: sql thread running
> + if (!$_slave_running_bits)
> + {
> + --source include/stop_slave_sql.inc
> + }
> + --dec $_slave_running_bits
> + # $_slave_running_bits=2: both threads running
> + if (!$_slave_running_bits)
> + {
> + STOP SLAVE;
> + --source include/wait_for_slave_to_stop.inc
> + }
It can be simplified as
if (`SELECT '$_slave_io_running' = 'YES'`)
{
--source include/stop_slave_io.inc
}
if (`SELECT '$_slave_sql_running' = 'YES'`)
{
--source include/stop_slave_sql.inc
}
> + }
> +}
> +if (!$rpl_only_running_threads)
> +{
> + STOP SLAVE;
> + --source include/wait_for_slave_to_stop.inc
> +}
> +
> +
> +--let $rpl_include_filename= stop_slave.inc
> +--source include/rpl_end_include_file.inc
>
>
> === added file 'mysql-test/include/sync_io_with_master.inc'
> --- a/mysql-test/include/sync_io_with_master.inc 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/include/sync_io_with_master.inc 2010-07-26 13:31:58 +0000
I think we should combine sync_io_with_master.inc and
sync_slave_io_with_master.inc.
> === modified file 'mysql-test/include/sync_slave_io_with_master.inc'
> --- a/mysql-test/include/sync_slave_io_with_master.inc 2009-01-09 14:12:31 +0000
> +++ b/mysql-test/include/sync_slave_io_with_master.inc 2010-07-26 13:31:58 +0000
see above.
>
> === added file 'mysql-test/include/wait_for_query_to_fail.inc'
> --- a/mysql-test/include/wait_for_query_to_fail.inc 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/include/wait_for_query_to_fail.inc 2010-07-26 13:31:58 +0000
> @@ -0,0 +1,25 @@
> +#
> +# Run a query over and over until it succeeds ot timeout occurs
typo: s/succeeds/fails/ s/ot/or/
> +#
> +
> +
> +let $counter= 100;
> +
> +disable_abort_on_error;
> +disable_query_log;
> +disable_result_log;
> +eval $query;
> +while (!$mysql_errno)
> +{
> + eval $query;
> + sleep 0.1;
> + dec $counter;
> +
> + if (!$counter)
> + {
> + --die "Waited too long for query to fail";
> + }
> +}
> +enable_abort_on_error;
> +enable_query_log;
> +enable_result_log;
>
> === modified file 'mysql-test/include/wait_for_slave_io_error.inc'
> --- a/mysql-test/include/wait_for_slave_io_error.inc 2010-05-28 02:57:45 +0000
> +++ b/mysql-test/include/wait_for_slave_io_error.inc 2010-07-26 13:31:58 +0000
> @@ -4,32 +4,37 @@
> # error, or until a timeout is reached. Also waits until the IO
> # thread has completely stopped.
It don't stop IO thread, please remove the comments.
--
Your Sincerely,
Libing Song
==================================
MySQL Replication Team
Software Engineer
Email : Li-Bing.Song@stripped
Skype : libing.song
MSN : slb_database@stripped
Phone : +86 010-6505-4020 ext. 319
Mobile: +86 138-1144-2038
==================================
