Zhen Xing, Dao-Gang, good morning!
Let me add couple of remarks to your conversation to avoid requiring
contradictory changes.
> Dao-Gang.Qu@stripped wrote:
>> #At file:///home/daogangq/mysql/bzrwork/bug45214/5.1-bt/ based on
> revid:davi.arnaut@stripped
>>
>> 2941 Dao-Gang.Qu@stripped 2009-07-01
>> Bug #45214 get_master_version_and_clock does not report error when queries
> fail
>>
>> The "get_master_version_and_clock(...)" function in sql/slave.cc ignores
>> error and passes directly when queries fail, or queries succeed
>> but the result retrieved is empty.
>>
>> The "get_master_version_and_clock(...)" function should try to reconnect
> master
>> if queries fail because of transient network problems, and fail otherwise.
>> The I/O thread should print a warning information if the queries succeed
>> but the result retrieved is empty.
...
>> +bool is_network_issue(MYSQL* mysql, Master_info* mi)
>> +{
>> + int errno;
>> + errno= mysql_errno(mysql);
>> + if (errno == CR_CONNECTION_ERROR ||
>> + errno == CR_CONN_HOST_ERROR ||
>> + errno == CR_SERVER_GONE_ERROR ||
>> + errno == CR_SERVER_LOST ||
>> + errno == ER_CON_COUNT_ERROR ||
>> + errno == ER_SERVER_SHUTDOWN)
>> + {
>> + mi->report(WARNING_LEVEL, errno, mysql_error(mysql));
>> + sql_print_error("network issue daogang test: %s, %d", mysql_error(mysql),
> errno);
>
> Hmm, I think you forget to remove this line.
>
That's right. Even sql_print_warning() is unnecessary because of report() calls one.
I suggest always call rli,mi->report() if we need to report an error or a warning.
[The definition of the method is under revision due to BUG#36524, BUG#25597, you're
welcome to look at proposals in the latest patch]
...
>> @@ -967,26 +1011,64 @@ static int get_master_version_and_clock(
>> Note: we could have put a @@SERVER_ID in the previous SELECT
>> UNIX_TIMESTAMP() instead, but this would not have worked on 3.23 masters.
>> */
>> - if (!mysql_real_query(mysql,
>> - STRING_WITH_LEN("SHOW VARIABLES LIKE 'SERVER_ID'"))
> &&
>> - (master_res= mysql_store_result(mysql)))
>> - {
>> - if ((master_row= mysql_fetch_row(master_res)) &&
>> - (::server_id == strtoul(master_row[1], 0, 10)) &&
>> - !mi->rli.replicate_same_server_id)
>> - {
>> - errmsg= "The slave I/O thread stops because master and slave have equal \
>> + DBUG_SYNC_POINT("debug_lock.before_get_SERVER_ID", 10);
>> + query_re= mysql_real_query(mysql, STRING_WITH_LEN("SHOW VARIABLES LIKE
> 'SERVER_ID'"));
>> + if (query_re)
>> + {
>> + if (is_network_issue(mysql, mi))
>> + DBUG_RETURN(2);
>> + /* Fatal error */
>> + sql_print_error(mysql_error(mysql));
>
> Please use something like this: sql_print_error("Query \"SHOW VARIABLES
> LIKE 'SEVER_ID'\" failed with error: %s (%d)", mysql_error(mysql),
> mysql_errno(mysql));
>
> Please also change the following similar cases.
Guys, neither sql_print_error() is needed. As noted mi,rli->report() that will
be called for the error path takes care of error-logging calling
sql_print_error().
cheers,
Andrei
