From: Arran Cudbard-Bell Date: Tue, 17 Jul 2012 14:04:18 +0000 (+0100) Subject: Run through *all* connections in the connection pool, to force connection API to... X-Git-Tag: release_3_0_0_beta0~127^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2c34154b2e27d1b7ff55bc44465430b5f7ca5b58;p=thirdparty%2Ffreeradius-server.git Run through *all* connections in the connection pool, to force connection API to attempt to reconnect before failing a query. Move connections errors into sql.c Do not reconnect if we find a dead connection in sql_fetch_row --- diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index 7605969c9ca..1d55cf5d6d2 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -184,11 +184,8 @@ static int sql_xlat(void *instance, REQUEST *request, char buffer[21]; /* 64bit max is 20 decimal chars + null byte */ if (rlm_sql_query(&sqlsocket,inst,querystr)) { - radlog(L_ERR, "rlm_sql (%s): database query error, %s: %s", - inst->config->xlat_name, querystr, - (inst->module->sql_error)(sqlsocket, - inst->config)); sql_release_socket(inst,sqlsocket); + return 0; } @@ -226,15 +223,11 @@ static int sql_xlat(void *instance, REQUEST *request, } /* else it's a SELECT statement */ if (rlm_sql_select_query(&sqlsocket,inst,querystr)){ - radlog(L_ERR, "rlm_sql (%s): database query error, %s: %s", - inst->config->xlat_name,querystr, - (inst->module->sql_error)(sqlsocket, inst->config)); sql_release_socket(inst,sqlsocket); return 0; } ret = rlm_sql_fetch_row(&sqlsocket, inst); - if (ret) { RDEBUG("SQL query did not succeed"); (inst->module->sql_finish_select_query)(sqlsocket, inst->config); @@ -295,10 +288,6 @@ static int generate_sql_clients(SQL_INST *inst) if (sqlsocket == NULL) return -1; if (rlm_sql_select_query(&sqlsocket,inst,querystr)){ - radlog(L_ERR, "rlm_sql (%s): database query error, %s: %s", - inst->config->xlat_name,querystr, - (inst->module->sql_error)(sqlsocket, inst->config)); - sql_release_socket(inst,sqlsocket); return -1; } @@ -307,16 +296,16 @@ static int generate_sql_clients(SQL_INST *inst) row = sqlsocket->row; if (row == NULL) break; - /* - * The return data for each row MUST be in the following order: - * - * 0. Row ID (currently unused) - * 1. Name (or IP address) - * 2. Shortname - * 3. Type - * 4. Secret - * 5. Virtual Server (optional) - */ + /* + * The return data for each row MUST be in the following order: + * + * 0. Row ID (currently unused) + * 1. Name (or IP address) + * 2. Shortname + * 3. Type + * 4. Secret + * 5. Virtual Server (optional) + */ if (!row[0]){ radlog(L_ERR, "rlm_sql (%s): No row id found on pass %d",inst->config->xlat_name,i); continue; @@ -544,10 +533,6 @@ static int sql_get_grouplist (SQL_INST *inst, SQLSOCK *sqlsocket, REQUEST *reque } if (rlm_sql_select_query(&sqlsocket, inst, querystr) < 0) { - radlog_request(L_ERR, 0, request, - "database query error, %s: %s", - querystr, - (inst->module->sql_error)(sqlsocket,inst->config)); return -1; } while (rlm_sql_fetch_row(&sqlsocket, inst) == 0) { @@ -1171,7 +1156,7 @@ static int rlm_sql_accounting(void *instance, REQUEST * request) { SQLSOCK *sqlsocket = NULL; VALUE_PAIR *pair; SQL_INST *inst = instance; - int ret = RLM_MODULE_OK; + int ret = RLM_MODULE_OK; int numaffected = 0; int acctstatustype = 0; char querystr[MAX_QUERY_LEN]; @@ -1211,8 +1196,8 @@ static int rlm_sql_accounting(void *instance, REQUEST * request) { return(RLM_MODULE_FAIL); if (*querystr) { /* non-empty query */ if (rlm_sql_query(&sqlsocket, inst, querystr)) { - radlog_request(L_ERR, 0, request, "Couldn't update SQL accounting for Acct On/Off packet - %s", - (inst->module->sql_error)(sqlsocket, inst->config)); + radlog_request(L_ERR, 0, request, "Couldn't update SQL accounting for Acct On/Off packet"); + ret = RLM_MODULE_FAIL; } /* @@ -1242,8 +1227,8 @@ static int rlm_sql_accounting(void *instance, REQUEST * request) { return(RLM_MODULE_FAIL); if (*querystr) { /* non-empty query */ if (rlm_sql_query(&sqlsocket, inst, querystr)) { - radlog_request(L_ERR, 0, request, "Couldn't update SQL accounting ALIVE record - %s", - (inst->module->sql_error)(sqlsocket, inst->config)); + radlog_request(L_ERR, 0, request, "Couldn't update SQL accounting ALIVE record"); + ret = RLM_MODULE_FAIL; } else { @@ -1260,8 +1245,8 @@ static int rlm_sql_accounting(void *instance, REQUEST * request) { query_log(request, inst, querystr); if (*querystr) { /* non-empty query */ if (rlm_sql_query(&sqlsocket, inst, querystr)) { - radlog_request(L_ERR, 0, request, "Couldn't insert SQL accounting ALIVE record - %s", - (inst->module->sql_error)(sqlsocket, inst->config)); + radlog_request(L_ERR, 0, request, "Couldn't insert SQL accounting ALIVE record"); + ret = RLM_MODULE_FAIL; } else { numaffected = (inst->module->sql_affected_rows)(sqlsocket, inst->config); @@ -1294,8 +1279,7 @@ static int rlm_sql_accounting(void *instance, REQUEST * request) { return(RLM_MODULE_FAIL); if (*querystr) { /* non-empty query */ if (rlm_sql_query(&sqlsocket, inst, querystr)) { - radlog_request(L_ERR, 0, request, "Couldn't insert SQL accounting START record - %s", - (inst->module->sql_error)(sqlsocket, inst->config)); + radlog_request(L_ERR, 0, request, "Couldn't insert SQL accounting START record"); /* * We failed the insert above. It's probably because @@ -1307,8 +1291,8 @@ static int rlm_sql_accounting(void *instance, REQUEST * request) { if (*querystr) { /* non-empty query */ if (rlm_sql_query(&sqlsocket, inst, querystr)) { - radlog_request(L_ERR, 0, request, "Couldn't update SQL accounting START record - %s", - (inst->module->sql_error)(sqlsocket, inst->config)); + radlog_request(L_ERR, 0, request, "Couldn't update SQL accounting START record"); + ret = RLM_MODULE_FAIL; } } @@ -1340,8 +1324,8 @@ static int rlm_sql_accounting(void *instance, REQUEST * request) { return(RLM_MODULE_FAIL); if (*querystr) { /* non-empty query */ if (rlm_sql_query(&sqlsocket, inst, querystr)) { - radlog_request(L_ERR, 0, request, "Couldn't update SQL accounting STOP record - %s", - (inst->module->sql_error)(sqlsocket, inst->config)); + radlog_request(L_ERR, 0, request, "Couldn't update SQL accounting STOP record"); + ret = RLM_MODULE_FAIL; } else { @@ -1376,9 +1360,8 @@ static int rlm_sql_accounting(void *instance, REQUEST * request) { if (*querystr) { /* non-empty query */ if (rlm_sql_query(&sqlsocket, inst, querystr)) { - radlog_request(L_ERR, 0, request, "Couldn't insert SQL accounting STOP record - %s", - - (inst->module->sql_error)(sqlsocket, inst->config)); + radlog_request(L_ERR, 0, request, "Couldn't insert SQL accounting STOP record"); + ret = RLM_MODULE_FAIL; } else { numaffected = (inst->module->sql_affected_rows)(sqlsocket, inst->config); @@ -1458,13 +1441,11 @@ static int rlm_sql_checksimul(void *instance, REQUEST * request) { return RLM_MODULE_FAIL; if(rlm_sql_select_query(&sqlsocket, inst, querystr)) { - radlog(L_ERR, "rlm_sql (%s) sql_checksimul: Database query failed", inst->config->xlat_name); sql_release_socket(inst, sqlsocket); return RLM_MODULE_FAIL; } ret = rlm_sql_fetch_row(&sqlsocket, inst); - if (ret != 0) { (inst->module->sql_finish_select_query)(sqlsocket, inst->config); sql_release_socket(inst, sqlsocket); @@ -1498,7 +1479,6 @@ static int rlm_sql_checksimul(void *instance, REQUEST * request) { radius_xlat(querystr, sizeof(querystr), inst->config->simul_verify_query, request, sql_escape_func); if(rlm_sql_select_query(&sqlsocket, inst, querystr)) { - radlog_request(L_ERR, 0, request, "Database query error"); sql_release_socket(inst, sqlsocket); return RLM_MODULE_FAIL; } @@ -1632,10 +1612,8 @@ static int rlm_sql_postauth(void *instance, REQUEST *request) { /* Process the query */ if (rlm_sql_query(&sqlsocket, inst, querystr)) { - radlog(L_ERR, "rlm_sql (%s) in sql_postauth: Database query error - %s", - inst->config->xlat_name, - (inst->module->sql_error)(sqlsocket, inst->config)); sql_release_socket(inst, sqlsocket); + return RLM_MODULE_FAIL; } (inst->module->sql_finish_query)(sqlsocket, inst->config); diff --git a/src/modules/rlm_sql/sql.c b/src/modules/rlm_sql/sql.c index b323e9603dc..63282fd1caf 100644 --- a/src/modules/rlm_sql/sql.c +++ b/src/modules/rlm_sql/sql.c @@ -253,24 +253,20 @@ int rlm_sql_fetch_row(SQLSOCK **sqlsocket, SQL_INST *inst) { int ret; - if ((*sqlsocket)->conn) { - ret = (inst->module->sql_fetch_row)(*sqlsocket, inst->config); - } else { - ret = SQL_DOWN; + if (!*sqlsocket || !(*sqlsocket)->conn) { + return -1; } - - if (ret == SQL_DOWN) { - *sqlsocket = fr_connection_reconnect(inst->pool, *sqlsocket); - if (!*sqlsocket) return -1; - - /* retry the query on the newly connected socket */ - ret = (inst->module->sql_fetch_row)(*sqlsocket, inst->config); - - if (ret) { - radlog(L_ERR, "rlm_sql (%s): failed after re-connect", - inst->config->xlat_name); - return -1; - } + + /* + * We can't implement reconnect logic here, because the caller may require + * the original connection to free up queries or result sets associated with + * that connection. + */ + ret = (inst->module->sql_fetch_row)(*sqlsocket, inst->config); + + if (ret < 0) { + radlog(L_ERR, "rlm_sql (%s): error fetching row: %s", inst->config->xlat_name, + (inst->module->sql_error)(*sqlsocket, inst->config)); } return ret; @@ -294,27 +290,34 @@ int rlm_sql_query(SQLSOCK **sqlsocket, SQL_INST *inst, char *query) return -1; } - if ((*sqlsocket)->conn) { - ret = (inst->module->sql_query)(*sqlsocket, inst->config, query); - } else { - ret = SQL_DOWN; + if (!*sqlsocket || !(*sqlsocket)->conn) { + ret = -1; + goto sql_down; } - - if (ret == SQL_DOWN) { - *sqlsocket = fr_connection_reconnect(inst->pool, *sqlsocket); - if (!*sqlsocket) return -1; - - /* retry the query on the newly connected socket */ + + while (1) { ret = (inst->module->sql_query)(*sqlsocket, inst->config, query); - - if (ret) { - radlog(L_ERR, "rlm_sql (%s): failed after re-connect", - inst->config->xlat_name); - return -1; + /* + * Run through all available sockets until we exhaust all existing sockets + * in the pool and fail to establish a *new* connection. + */ + if (ret == SQL_DOWN) { + sql_down: + *sqlsocket = fr_connection_reconnect(inst->pool, *sqlsocket); + if (!*sqlsocket) return -1; + + continue; + } + + if (ret < 0) { + radlog(L_ERR, "rlm_sql (%s): database query error, %s: %s", + inst->config->xlat_name, + query, + (inst->module->sql_error)(*sqlsocket, inst->config)); } + + return ret; } - - return ret; } /************************************************************************* @@ -335,28 +338,34 @@ int rlm_sql_select_query(SQLSOCK **sqlsocket, SQL_INST *inst, char *query) return -1; } - if ((*sqlsocket)->conn) { - ret = (inst->module->sql_select_query)(*sqlsocket, inst->config, - query); - } else { - ret = SQL_DOWN; + if (!*sqlsocket || !(*sqlsocket)->conn) { + ret = -1; + goto sql_down; } - - if (ret == SQL_DOWN) { - *sqlsocket = fr_connection_reconnect(inst->pool, *sqlsocket); - if (!*sqlsocket) return -1; - - /* retry the query on the newly connected socket */ + + while (1) { ret = (inst->module->sql_select_query)(*sqlsocket, inst->config, query); - - if (ret) { - radlog(L_ERR, "rlm_sql (%s): failed after re-connect", - inst->config->xlat_name); - return -1; + /* + * Run through all available sockets until we exhaust all existing sockets + * in the pool and fail to establish a *new* connection. + */ + if (ret == SQL_DOWN) { + sql_down: + *sqlsocket = fr_connection_reconnect(inst->pool, *sqlsocket); + if (!*sqlsocket) return -1; + + continue; } + + if (ret < 0) { + radlog(L_ERR, "rlm_sql (%s): database query error, %s: %s", + inst->config->xlat_name, + query, + (inst->module->sql_error)(*sqlsocket, inst->config)); + } + + return ret; } - - return ret; }