Hi Sven,
Overall the patch looks good. I just have one request
for you below (see assert.inc question).
On 03/15/2011 11:47 AM, Sven Sandberg wrote:
> #At file:///home/sven/bzr/debug-max/5.5/ based on
> revid:magne.mahre@stripped
>
> 3369 Sven Sandberg 2011-03-15
> BUG#11872422: rpl_slave_load_remove_tmpfile fails sporadically in pb2
> Problem: the test failed because errors were found in the error log.
> The test case contains suppressions for an old version of the error message,
> but the format of the error message has changed without updating the
> suppression.
I think that it may have been that the work done in
http://lists.mysql.com/commits/131914 has not fixed
properly the suppressions for that test case.
> Fix: Update the suppression. Also small fixes to improve the test.
> @ mysql-test/suite/rpl/r/rpl_slave_load_remove_tmpfile.result
> update result file
> @ mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile-slave.opt
> Use variables instead of .opt files to avoid server restarts.
> @ mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile.test
> 1. To fix the bug, we update the regular expression in mtr.add_suppression
> so that it matches the real error text.
OK
> 2. Use wait_for_slave_sql_error.inc when we wait for an error.
> This makes the test easier to understand and will produce better
> debug info if the test fails.
OK (mysql-trunk seems to have this already).
> 3. Use server variables instead of command line options to set
> the @@GLOBAL.DEBUG variable. This avoids server restarts when
> running the test suite.
OK
> 4. Clarify the comment at the top of the file and add bug reference.
Good.
> removed:
> mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile-slave.opt
> modified:
> mysql-test/include/assert.inc
Can you please help me understanding why the changes in assert.inc are
needed?
Thanks,
Luís
> mysql-test/suite/rpl/r/rpl_slave_load_remove_tmpfile.result
> mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile.test
> === modified file 'mysql-test/include/assert.inc'
> --- a/mysql-test/include/assert.inc 2010-12-19 17:07:28 +0000
> +++ b/mysql-test/include/assert.inc 2011-03-15 11:47:05 +0000
> @@ -5,9 +5,12 @@
> # The condition is parsed before executed. The following constructs
> # are supported:
> #
> -# [SQL_STATEMENT, COLUMN, ROW]
> -# The square bracket is replaced by the result from SQL_STATEMENT,
> -# in the given COLUMN and ROW.
> +# [SQL_STATEMENT]
> +# [SQL_STATEMENT][COLUMN][ROW]
> +# The SQL_STATEMENT is executed. In the first form, the result
> +# should consist in only one value. In the second form, a value is
> +# taken from the given COLUMN and ROW. The entire square bracket
> +# is then replaced by the value.
> #
> # Optionally, SQL_STATEMENT may have the form:
> # connection:SQL_STATEMENT
> @@ -100,26 +103,61 @@ while ($_assert_lbracket)
> # Interpolate escapes before using condition outside string context.
> --let $_assert_substmt_interp= `SELECT $_assert_substmt`
>
> - # Execute and get result from sub-statement
> + # Get row and column, if substmt has format '[STATEMENT][COL][ROW]'
> + --let $_assert_col=
> + --let $_assert_row= 0
> + if (`SELECT SUBSTRING($_assert_cond_interp, $_assert_rbracket + 1) REGEXP
> '^[[][^],]*[]][[][1-9][0-9]*[]]'`)
> + {
> + --let $_assert_col_end= `SELECT LOCATE(']', $_assert_cond_interp,
> $_assert_rbracket + 1)`
> + --let $_assert_row_end= `SELECT LOCATE(']', $_assert_cond_interp,
> $_assert_col_end + 1)`
> + --let $_assert_col= `SELECT SUBSTR($_assert_cond_interp, $_assert_rbracket + 2,
> $_assert_col_end - $_assert_rbracket - 2)`
> + --let $_assert_row= `SELECT SUBSTR($_assert_cond_interp, $_assert_col_end + 2,
> $_assert_row_end - $_assert_col_end - 2)`
> + --let $_assert_full_substmt= $_assert_full_substmt,
> '][$_assert_col][$_assert_row'
> + }
> +
> + # Compute debug text
> + if ($rpl_debug)
> + {
> + if ($_assert_row)
> + {
> + --let $_assert_debug_text= sub-statement=$_assert_substmt row=$_assert_row
> col=$_assert_col
> + }
> + if (!$_assert_row)
> + {
> + --let $_assert_debug_text= sub-statement=$_assert_substmt (should result in
> one row, one column)
> + }
> + }
> + # Switch connection
> if ($_assert_connection)
> {
> if ($rpl_debug)
> {
> - --echo # debug: connection='$_assert_connection'
> sub-statement=$_assert_substmt
> + --echo # debug: connection='$_assert_connection' $_assert_debug_text
> }
> --let $rpl_connection_name= $_assert_connection
> --source include/rpl_connection.inc
> - --let $_assert_substmt_result= query_get_value($_assert_substmt_interp)
> - --let $rpl_connection_name= $_assert_old_connection
> - --source include/rpl_connection.inc
> }
> if (!$_assert_connection)
> {
> if ($rpl_debug)
> {
> - --echo # debug: old connection, sub-statement=$_assert_substmt
> + --echo # debug: keep old connection ($_assert_old_connection)
> $_assert_debug_text
> }
> - --let $_assert_substmt_result= query_get_value($_assert_substmt_interp)
> + }
> + # Execute and get result from sub-statement
> + if ($_assert_row)
> + {
> + --let $_assert_substmt_result= query_get_value('$_assert_substmt_interp',
> $_assert_col, $_assert_row)
> + }
> + if (!$_assert_row)
> + {
> + --let $_assert_substmt_result= `$_assert_substmt_interp`
> + }
> + # Switch back connection
> + if ($_assert_connection)
> + {
> + --let $rpl_connection_name= $_assert_old_connection
> + --source include/rpl_connection.inc
> }
> if ($rpl_debug)
> {
>
> === modified file 'mysql-test/suite/rpl/r/rpl_slave_load_remove_tmpfile.result'
> --- a/mysql-test/suite/rpl/r/rpl_slave_load_remove_tmpfile.result 2011-02-24 14:48:35
> +0000
> +++ b/mysql-test/suite/rpl/r/rpl_slave_load_remove_tmpfile.result 2011-03-15 11:47:05
> +0000
> @@ -1,12 +1,13 @@
> include/master-slave.inc
> [connection master]
> +SET @@GLOBAL.DEBUG = '+d,remove_slave_load_file_before_write';
> create table t1(a int not null auto_increment, b int, primary key(a))
> engine=innodb;
> start transaction;
> insert into t1(b) values (1);
> insert into t1(b) values (2);
> load data infile '../../std_data/rpl_loaddata.dat' into table t1;
> commit;
> -include/wait_for_slave_sql_to_stop.inc
> +include/wait_for_slave_sql_error.inc [errno=29, 13]
> drop table t1;
> include/sync_slave_io_with_master.inc
> include/stop_slave_io.inc
> @@ -14,5 +15,6 @@ RESET SLAVE;
> drop table t1;
> call mtr.add_suppression("Slave: Can't get stat of .*");
> call mtr.add_suppression("Slave SQL: Error .Can.t get stat of.* Error_code: 13");
> -call mtr.add_suppression("Slave: File.* not found.*");
> +call mtr.add_suppression("Slave SQL: Error .File.* not found.*");
> +SET @@GLOBAL.DEBUG = '';
> include/rpl_end.inc
>
> === removed file 'mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile-slave.opt'
> --- a/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile-slave.opt 2009-03-18
> 10:31:17 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile-slave.opt 1970-01-01
> 00:00:00 +0000
> @@ -1 +0,0 @@
> ---loose-debug=d,remove_slave_load_file_before_write
>
> === modified file 'mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile.test'
> --- a/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile.test 2011-02-24 14:48:35
> +0000
> +++ b/mysql-test/suite/rpl/t/rpl_slave_load_remove_tmpfile.test 2011-03-15 11:47:05
> +0000
> @@ -1,12 +1,21 @@
> -##########################################################################
> +# ==== Purpose ====
> +#
> # This test verifies if the slave fails gracefully when the temporary
> -# file used to load data is removed while it is about to be used it.
> +# file used to load data is removed while it is about to be used.
> # Similar errors are caught if the temporary directory is removed.
> #
> +# ==== Implementation ====
> +#
> # Steps:
> +# 0 - Set debug variable remove_slave_load_file_before_write. This
> +# causes the slave to remove the file.
> # 1 - Creates a table and populates it through "LOAD DATA INFILE".
> # 2 - Catches error.
> -##########################################################################
> +#
> +# ==== References ====
> +#
> +# BUG#42861: Assigning invalid directories to --slave-load-tmpdir crashes the slave
> +# BUG#11872422: rpl_slave_load_remove_tmpfile fails sporadically in pb2
>
> --source include/have_binlog_format_statement.inc
> --source include/have_innodb.inc
> @@ -18,6 +27,11 @@
> ##########################################################################
> # Loading data
> ##########################################################################
> +
> +connection slave;
> +--let $old_debug= `SELECT @@GLOBAL.DEBUG`
> +SET @@GLOBAL.DEBUG = '+d,remove_slave_load_file_before_write';
> +
> connection master;
>
> create table t1(a int not null auto_increment, b int, primary key(a))
> engine=innodb;
> @@ -32,19 +46,14 @@ commit;
> # Catch Error
> ##########################################################################
> connection slave;
> -source include/wait_for_slave_sql_to_stop.inc;
>
> ---let $error= query_get_value(SHOW SLAVE STATUS, Last_Errno, 1)
> # windows and linux different error numbers here:
> # Windows:
> # - Last_Errno 29 (File not found)
> # Unix like OS:
> # - Last_Errno 13 (Can't stat file)
> ---let $assertion= `SELECT $error=29 OR $error=13`
> -if (!$assertion)
> -{
> - --echo UNEXPECTED ERROR NUMBER: $error
> -}
> +--let $slave_sql_errno= 29, 13
> +--source include/wait_for_slave_sql_error.inc
This fix seems to be already in place for mysql-trunk.
> ##########################################################################
> # Clean up
> @@ -61,7 +70,10 @@ drop table t1;
>
> call mtr.add_suppression("Slave: Can't get stat of .*");
> call mtr.add_suppression("Slave SQL: Error .Can.t get stat of.* Error_code: 13");
> -call mtr.add_suppression("Slave: File.* not found.*");
> +call mtr.add_suppression("Slave SQL: Error .File.* not found.*");
> --let $rpl_only_running_threads= 1
> +
> +eval SET @@GLOBAL.DEBUG = '$old_debug';
> +
> --source include/rpl_end.inc
>
>
>
>
>
>
