Define max_retries for pool connections (#4908)

Allows control over the number of times a connection operation can be
retried before the module call fails.

Previously this was always set to the number of connections in the pool
- so on a system with a large number of open connections, and a remote
server going slow, this would easily block threads.

diff --git a/raddb/mods-available/ldap b/raddb/mods-available/ldap
index 5d87f848cadda957e64a1254596ba5ad6da5515b..e3032a44d3ac554117ff33179447d033ce03ef35 100644 (file)
--- a/raddb/mods-available/ldap
+++ b/raddb/mods-available/ldap
@@ -696,5 +696,10 @@ ldap {
                #
                #  The solution is to either lower the 'min' connections,
                #  or increase lifetime/idle_timeout.
+
+               #  Maximum number of times an operation can be retried
+               #  if it returns an error which indicates the connection
+               #  needs to be restarted.  This includes timeouts.
+               max_retries = 5
        }
 }

diff --git a/raddb/mods-available/sql b/raddb/mods-available/sql
index 7bcb664d32fecca0cd20c1d81bac13f0e1b9991b..0f435add70c1816005deb12975aabee5d64e5e1c 100644 (file)
--- a/raddb/mods-available/sql
+++ b/raddb/mods-available/sql
@@ -325,6 +325,11 @@ sql {
                #
                #  The solution is to either lower the "min" connections,
                #  or increase lifetime/idle_timeout.
+
+               #  Maximum number of times an operation can be retried
+               #  if it returns an error which indicates the connection
+               #  needs to be restarted.  This includes timeouts.
+               max_retries = 5
        }
 
        # Set to 'yes' to read radius clients from the database ('nas' table)

diff --git a/src/include/connection.h b/src/include/connection.h
index e63820db84e9e3129a71ee8599280c2592f78605..1fd4be4c8e79acc02961bb60437d688a15fb54ff 100644 (file)
--- a/src/include/connection.h
+++ b/src/include/connection.h
@@ -84,6 +84,8 @@ fr_connection_pool_t  *fr_connection_pool_module_init(CONF_SECTION *module,
  */
 int    fr_connection_pool_get_num(fr_connection_pool_t *pool);
 
+int    fr_connection_pool_get_retries(fr_connection_pool_t *pool);
+
 /*
  *     Pool management
  */

diff --git a/src/main/connection.c b/src/main/connection.c
index b5a0eea32eb7f8581bd30fc3ea67d24345b1ddc2..7ae4a2a19bc09ce030512b75a1bf09affca11809 100644 (file)
--- a/src/main/connection.c
+++ b/src/main/connection.c
@@ -97,6 +97,8 @@ struct fr_connection_pool_t {
        uint32_t        spare;                  //!< Number of spare connections to try.
        uint32_t        pending;                //!< Number of pending open connections.
        uint32_t        retry_delay;            //!< seconds to delay re-open after a failed open.
+       uint32_t        max_retries;            //!< Maximum number of retries to attempt for any given
+                                               //!< operation (e.g. query or bind)
        uint32_t        cleanup_interval;       //!< Initial timer for how often we sweep the pool
                                                //!< for free connections. (0 is infinite).
        int             delay_interval;         //!< When we next do a cleanup.  Initialized to
@@ -158,6 +160,7 @@ static const CONF_PARSER connection_config[] = {
        { "cleanup_interval", FR_CONF_OFFSET(PW_TYPE_INTEGER, fr_connection_pool_t, cleanup_interval), "30" },
        { "idle_timeout", FR_CONF_OFFSET(PW_TYPE_INTEGER, fr_connection_pool_t, idle_timeout), "60" },
        { "retry_delay", FR_CONF_OFFSET(PW_TYPE_INTEGER, fr_connection_pool_t, retry_delay), "1" },
+       { "max_retries", FR_CONF_OFFSET(PW_TYPE_INTEGER, fr_connection_pool_t, max_retries), "5" },
        { "spread", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, fr_connection_pool_t, spread), "no" },
        CONF_PARSER_TERMINATOR
 };
@@ -1257,6 +1260,18 @@ int fr_connection_pool_get_num(fr_connection_pool_t *pool)
        return pool->stats.num;
 }
 
+/** Get the number of times an operation should be retried
+ *
+ * The lower of either the number of available connections or
+ * the configured max_retries.
+ *
+ * @param pool to get the retry count for.
+ * @return the number of times an operation can be retried.
+ */
+int fr_connection_pool_get_retries(fr_connection_pool_t *pool)
+{
+       return (pool->max_retries < pool->stats.num) ? pool->max_retries : pool->stats.num;
+}
 
 /** Get the number of connections currently in the pool
  *

diff --git a/src/modules/rlm_ldap/ldap.c b/src/modules/rlm_ldap/ldap.c
index 97fdcbe2f35d57c7f7e6bebbfc44a94c6da4304e..542c0862239dd341be15dc7be46ea3ec2f5295de 100644 (file)
--- a/src/modules/rlm_ldap/ldap.c
+++ b/src/modules/rlm_ldap/ldap.c
@@ -717,7 +717,7 @@ ldap_rcode_t rlm_ldap_bind(rlm_ldap_t const *inst, REQUEST *request, ldap_handle
         *      For sanity, for when no connections are viable,
         *      and we can't make a new one.
         */
-       num = retry ? fr_connection_pool_get_num(inst->pool) : 0;
+       num = retry ? fr_connection_pool_get_retries(inst->pool) : 0;
        for (i = num; i >= 0; i--) {
 #ifdef WITH_SASL
                if (sasl && sasl->mech) {
@@ -877,7 +877,7 @@ ldap_rcode_t rlm_ldap_search(LDAPMessage **result, rlm_ldap_t const *inst, REQUE
         *      For sanity, for when no connections are viable,
         *      and we can't make a new one.
         */
-       for (i = fr_connection_pool_get_num(inst->pool); i >= 0; i--) {
+       for (i = fr_connection_pool_get_retries(inst->pool); i >= 0; i--) {
                (void) ldap_search_ext((*pconn)->handle, dn, scope, filter, search_attrs,
                                       0, serverctrls, clientctrls, &tv, 0, &msgid);
 
@@ -1004,7 +1004,7 @@ ldap_rcode_t rlm_ldap_modify(rlm_ldap_t const *inst, REQUEST *request, ldap_hand
         *      For sanity, for when no connections are viable,
         *      and we can't make a new one.
         */
-       for (i = fr_connection_pool_get_num(inst->pool); i >= 0; i--) {
+       for (i = fr_connection_pool_get_retries(inst->pool); i >= 0; i--) {
                RDEBUG2("Modifying object with DN \"%s\"", dn);
                (void) ldap_modify_ext((*pconn)->handle, dn, mods, NULL, NULL, &msgid);
 

diff --git a/src/modules/rlm_sql/sql.c b/src/modules/rlm_sql/sql.c
index 5cc020eb452a961ed3cb036314d8a25f47b21c6c..a18e00b1fe7bc6c189ef5d0aaaed3b9d3d9d8b00 100644 (file)
--- a/src/modules/rlm_sql/sql.c
+++ b/src/modules/rlm_sql/sql.c
@@ -292,7 +292,7 @@ sql_rcode_t rlm_sql_query(rlm_sql_t *inst, REQUEST *request, rlm_sql_handle_t **
        /*
         *  inst->pool may be NULL is this function is called by mod_conn_create.
         */
-       count = inst->pool ? fr_connection_pool_get_num(inst->pool) : 0;
+       count = inst->pool ? fr_connection_pool_get_retries(inst->pool) : 0;
 
        /*
         *  Here we try with each of the existing connections, then try to create
@@ -392,7 +392,7 @@ sql_rcode_t rlm_sql_select_query(rlm_sql_t *inst, REQUEST *request, rlm_sql_hand
        /*
         *  inst->pool may be NULL is this function is called by mod_conn_create.
         */
-       count = inst->pool ? fr_connection_pool_get_num(inst->pool) : 0;
+       count = inst->pool ? fr_connection_pool_get_retries(inst->pool) : 0;
 
        /*
         *  For sanity, for when no connections are viable, and we can't make a new one