Run through *all* connections in the connection pool, to force connection API to...

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 7605969..1d55cf5 100644 (file)
--- 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 b323e96..63282fd 100644 (file)
--- 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;
 }