List:Commits« Previous MessageNext Message »
From:Luís Soares Date:March 15 2011 12:11pm
Subject:Re: bzr commit into mysql-5.5 branch (sven.sandberg:3369) Bug#11872422
View as plain text  
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
>
>
>
>
>
>

Thread
bzr commit into mysql-5.5 branch (sven.sandberg:3369) Bug#11872422Sven Sandberg15 Mar
  • Re: bzr commit into mysql-5.5 branch (sven.sandberg:3369) Bug#11872422Luís Soares15 Mar
  • Re: bzr commit into mysql-5.5 branch (sven.sandberg:3369)Bug#11872422Bjorn Munch15 Mar