| Bug #20782 | mysqladmin, ping command, small logic mistake | ||
|---|---|---|---|
| Submitted: | 29 Jun 2006 17:47 | Modified: | 30 Jul 2006 13:10 |
| Reporter: | Kai Voigt | Email Updates: | |
| Status: | No Feedback | Impact on me: | |
| Category: | MySQL Server | Severity: | S3 (Non-critical) |
| Version: | 5.1.7-beta-log | OS: | MacOS (MacOSX 10.4) |
| Assigned to: | CPU Architecture: | Any | |
[29 Jun 2006 18:01]
Kai Voigt
Cut'n'paste mistake on my side. Here's the fix for mysqladmin.cc
*** mysqladmin.cc.orig Thu May 4 14:14:43 2006
--- mysqladmin.cc Thu May 4 14:15:36 2006
***************
*** 952,963 ****
mysql->reconnect=1;
if (!mysql_ping(mysql))
puts("connection was down, but mysqld is now alive");
! }
! else
! {
! my_printf_error(0,"mysqld doesn't answer to ping, error: '%s'",
! MYF(ME_BELL),mysql_error(mysql));
! return -1;
}
}
mysql->reconnect=1; /* Automatic reconnect is default */
--- 952,963 ----
mysql->reconnect=1;
if (!mysql_ping(mysql))
puts("connection was down, but mysqld is now alive");
! else
! {
! my_printf_error(0,"mysqld doesn't answer to ping, error: '%s'",
! MYF(ME_BELL),mysql_error(mysql));
! return -1;
! }
}
}
mysql->reconnect=1; /* Automatic reconnect is default */
[30 Jun 2006 13:10]
Valeriy Kravchuk
Thank you for a problem report. For patches, please, use the latest -BK source (or, at least, the latest version released, 5.1.11).
I do not agree with you point, though:
> I believe this is wrong, because this way the "doesn't answer" branch will never
> be reached. It should go like this:
In this code:
if (!mysql_ping(mysql))
{
if (option_silent < 2)
puts("mysqld is alive");
}
else
{
if (mysql_errno(mysql) == CR_SERVER_GONE_ERROR)
{
mysql->reconnect=1;
if (!mysql_ping(mysql))
puts("connection was down, but mysqld is now alive");
}
else
{
my_printf_error(0,"mysqld doesn't answer to ping, error: '%s'",
MYF(ME_BELL),mysql_error(mysql));
return -1;
}
}
mysql->reconnect=1; /* Automatic reconnect is default */
if mysql_ping returns CR_SERVER_GONE_ERROR, we ping it for a second time, and in case of ANY failure prints nothing. In case of any other error for the first mysql_ping, we print it. So, why do you think "mysqld doesn't answer to ping, error" wiil never be reached? There are two other possible errors, according to http://dev.mysql.com/doc/refman/5.1/en/mysql-ping.html.
With your change in place you'll never be informed about any of two other error codes (like CR_COMMANDS_OUT_OF_SYNC) for the initail connection! And this is important. So, I am not sure your patch should be applied.
[30 Jul 2006 23:00]
Bugs System
No feedback was provided for this bug for over a month, so it is being suspended automatically. If you are able to provide the information that was originally requested, please do so and change the status of the bug back to "Open".
Description: When looking at the logic of how mysqladmin does the ping subcommand, it goes like this. mysql->reconnect=0; /* We want to know of reconnects */ if (!mysql_ping(mysql)) { if (option_silent < 2) puts("mysqld is alive"); } else { if (mysql_errno(mysql) == CR_SERVER_GONE_ERROR) { mysql->reconnect=1; if (!mysql_ping(mysql)) puts("connection was down, but mysqld is now alive"); } else { my_printf_error(0,"mysqld doesn't answer to ping, error: '%s'", MYF(ME_BELL),mysql_error(mysql)); return -1; } } mysql->reconnect=1; /* Automatic reconnect is default */ I believe this is wrong, because this way the "doesn't answer" branch will never be reached. It should go like this: mysql->reconnect=0; /* We want to know of reconnects */ if (!mysql_ping(mysql)) { if (option_silent < 2) puts("mysqld is alive"); } else { if (mysql_errno(mysql) == CR_SERVER_GONE_ERROR) { mysql->reconnect=1; if (!mysql_ping(mysql)) puts("connection was down, but mysqld is now alive"); else { my_printf_error(0,"mysqld doesn't answer to ping, error: '%s'", MYF(ME_BELL),mysql_error(mysql)); return -1; } } } mysql->reconnect=1; /* Automatic reconnect is default */ How to repeat: It's actually not repeatable, because mysqladmin will not maintain a connection during subsequent invocations. If you manage to shutdown the server between the 2. and 3. if test, it should be visible. It's visible if you implement a PING command in mysql command line client (patch appended) and shutdown the server, restart it and try the ping command inside the mysql command line client. Suggested fix: Here's two patches for mysqladmin.cc to fix the problem and for mysql.cc to implement a ping command. mysql->reconnect=0; /* We want to know of reconnects */ if (!mysql_ping(mysql)) { if (option_silent < 2) puts("mysqld is alive"); } else { if (mysql_errno(mysql) == CR_SERVER_GONE_ERROR) { mysql->reconnect=1; if (!mysql_ping(mysql)) puts("connection was down, but mysqld is now alive"); else { my_printf_error(0,"mysqld doesn't answer to ping, error: '%s'", MYF(ME_BELL),mysql_error(mysql)); return -1; } } } mysql->reconnect=1; /* Automatic reconnect is default */ *** mysql.cc.orig Thu May 4 13:50:12 2006 --- mysql.cc Thu May 4 14:17:31 2006 *************** *** 190,195 **** --- 190,196 ---- static int com_quit(String *str,char*), com_go(String *str,char*), com_ego(String *str,char*), com_print(String *str,char*), + com_ping(String *str,char*), com_help(String *str,char*), com_clear(String *str,char*), com_connect(String *str,char*), com_status(String *str,char*), com_use(String *str,char*), com_source(String *str, char*), *************** *** 234,239 **** --- 235,241 ---- static COMMANDS commands[] = { { "?", '?', com_help, 1, "Synonym for `help'." }, { "clear", 'c', com_clear, 0, "Clear command."}, + { "ping", 'S', com_ping, 0, "Ping server."}, { "connect",'r', com_connect,1, "Reconnect to the server. Optional arguments are db and host." }, { "delimiter", 'd', com_delimiter, 1, *************** *** 1865,1870 **** --- 1867,1896 ---- } static int + com_ping(String *buffer __attribute__((unused)), + char *line __attribute__((unused))) + { + /* We want to know of redirects */ + mysql.reconnect=0; + if (!mysql_ping(&mysql)) + put_info("mysqld is alive", INFO_INFO); + else + { + /* Trying to reconnect */ + if (mysql_errno(&mysql) == CR_SERVER_GONE_ERROR) + { + mysql.reconnect=1; + if (!mysql_ping(&mysql)) + put_info("connection was down, but mysqld is now alive", INFO_INFO); + else + put_info("mysqld doesn't answer to ping", INFO_INFO); + } + } + /* Automatic reconnect is default */ + mysql.reconnect=1; + } + + static int com_help(String *buffer __attribute__((unused)), char *line __attribute__((unused))) {