Alexey, hello.
The patch is approved here.
> Hi Andrei,
>
> On Thursday 22 November 2007 15:39:07, Andrei Elkin wrote:
>>Alexey, hello.
>>
>>> Below is the list of changes that have just been committed into a local
>>> 5.1 repository of kaa. When kaa does a push these changes will
>>> be propagated to the main repository and, within 24 hours after the
>>> push, to the public repository.
>>> For information on how to access the public repository
>>> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>>>
>>> ChangeSet@stripped, 2007-11-14 14:42:40+03:00, kaa@polly.(none) +1 -0
>>> Fix for bug #29976: Excessive Slave I/O errors in replication tests
>>>
>>> Problem:
>>>
>>> The "Slave I/O thread couldn't register on master" error sporadically
>>> occurred in replication tests because the slave I/O thread got
>>> killed by STOP SLAVE before or while registering on master.
>>>
>>>
>>> Fixed by checking the state of the I/O thread, and issueing
>>> the error only if it was not explicitely killed by a user.
>>
>>I think that's a sound idea. If we know why the IO thread can not
>>connect we should not issue an error. The error is for a case we
>>don't know why.
>>
>>What I'd like for you to do is to extend it to cover related issues.
>>Before the IO thread gains ON status it makes several client-api
>>calls. One your patch is fixing and others are done just before
>>register_slave_on_master() in get_master_version_and_clock().
>>So let's not panic spewing error messages whenever a killed io fails
>>to query the master.
>>
>
> Done in the new patch. In fact, there is only one place in
> get_master_version_and_clock() where killing the I/O slave could lead to a
> warning. All other ones simply ignore any errors (of course, that would be
> just caught later in register_slave_on_master()).
Yes, thanks for discussing that!
>
>>> @@ -2170,11 +2171,15 @@ connected:
>>> thd->proc_info = "Registering slave on master";
>>> if (register_slave_on_master(mysql, mi, &suppress_warnings))
>>> {
>>> - sql_print_error("Slave I/O thread couldn't register on master");
>>> - if (check_io_slave_killed(thd, mi, "Slave I/O thread killed while \
>>> -registering slave on master") ||
>>> - try_to_reconnect(thd, mysql, mi, &retry_count,
>>> suppress_warnings, -
>>> reconnect_messages[SLAVE_RECON_ACT_REG])) + if
>>> (!check_io_slave_killed(thd, mi, "Slave I/O thread killed " +
>>> "while registering slave on master")) + {
>>>
>>> + sql_print_error("Slave I/O thread couldn't register on
>>> master");
>>
>>I think we are better to remove this like as there is already
>>mi->report(ERROR_LEVEL,...) in register_slave_on_master().
>>
>
> Not when mysql_errno() == ER_NET_READ_INTERRUPTED. In such case, we don't
> print any error about failing COM_REGISTER_SLAVE, but do print "Slave I/O
> thread couldn't register on master" (and then try to reconnect).
Right.
>
> That was the old behavior, I just did not see any point in changing it.
>
Agree now.
regards,
Andrei
