[Home]

Summary:ASTERISK-09158: [patch] Error condition checking when connection to mysql is lost
Reporter:Andrea Spadaccini (lupino3)Labels:
Date Opened:2007-03-31 04:13:27Date Closed:2007-06-21 10:11:57
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Addons/cdr_mysql
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) cdr_mysql_error.patch
Description:When the connection to mysql is lost, the return value of mysql_ping() is switched to see which error caused the loss of connection, but the checked error to compare to standard mysql error values should instead be the return value of the mysql_error() function.

****** ADDITIONAL INFORMATION ******

I sent a disclaimer some time ago for another small patch for the MYSQL() application: you can use that if you want to use this (quite trivial) code.
Comments:By: Andrea Spadaccini (lupino3) 2007-03-31 04:24:30

I uploaded a wrong patch file, but I can't delete it so I can't upload the new one.
Here's the content of the RIGHT patch file:
--- cdr_addon_mysql_orig.c      2007-03-31 10:14:26.000000000 +0200
+++ cdr_addon_mysql.c   2007-03-31 11:22:59.000000000 +0200
@@ -151,7 +151,7 @@
               if ((error = mysql_ping(&mysql))) {
                       connected = 0;
                       records = 0;
-                       switch (error) {
+                       switch (mysql_errno(&mysql)) {
                               case CR_SERVER_GONE_ERROR:
                               case CR_SERVER_LOST:
                                       ast_log(LOG_ERROR, "cdr_mysql: Server has gone away. Attempting to reconnect.\n");
By: Joseph Benden (jbenden) 2007-04-01 17:13:32

I agree it should take the mysql_errno return code. I had made this change back in bug id 4953, but someone suggested it was incorrect - so I reverted it. :(
By: Serge Vecher (serge-v) 2007-04-02 09:18:32

you can always upload a patch with a new name until a bug marshall catches up ;)
By: Andrea Spadaccini (lupino3) 2007-04-04 04:11:26

You are right. Thanks a lot! :)
I've attached the correct patch.
By: Jason Parker (jparker) 2007-04-05 10:54:33

According to the mysql developer documentation at http://dev.mysql.com/doc/refman/5.1/en/mysql-ping.html , mysql_ping() already returns the CR_ error code.
By: Joseph Benden (jbenden) 2007-04-05 11:34:11

Not quite. Read the page carefully:

"Zero if the connection to the server is alive. Non-zero if an error occurred. A non-zero return does not indicate whether the MySQL server itself is down; the connection might be broken for other reasons such as network problems. "

It does NOT return any useful code, ie: non-zero return does not indicate whether the MySQL server itself is down, etc.  

To get that info you NEED mysql_errno() and we need this to know if we should attempt to reconnect.
By: Joshua C. Colp (jcolp) 2007-06-21 10:11:56

Fixed in 1.2 as of revision 404, 1.4 as of revision 405, and trunk as of revision 406. Thanks!