"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. -----