List:Commits« Previous MessageNext Message »
From:Andrei Elkin Date:November 25 2008 8:16am
Subject:Re: bzr commit into mysql-6.0-rpl branch (skozlov:2738) Bug#39903
View as plain text  
Serge, hello.

The patch is good, still I'd love to improve it.


> #At file:///home/ksm/sun/repo/bug39903/mysql-6.0-rpl/ based on
> revid:aelkin@stripped
>
>  2738 Serge Kozlov	2008-11-25

>       Bug#39903

You omitted the problem description section, could you please write
down what it is about?

The following lines I interprete as the proposed solution.

>       1. include/wait_for_status_var.inc: new primitive for waiting value of 
>       a variable from SHOW STATUS.
>       2. include/maria_empty_log.inc: added waiting of restoring all connections 
>       to restarted server.
> added:
>   mysql-test/include/wait_for_status_var.inc
> modified:
>   mysql-test/include/maria_empty_logs.inc
>
> === modified file 'mysql-test/include/maria_empty_logs.inc'
> --- a/mysql-test/include/maria_empty_logs.inc	2008-07-24 18:26:12 +0000
> +++ b/mysql-test/include/maria_empty_logs.inc	2008-11-24 22:41:07 +0000
> @@ -7,6 +7,9 @@
>  connection default;
>  let $default_db=`select database()`;
>  let $MYSQLD_DATADIR= `SELECT @@datadir`;


> +# it will used at end of test for wait_for_status_var.inc primitive
> +let $status_var= Threads_connected;
> +let $status_var_value= query_get_value(SHOW STATUS LIKE 'Threads_connected', Value,
> 1);

The new `let' variables are used once at the referring
`wait_for_status_var.inc' invocation. Hence it makes more sense to declare
them right before the invocation which ease reading the test.
I think you need to move the declarations to ---- >

>  
>  connection admin;
>  -- echo * shut down mysqld, removed logs, restarted it
> @@ -74,6 +77,8 @@ EOF

____
    V 

this point. The comment `# it will used at end' would not needed.

>  --source include/wait_until_connected_again.inc
>  
>  connection default;
> +# Make sure that all connections are restored
> +--source include/wait_for_status_var.inc
>  # Restore current database as the effect of "use" was lost after restart
>  --disable_query_log
>  eval use $default_db;
>

The new macro looks okay.
Still, I think we are better need to use SELECT instead of SHOW:

select VARIABLE_VALUE from information_schema session_status where
        VARIABLE_NAME like %pattern% ;

> === added file 'mysql-test/include/wait_for_status_var.inc'
> --- a/mysql-test/include/wait_for_status_var.inc	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/include/wait_for_status_var.inc	2008-11-24 22:41:07 +0000
> @@ -0,0 +1,58 @@
> +# ==== Purpose ====
> +#
> +# Waits until a variable from SHOW STATUS has returned a specified 
> +# value, or until a timeout is reached.
> +#
> +# ==== Usage ====
> +#
> +# let $status_var= Threads_connected;
> +# let $status_var_value= 1;
> +# --source include/wait_for_status_var.inc
> +#
> +# Parameters:
> +#
> +# $status_var, $status_var_value
> +#   This macro will wait until the variable of SHOW STATUS 
> +#   named $status_var gets the value $status_var_value.  See
> +#   the example above.
> +#
> +# $status_var_comparsion
> +#   By default, this file waits until $status_var becomes equal to
> +#   $status_var_value.  If you want to wait until $status_var
> +#   becomes *unequal* to $status_var_value, set this parameter to the
> +#   string '!=', like this:
> +#     let $status_var_comparsion= !=;
> +#
> +# $status_timeout
> +#   The default timeout is 1 minute. You can change the timeout by
> +#   setting $status_timeout. The unit is tenths of seconds.
> +#
> +
> +let $_status_timeout_counter= $status_timeout;
> +if (!$_status_timeout_counter)
> +{
> +  let $_status_timeout_counter= 600;
> +}
> +
> +let $_status_var_comparsion= $status_var_comparsion;
> +if (`SELECT '$_status_var_comparsion' = ''`)
> +{
> +  let $_status_var_comparsion= =;
> +}
> +
> +let $_show_status_value= query_get_value("SHOW STATUS LIKE '$status_var'", Value,
> 1);
> +while (`SELECT NOT('$_show_status_value' $_status_var_comparsion
> '$status_var_value')`)
> +{
> +  if (!$_status_timeout_counter)
> +  {
> +    --echo **** ERROR: failed while waiting for  $status_var $_status_var_comparison
> $status_var_value ****
> +    --echo Note: the following output may have changed since the failure was
> detected
> +    --echo **** Showing STATUS, PROCESSLIST ****
> +    eval SHOW STATUS LIKE '$status_var';
> +    SHOW PROCESSLIST;
> +    exit;
> +  }
> +  dec $_status_timeout_counter;
> +  sleep 0.1;
> +  let $_show_status_value= query_get_value("SHOW STATUS LIKE '$status_var'", Value,
> 1);
> +}
>

I hope you find the suggestions as improvements.

regards,

Andrei
Thread
bzr commit into mysql-6.0-rpl branch (skozlov:2738) Bug#39903Serge Kozlov24 Nov
  • Re: bzr commit into mysql-6.0-rpl branch (skozlov:2738) Bug#39903Andrei Elkin25 Nov
    • Re: bzr commit into mysql-6.0-rpl branch (skozlov:2738) Bug#39903Serge Kozlov25 Nov
      • Re: bzr commit into mysql-6.0-rpl branch (skozlov:2738) Bug#39903Andrei Elkin25 Nov
        • Re: bzr commit into mysql-6.0-rpl branch (skozlov:2738) Bug#39903Serge Kozlov25 Nov
  • Re: bzr commit into mysql-6.0-rpl branch (skozlov:2738) Bug#39903Guilhem Bichot12 Feb