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

Andrei Elkin пишет:
> 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?

Added to the patch.

> 
> 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= 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 ---- >

No, this separation added correctly. Short description are:
1. Remember value of variable 'Threads_connected' ( $status_var/ 
$status_var_value ).
2. Stop server ('Threads_connected' is changed too)
3. Start server
4. Wait until variable 'Threads_connected' will have same value as in 
step 1 (include/wait_for_status_var.inc).

It is no make sense to move step 1 before step 4 and after step 3

> 
>>  
>>  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:
SHOW STATUS has the advantage: I get global and session variables in one 
query. So seems it would be better than SELECT;

> 
> 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


-- 
Serge Kozlov, QA Developer
MySQL AB, Moscow, Russia, www.mysql.com
Office:

Are you MySQL certified?  www.mysql.com/certification
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