Database connection failure and retry

Marko Dinic marko at yu.net
Tue Feb 28 17:35:40 CET 2006


>>"Ming-Ching Tiew" <mingching.tiew at redtone.com> wrote:
>>
>> A little while ago I posted about in my configuration, using unixodbc/freetds,
>> we noticed that database/network failure causes permanent disability in 
>> radius server to stop logging and I was given the reply that it is in the 
>> feature of radius server to retry upon failure.
>> 
>> So there is a discrepancy between what I noticed verses the "supposed to be" 
>> behaviour.
>> 
>> Upon further investigation I explored into the code, I found this in sql_mysql.c :-
>> 
>> static int sql_fetch_row(SQLSOCK * sqlsocket, SQL_CONFIG *config)
>> {
>>         rlm_sql_mysql_sock *mysql_sock = sqlsocket->conn;
>> 
>>         /*
>>          *  Check pointer before de-referencing it.
>>          */
>>         if (!mysql_sock->result) {
>>                 return SQL_DOWN;
>>         }
>> 
>>         sqlsocket->row = mysql_fetch_row(mysql_sock->result);
>> 
>>         if (sqlsocket->row == NULL) {
>>                 return sql_check_error(mysql_errno(mysql_sock->sock));
>>         }
>>         return 0;
>> }
>> 
>> 
>> However the code in sql_unixodbc.c is like this :-
>> 
>> 
>> static int sql_fetch_row(SQLSOCK *sqlsocket, SQL_CONFIG *config) {
>>    rlm_sql_unixodbc_sock *unixodbc_sock = sqlsocket->conn;
>>
>>    sqlsocket->row = NULL;
>>
>>    if(SQLFetch(unixodbc_sock->stmt_handle) == SQL_NO_DATA_FOUND)
>>        return 0;
>>
>>    /* XXX Check if return suggests we should return error or SQL_DOWN */
>>
>>    sqlsocket->row = unixodbc_sock->row;
>>    return 0;
>> }
>> 
>> 
>> There is no checking whatsoever, so unixodbc driver is unable to reconnect 
>> upon  failure.
>
>  Ok... are you willing to supply a patch?
>
>  Alan DeKok.
>




A while ago Alan suggested I should use odbc instead of native sybase driver,
so I set up unixODBC 2.2.11 with freetds 0.63 ODBC driver, but I encountered
the same problem as described above. However, I couldn't afford waiting for a
patch, so I patched it myself. I've never used ODBC API before, so I cannot
be sure if this is a complete solution to the problem, or whether it will
work for everyone - the only thing i do know is that it works well for me.

Here it is:

-------------------------------------PATCH STARTS HERE------------------------------------------
--- freeradius-1.1.0/src/modules/rlm_sql/drivers/rlm_sql_unixodbc/sql_unixodbc.c        2005-08-29 19:01:58.000000000 +0200
+++ freeradius-1.1.0-unixodbcfix/src/modules/rlm_sql/drivers/rlm_sql_unixodbc/sql_unixodbc.c    2006-02-28 17:25:27.000000000 +0100
@@ -40,6 +40,7 @@

 /* Forward declarations */
 static char *sql_error(SQLSOCK *sqlsocket, SQL_CONFIG *config);
+static int sql_state(long err_handle, SQLSOCK *sqlsocket, SQL_CONFIG *config);
 static int sql_free_result(SQLSOCK *sqlsocket, SQL_CONFIG *config);
 static int sql_affected_rows(SQLSOCK *sqlsocket, SQL_CONFIG *config);
 static int sql_num_fields(SQLSOCK *sqlsocket, SQL_CONFIG *config);
@@ -67,23 +68,23 @@

     /* 1. Allocate environment handle and register version */
     err_handle = SQLAllocHandle(SQL_HANDLE_ENV,SQL_NULL_HANDLE,&unixodbc_sock->env_handle);
-    if (!SQL_SUCCEEDED(err_handle))
+    if (sql_state(err_handle, sqlsocket, config))
     {
-       radlog(L_ERR, "rlm_sql_unixodbc: Can't allocate environment handle %s\n", sql_error(sqlsocket, config));
+       radlog(L_ERR, "rlm_sql_unixodbc: Can't allocate environment handle\n");
        return -1;
     }
     err_handle = SQLSetEnvAttr(unixodbc_sock->env_handle, SQL_ATTR_ODBC_VERSION, (void*)SQL_OV_ODBC3, 0);
-    if (!SQL_SUCCEEDED(err_handle))
+    if (sql_state(err_handle, sqlsocket, config))
     {
-       radlog(L_ERR, "rlm_sql_unixodbc: Can't register ODBC version %s\n", sql_error(sqlsocket, config));
+       radlog(L_ERR, "rlm_sql_unixodbc: Can't register ODBC version\n");
        SQLFreeHandle(SQL_HANDLE_ENV, unixodbc_sock->env_handle);
        return -1;
     }
     /* 2. Allocate connection handle */
     err_handle = SQLAllocHandle(SQL_HANDLE_DBC, unixodbc_sock->env_handle, &unixodbc_sock->dbc_handle);
-    if (!SQL_SUCCEEDED(err_handle))
+    if (sql_state(err_handle, sqlsocket, config))
     {
-       radlog(L_ERR, "rlm_sql_unixodbc: Can't allocate connection handle %s\n", sql_error(sqlsocket, config));
+       radlog(L_ERR, "rlm_sql_unixodbc: Can't allocate connection handle\n");
        SQLFreeHandle(SQL_HANDLE_ENV, unixodbc_sock->env_handle);
        return -1;
     }
@@ -93,9 +94,9 @@
        (SQLCHAR*) config->sql_server, strlen(config->sql_server),
        (SQLCHAR*) config->sql_login, strlen(config->sql_login),
        (SQLCHAR*) config->sql_password, strlen(config->sql_password));
-    if (!SQL_SUCCEEDED(err_handle))
+    if (sql_state(err_handle, sqlsocket, config))
     {
-       radlog(L_ERR, "rlm_sql_unixodbc: Connection failed %s\n", sql_error(sqlsocket, config));
+       radlog(L_ERR, "rlm_sql_unixodbc: Connection failed\n");
        SQLFreeHandle(SQL_HANDLE_DBC, unixodbc_sock->dbc_handle);
        SQLFreeHandle(SQL_HANDLE_ENV, unixodbc_sock->env_handle);
        return -1;
@@ -103,9 +104,9 @@

     /* 4. Allocate the statement */
     err_handle = SQLAllocStmt(unixodbc_sock->dbc_handle, &unixodbc_sock->stmt_handle);
-    if (!SQL_SUCCEEDED(err_handle))
+    if (sql_state(err_handle, sqlsocket, config))
     {
-       radlog(L_ERR, "rlm_sql_unixodbc: Can't allocate the statement %s\n", sql_error(sqlsocket, config));
+       radlog(L_ERR, "rlm_sql_unixodbc: Can't allocate the statement\n");
        return -1;
     }

@@ -139,16 +140,17 @@
 static int sql_query(SQLSOCK *sqlsocket, SQL_CONFIG *config, char *querystr) {
     rlm_sql_unixodbc_sock *unixodbc_sock = sqlsocket->conn;
     long err_handle;
+    int state;

     if (config->sqltrace)
         radlog(L_DBG, "query:  %s", querystr);

     /* Executing query */
     err_handle = SQLExecDirect(unixodbc_sock->stmt_handle, (SQLCHAR *)querystr, strlen(querystr));
-    if (!SQL_SUCCEEDED(err_handle))
-    {
-       radlog(L_ERR, "rlm_sql_unixodbc: '%s'\n", sql_error(sqlsocket, config));
-       return -1;
+    if ((state = sql_state(err_handle, sqlsocket, config))) {
+       if(state == SQL_DOWN)
+           radlog(L_ERR, "rlm_sql_unixodbc: rlm_sql should attempt to reconnect\n");
+       return state;
     }
     return 0;
 }
@@ -165,9 +167,11 @@
     rlm_sql_unixodbc_sock *unixodbc_sock = sqlsocket->conn;
     SQLINTEGER column, len;
     int numfields;
+    int state;

-    if(sql_query(sqlsocket, config, querystr) < 0)
-       return -1;
+    /* Only state = 0 means success */
+    if((state = sql_query(sqlsocket, config, querystr)))
+       return state;

     numfields=sql_num_fields(sqlsocket, config);
     if(numfields < 0)
@@ -214,11 +218,9 @@
     int num_fields = 0;

     err_handle = SQLNumResultCols(unixodbc_sock->stmt_handle,(SQLSMALLINT *)&num_fields);
-    if (!SQL_SUCCEEDED(err_handle))
-    {
-       radlog(L_ERR, "rlm_sql_unixodbc: '%s'\n", sql_error(sqlsocket, config));
+    if (sql_state(err_handle, sqlsocket, config))
        return -1;
-    }
+
     return num_fields;
 }

@@ -247,14 +249,19 @@
  *************************************************************************/
 static int sql_fetch_row(SQLSOCK *sqlsocket, SQL_CONFIG *config) {
     rlm_sql_unixodbc_sock *unixodbc_sock = sqlsocket->conn;
+    long err_handle;
+    int state;

     sqlsocket->row = NULL;

-    if(SQLFetch(unixodbc_sock->stmt_handle) == SQL_NO_DATA_FOUND)
-       return 0;
-
-    /* XXX Check if return suggests we should return error or SQL_DOWN */
-
+    err_handle = SQLFetch(unixodbc_sock->stmt_handle);
+    if(err_handle == SQL_NO_DATA_FOUND)
+       return 0;
+    if ((state = sql_state(err_handle, sqlsocket, config))) {
+       if(state == SQL_DOWN)
+           radlog(L_ERR, "rlm_sql_unixodbc: rlm_sql should attempt to reconnect\n");
+       return state;
+    }
     sqlsocket->row = unixodbc_sock->row;
     return 0;
 }
@@ -346,6 +353,41 @@
     SQLINTEGER errornum = 0;
     SQLSMALLINT length = 255;
     static char result[1024];  /* NOT thread-safe! */
+
+    rlm_sql_unixodbc_sock *unixodbc_sock = sqlsocket->conn;
+
+    SQLError(
+       unixodbc_sock->env_handle,
+       unixodbc_sock->dbc_handle,
+       unixodbc_sock->stmt_handle,
+       state,
+       &errornum,
+       error,
+       256,
+       &length);
+
+    sprintf(result, "%s %s", state, error);
+    result[sizeof(result) - 1] = '\0'; /* catch idiot thread issues */
+    return result;
+}
+
+/*************************************************************************
+ *
+ *     Function: sql_state
+ *
+ *     Purpose: Returns 0 for success, SQL_DOWN if the error was
+ *               connection related or -1 for other errors
+ *
+ *************************************************************************/
+static int sql_state(long err_handle, SQLSOCK *sqlsocket, SQL_CONFIG *config) {
+    SQLCHAR state[256] = "";
+    SQLCHAR error[256] = "";
+    SQLINTEGER errornum = 0;
+    SQLSMALLINT length = 255;
+    int res = -1;
+
+    if(SQL_SUCCEEDED(err_handle))
+       return 0;               /* on success, just return 0 */

     rlm_sql_unixodbc_sock *unixodbc_sock = sqlsocket->conn;

@@ -359,9 +401,25 @@
        256,
        &length);

-    sprintf(result, "%s %s", state, error);
-    result[sizeof(result) - 1] = '\0'; /* catch idiot thread issues */
-    return result;
+    if(state[0] == '0') {
+       switch(state[1]) {
+       case '1':               /* SQLSTATE 01 class contains info and warning messages */
+           radlog(L_ERR, "rlm_sql_unixodbc: Warning %s %s\n", state, error);
+       case '0':               /* SQLSTATE 00 class means success */
+           res = 0;
+           break;
+       case '8':               /* SQLSTATE 08 class describes various connection errors */
+           radlog(L_ERR, "rlm_sql_unixodbc: SQL down %s %s\n", state, error);
+           res = SQL_DOWN;
+           break;
+       default:                /* any other SQLSTATE means error */
+           radlog(L_ERR, "rlm_sql_unixodbc: Error %s %s\n", state, error);
+           res = -1;
+           break;
+       }
+    }
+
+    return res;
 }

 /*************************************************************************
@@ -378,11 +436,9 @@
     int affected_rows;

     err_handle = SQLRowCount(unixodbc_sock->stmt_handle, (SQLINTEGER *)&affected_rows);
-    if (!SQL_SUCCEEDED(err_handle))
-    {
-       radlog(L_ERR, "rlm_sql_unixodbc: '%s'\n", sql_error(sqlsocket, config));
+    if (sql_state(err_handle, sqlsocket, config))
        return -1;
-    }
+
     return affected_rows;
 }

-------------------------------------PATCH ENDS HERE------------------------------------------

-- 
Best regards,

Marko Dinic, System Engineer
----- 
YUnet International  http://www.eunet.yu
Dubrovacka 35/III,   11000 Belgrade
Tel: +381 11 311 9901;  Fax: + 381 11 311 9901
-----
This  e-mail  is confidential and intended only for the recipient.
Unauthorized  distribution,  modification  or  disclosure  of  its
contents is prohibited. If you have received this e-mail in error,
please notify the sender by telephone  +381 11 311 9901.
-----



More information about the Freeradius-Users mailing list