List:Commits« Previous MessageNext Message »
From:Libing Song Date:September 25 2010 5:36am
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (Dao-Gang.Qu:3273)
Bug#56416
View as plain text  
On Sat, 2010-09-25 at 11:51 +0800, Libing Song wrote:
> On Fri, 2010-09-24 at 12:55 +0100, Alfranio Correia wrote:
> > 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
> > 
> Unfortunately, the sequence of same errors shows only one times.
> So expected entry is :
> 100922 13:47:10 [ERROR] Slave I/O: error connecting to master 'root@dummy:13020' -
> retry-time: 1  retries: 1, Error_code: 2005
Sorry, I missed that '(int)mysql_errno(mysql) != last_errno' is removed.
So I agree with you, there is no problem with this patch.

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

-- 
Your Sincerely,
Libing Song
==================================
MySQL Replication Team
Software Engineer


Email : Li-Bing.Song@stripped
Skype : libing.song
MSN   : slb_database@stripped
Phone : +86 010-6505-4020 ext. 319
Mobile: +86 138-1144-2038
==================================

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