Re: bzr commit into mysql-5.1 branch (sven:2709) Bug#37718

[Sven Sandberg]

He Zhenxing wrote:
> Hi Sven,
> 
> Great work! Thanks for the improvement of the test code!
> 
> Patch approved, only one minor comment.
> 
> Sven Sandberg wrote:
>> #At file:///home/sven/bzr/b37718-rpl_stm_mystery22/5.1-rpl/ based on
> revid:zhenxing.he@stripped
>>
>>  2709 Sven Sandberg	2009-01-07
>>       BUG#37718: rpl.rpl_stm_mystery22 fails sporadically on pushbuild
>>       Problem 1: The test waits for an error in the slave sql thread,
>>       then resolves the error and issues 'start slave'. However, there
>>       is a gap between when the error is reported and the slave sql
>>       thread stops. If this gap was long, the slave would still be
>>       running when 'start slave' happened, so 'start slave' would fail
>>       and cause a test failure.
>>       Fix 1: Made wait_for_slave_sql_error wait for the slave to stop
>>       instead of wait for error in the IO thread. After stopping, the
>>       error code is verified. If the error code is wrong, debug info
>>       is printed. To print debug info, the debug printing code in
>>       wait_for_slave_param.inc was moved out to a new file,
>>       show_rpl_debug_info.inc.
>>       Problem 2: rpl_stm_mystery22 is a horrible name, the comments in
>>       the file didn't explain anything useful, the test was generally
>>       hard to follow, and the test was essentially duplicated between
>>       rpl_stm_mystery22 and rpl_row_mystery22.
>>       Fix 2: The test is about conflicts in the slave SQL thread,
>>       hence I renamed the tests to rpl_{stm,row}_conflicts. Refactored
>>       the test so that the work is done in
>>       extra/rpl_tests/rpl_conflicts.inc, and
>>       rpl.rpl_{row,stm}_conflicts merely sets some variables and then
>>       sourced extra/rpl_tests/rpl_conflicts.inc.
>>       The tests have been rewritten and comments added.
>>       Problem 3: When calling wait_for_slave_sql_error.inc, you always
>>       want to verify that the sql thread stops because of the expected
>>       error and not because of some other error. Currently,
>>       wait_for_slave_sql_error.inc allows the caller to omit the error
>>       code, in which case all error codes are accepted.
>>       Fix 3: Made wait_for_slave_sql_error.inc fail if no error code
>>       is given. Updated rpl_filter_tables_not_exist accordingly.
>>       Problem 4: rpl_filter_tables_not_exist had a typo, the dollar
>>       sign was missing in a 'let' statement.
>>       Fix 4: Added dollar sign.
>>       Problem 5: When replicating from other servers than the one named
>>       'master', the wait_for_slave_* macros were unable to print debug
>>       info on the master.
>>       Fix 5: Remove parameter $slave_keep_connection by
>>       $master_connection.
> 
> [snip]
> 
>> === added file 'mysql-test/include/show_rpl_debug_info.inc'
>> --- a/mysql-test/include/show_rpl_debug_info.inc	1970-01-01 00:00:00 +0000
>> +++ b/mysql-test/include/show_rpl_debug_info.inc	2009-01-07 08:07:07 +0000
>> @@ -0,0 +1,75 @@
>> +# ==== Purpose ====
>> +#
>> +# Print status information for replication, typically used to debug
>> +# test failures.
>> +#
>> +# First, the following is printed on slave:
>> +#
>> +#   SHOW SLAVE STATUS
>> +#   SHOW PROCESSLIST
>> +#   SHOW BINLOG EVENTS IN <binlog_name>
>> +#
>> +# Where <binlog_name> is the currently active binlog.
>> +#
>> +# Then, the following is printed on master:
>> +#
>> +#   SHOW MASTER STATUS
>> +#   SHOW PROCESSLIST
>> +#   SHOW BINLOG EVENTS IN <sql_binlog_name>
>> +#   SHOW BINLOG EVENTS IN <io_binlog_name>
>> +#
>> +# Where <sql_binlog_name> is the binlog name that the slave sql thread
>> +# is currently reading from and <io_binlog_name> is the binlog that
>> +# the slave IO thread is currently reading from.
>> +#
>> +# ==== Usage ====
>> +#
>> +# [let $master_connection= <connection>;]
>> +# source include/show_rpl_debug_info.inc;
>> +#
>> +# If $master_connection is set, debug info will be retrieved from the
>> +# connection named $master_connection.  Otherwise, it will be
>> +# retrieved from the 'master' connection if the current connection is
>> +# 'slave'.
>> +
>> +let $_con= $CURRENT_CONNECTION;
>> +--echo [on $_con]
>> +--echo **** SHOW SLAVE STATUS on $_con ****
>> +query_vertical SHOW SLAVE STATUS;
>> +--echo **** SHOW PROCESSLIST on $_con ****
>> +SHOW PROCESSLIST;
>> +--echo **** SHOW BINLOG EVENTS on $_con ****
>> +let $binlog_name= query_get_value("SHOW MASTER STATUS", File, 1);
>> +eval SHOW BINLOG EVENTS IN '$binlog_name';
>> +
>> +let $_master_con= $master_connection;
>> +if (!$_master_con) {
>> +  if (`SELECT '$_master_con' = 'slave') {
>> +    let $_master_con= master;
>> +  }
>> +  if (!$_master_con) {
>> +    --echo Unable to determine master connection. No debug info printed for
> master.
>> +    --echo Please fix the test case by setting $master_connection before
> sourcing
>> +    --echo show_rpl_debug_info.inc.
> 
> I think an 'exit' should be added here to quit the test case.

The purpose of show_rpl_debug_info.inc is just to print debug info. In 
the two use cases we have currently, there is an 'exit' right after 
'source show_rpl_debug_info.inc', but I think it is possible to think of 
situations when 'exit' is not desired. E.g., a test case may want to 
print other debug info after 'source show_rpl_debug_info.inc'. So I 
would prefer to not add exit. OK?

/Sven