List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:September 24 2010 11:55am
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3273)
Bug#56416
View as plain text  
Hi Libing and Daogang,

I don't see any problem with Daogang's current patch.


On 09/21/2010 11:26 AM, Libing Song wrote:
> Hi Daogang,
>
> Nice work!
>
> Please find my review comments below.
>
> STATUS
> ------
>
>    Not Approved.
>
> REQUIRED CHANGES
> ----------------
> RC1. It will result another problem.
> Because it just print the error if it is its first happening.
> So retry times will always be 1 even through it retries so many times.

Can you explain this?
After Daongang's patch there are the following entries as expected because
retry_count is 3.

100922 13:47:10 [ERROR] Slave I/O: error connecting to master 'root@dummy:13020' -
retry-time: 1  retries: 1, Error_code: 2005
100922 13:47:12 [ERROR] Slave I/O: error connecting to master 'root@dummy:13020' -
retry-time: 1  retries: 2, Error_code: 2005
100922 13:47:13 [ERROR] Slave I/O: error connecting to master 'root@dummy:13020' -
retry-time: 1  retries: 3, Error_code: 2005

Cheers.

>
> I think there are two solutions for this bug:
> 1. Print the error if it is its last happening.
>     and print err_count.
> 2. According to the bug report, It just print a wrong number at the
> first connect. That is because the code that:
>    if ((int)mysql_errno(mysql) != last_errno).
> when on its first connecting, last_errno is probably 0.
> But fatal error never retry.
>
> I prefer the first one.
>
>
> REQUESTS
> --------
>
> SUGGESTIONS
> -----------
>
> DETAILS
> -------
> n/a
> On Mon, 2010-09-20 at 06:36 +0000, Dao-Gang.Qu@stripped wrote:
>> #At file:///home/daogangqu/mysql/bzrwork1/bugtest/mysql-next-mr-bugfixing/ based
> on revid:wlad@stripped
>>
>>   3273 Dao-Gang.Qu@stripped	2010-09-20
>>        Bug #56416    errors in first connection for IO thread are not retried
>>
>>        SHOW SLAVE STATUS can not display the number of real retries
>>        for each connection attempt of I/O thread.
>>
>>        To fix the problem to make SHOW SLAVE STATUS to display the
>>        number of retries which would be real retry counts instead of
>>        mi->retry_count(3600*24 by default) for each connection
>>        attempt of I/O thread.
>>       @ mysql-test/suite/rpl/r/rpl_change_master_dbug.result
>>          Updated for the patch of BUG #56416
>>       @ mysql-test/suite/rpl/t/rpl_change_master_dbug.test
>>          Updated test case to also verifies that the 'Last_IO_Error'
>>          entry of SHOW SLAVE STATUS will display the number of real
>>          retries.
>>       @ sql/rpl_slave.cc
>>          Added code to make SHOW SLAVE STATUS to display the number of
>>          real retries for each connection attempt of I/O thread.
>>
>>      modified:
>>        mysql-test/suite/rpl/r/rpl_change_master_dbug.result
>>        mysql-test/suite/rpl/t/rpl_change_master_dbug.test
>>        sql/rpl_slave.cc
>> === modified file 'mysql-test/suite/rpl/r/rpl_change_master_dbug.result'
>> --- a/mysql-test/suite/rpl/r/rpl_change_master_dbug.result	2010-07-29 01:51:38
> +0000
>> +++ b/mysql-test/suite/rpl/r/rpl_change_master_dbug.result	2010-09-20 06:36:19
> +0000
>> @@ -5,11 +5,8 @@ reset slave;
>>   drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>   start slave;
>>   include/stop_slave.inc
>> -SET @old_debug= @@global.debug;
>> -SET GLOBAL debug="+d,connect_to_master_always_report_error";
>>   CHANGE MASTER TO master_retry_count=3, master_host='dummy',
> master_connect_retry=1;
>>   START SLAVE io_thread;
>> -SET @@global.debug= @old_debug;
>>   stop slave;
>>   drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>   reset master;
>>
>> === modified file 'mysql-test/suite/rpl/t/rpl_change_master_dbug.test'
>> --- a/mysql-test/suite/rpl/t/rpl_change_master_dbug.test	2010-07-29 01:51:38
> +0000
>> +++ b/mysql-test/suite/rpl/t/rpl_change_master_dbug.test	2010-09-20 06:36:19
> +0000
>> @@ -6,7 +6,9 @@
>>   #
>>   # BUG#44209: MASTER_CONNECT_RETRY and --master-retry-count disconnected from
> each other
>>   #
>> -# Second part of the tests. Requires debug build.
>> +# BUG#56416
>> +# The test also verifies that the 'Last_IO_Error' entry of SHOW SLAVE STATUS
>> +# will display the number of real retries
>>   #
>>
>>   ## Checks that the slave actually gives up after retrying N times, where
>> @@ -14,8 +16,6 @@
>>
>>   -- connection slave
>>   -- source include/stop_slave.inc
>> -SET @old_debug= @@global.debug;
>> -SET GLOBAL debug="+d,connect_to_master_always_report_error";
>>   CHANGE MASTER TO master_retry_count=3, master_host='dummy',
> master_connect_retry=1;
>>   START SLAVE io_thread;
>>
>> @@ -32,7 +32,6 @@ if (!`SELECT "$error" LIKE "%retries: 3"
>>     -- echo Expected number of retries was: 3
>>     -- die
>>   }
>> -SET @@global.debug= @old_debug;
>>
>>   -- source include/master-slave-reset.inc
>>   -- source include/master-slave-end.inc
>>
>> === modified file 'sql/rpl_slave.cc'
>> --- a/sql/rpl_slave.cc	2010-09-01 02:51:08 +0000
>> +++ b/sql/rpl_slave.cc	2010-09-20 06:36:19 +0000
>> @@ -4503,33 +4503,19 @@ static int connect_to_master(THD* thd, M
>>             mysql_real_connect(mysql, mi->host, mi->user, mi->password,
> 0,
>>                                mi->port, 0, client_flag) == 0))
>>     {
>> -    /* Don't repeat last error */
>> -    if ((int)mysql_errno(mysql) != last_errno ||
>> -        DBUG_EVALUATE_IF("connect_to_master_always_report_error",
>> -                         TRUE, FALSE))
>> -    {
>> -      /*
>> -        TODO: would be great that when issuing SHOW SLAVE STATUS
>> -              the number of retries would actually show err_count
>> -              instead of mi->retry_count.
>> -
>> -              We can achieve that if we remove the 'if' above and
>> -              replace mi->retry_count with err_count. However, we
>> -              would be adding an entry in the error log for each
>> -              connection attempt (ie, for each retry).
>> -       */
>> -      last_errno=mysql_errno(mysql);
>> -      suppress_warnings= 0;
>> -      mi->report(ERROR_LEVEL, last_errno,
>> -                 "error %s to master '%s@%s:%d'"
>> -                 " - retry-time: %d  retries: %lu",
>> -                 (reconnect ? "reconnecting" : "connecting"),
>> -                 mi->user, mi->host, mi->port,
>> -                 mi->connect_retry,
>> -                 DBUG_EVALUATE_IF("connect_to_master_always_report_error",
>> -                                  err_count+1,
>> -                                  mi->retry_count));
>> -    }
>> +    /*
>> +       SHOW SLAVE STATUS will display the number of retries which
>> +       would be real retry counts instead of mi->retry_count for
>> +       each connection attempt by 'Last_IO_Error' entry.
>> +    */
>> +    last_errno=mysql_errno(mysql);
>> +    suppress_warnings= 0;
>> +    mi->report(ERROR_LEVEL, last_errno,
>> +               "error %s to master '%s@%s:%d'"
>> +               " - retry-time: %d  retries: %lu",
>> +               (reconnect ? "reconnecting" : "connecting"),
>> +               mi->user, mi->host, mi->port,
>> +               mi->connect_retry, err_count+1);
>>       /*
>>         By default we try forever. The reason is that failure will trigger
>>         master election, so if the user did not set mi->retry_count we
>>
>
>

Thread
bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3273) Bug#56416Dao-Gang.Qu20 Sep
  • Re: bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3273)Bug#56416Libing Song21 Sep
    • Re: bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3273)Bug#56416Alfranio Correia24 Sep
      • Re: bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3273)Bug#56416Daogang Qu25 Sep
      • Re: bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3273)Bug#56416Libing Song25 Sep
        • Re: bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3273)Bug#56416Libing Song25 Sep
          • Re: bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3273)Bug#56416Daogang Qu25 Sep