rework connection management.

diff --git a/src/lib/server/pool.c b/src/lib/server/pool.c
index bb93a3b131a63f11a3aec65b7a13f75e69f00cc6..daed1615fa63e3fe73db73e963cc41b6bc4a322a 100644 (file)
--- a/src/lib/server/pool.c
+++ b/src/lib/server/pool.c
@@ -643,7 +643,7 @@ static int connection_manage(fr_pool_t *pool, request_t *request, fr_pool_connec
  */
 static int connection_check(fr_pool_t *pool, request_t *request)
 {
-       uint32_t        spawn, idle, extra;
+       uint32_t                num, spare;
        fr_time_t               now = fr_time();
        fr_pool_connection_t    *this, *next;
 
@@ -653,118 +653,85 @@ static int connection_check(fr_pool_t *pool, request_t *request)
        }
 
        /*
-        *      Some idle connections are OK, if they're within the
-        *      configured "spare" range.  Any extra connections
-        *      outside of that range can be closed.
+        *      Get "real" number of connections, and count pending
+        *      connections as spare.
         */
-       idle = pool->state.num - pool->state.active;
-       if (idle <= pool->spare) {
-               extra = 0;
-       } else {
-               extra = idle - pool->spare;
-       }
+       num = pool->state.num + pool->state.pending;
+       spare = pool->state.pending + (pool->state.num - pool->state.active);
 
        /*
         *      The other end can close connections.  If so, we'll
         *      have fewer than "min".  When that happens, open more
         *      connections to enforce "min".
+        *
+        *      The code for spawning connections enforces that
+        *      num + pending <= max.
         */
-       if ((pool->state.num + pool->state.pending) < pool->min) {
-               spawn = pool->min - (pool->state.num + pool->state.pending);
-               extra = 0;
-
-               ROPTIONAL(RINFO, INFO, "Need %i more connections to reach min connections (%i)", spawn, pool->min);
-
-       /*
-        *      But if we're already at "min", then don't spawn more,
-        *      and set extra to zero if there's no possibility for extra.
-        */
-       } else if ((pool->state.num + pool->state.pending) >= pool->min) {
-               spawn = 0;
-
-               if (pool->min == pool->max) extra = 0;
+       if (num < pool->min) {
+               ROPTIONAL(RINFO, INFO, "Need %i more connections to reach min connections (%i)", pool->min - num, pool->min);
+               goto add_connection;
+       }
 
        /*
-        *      If we're about to create more than "max",
-        *      don't create more.
+        *      On the odd chance that we've opened too many
+        *      connections, take care of that.
         */
-       } else if ((pool->state.num + pool->state.pending) >= pool->max) {
+       if (num > pool->max) {
                /*
-                *      Ensure we don't spawn more connections.  If
-                *      there are extra idle connections, we can
-                *      delete all of them.
+                *      Pending connections don't get closed as "spare".
                 */
-               spawn = 0;
-
-               /* Otherwise, leave extra alone from above */
+               if (pool->state.pending > 0) goto manage_connections;
 
-       /*
-        *      min < num < max
-        *
-        *      AND we don't have enough idle connections.
-        *      Open some more.
-        */
-       } else if (idle < pool->spare) {
                /*
-                *      Not enough spare connections.  Spawn a few.
-                *      But cap the pool size at "max"
+                *      Otherwise close one of the connections to
+                *      bring us down to "max".
                 */
-               spawn = pool->spare - idle;
-               extra = 0;
-
-               if ((pool->state.num + pool->state.pending + spawn) > pool->max) {
-                       spawn = pool->max - (pool->state.num + pool->state.pending);
-               }
+               goto close_connection;
+       }
 
-               ROPTIONAL(RINFO, INFO, "Need %i more connections to reach %i spares", spawn, pool->spare);
+       /*
+        *      Now that we've enforced min/max connections, try to
+        *      keep the "spare" connections at the correct number.
+        */
 
        /*
-        *      min < num < max
-        *
-        *      We have more than enough idle connections, AND
-        *      some are pending.  Don't open or close any.
+        *      Nothing to do?  Go check all of the connections for
+        *      timeouts, etc.
         */
-       } else if (pool->state.pending) {
-               spawn = 0;
-               extra = 0;
+       if (spare == pool->spare) goto manage_connections;
 
        /*
-        *      We have too many idle connections, but closing
-        *      some would take us below "min", so we only
-        *      close enough to take us to "min".
+        *      Too many spare connections, delete some.
         */
-       } else if ((pool->min + extra) >= pool->state.num) {
-               spawn = 0;
-               extra = pool->state.num - pool->min;
+       if (spare > pool->spare) {
+               fr_pool_connection_t *found;
 
-       } else {
                /*
-                *      Closing the "extra" connections won't take us
-                *      below "min".  It's therefore safe to close
-                *      them all.
+                *      Pending connections don't get closed as "spare".
                 */
-               spawn = 0;
-               /* leave extra alone from above */
-       }
+               if (pool->state.pending > 0) goto manage_connections;
 
-       /*
-        *      Only try to open spares if we're not already attempting to open
-        *      a connection. Avoids spurious log messages.
-        */
-       if (spawn) {
-               pthread_mutex_unlock(&pool->mutex);
-               (void) connection_spawn(pool, request, now, false, true);
-               pthread_mutex_lock(&pool->mutex);
-       }
+               /*
+                *      Don't close too many connections, even they
+                *      are spare.
+                */
+               if (num <= pool->min) goto manage_connections;
 
-       /*
-        *      We haven't spawned connections in a while, and there
-        *      are too many spare ones.  Close the one which has been
-        *      unused for the longest.
-        */
-       if (extra && (now >= (pool->state.last_spawned + pool->delay_interval))) {
-               fr_pool_connection_t *found = NULL;
+               /*
+                *      Too many spares, go close one.
+                */
 
+       close_connection:
+               /*
+                *      Don't close connections too often, in order to
+                *      prevent flapping.
+                */
+               if (now < (pool->state.last_spawned + pool->delay_interval)) goto manage_connections;
+
+               /*
+                *      Find a connection to close.
+                */
+               found = NULL;
                for (this = pool->tail; this != NULL; this = this->prev) {
                        if (this->in_use) continue;
 
@@ -773,28 +740,64 @@ static int connection_check(fr_pool_t *pool, request_t *request)
 
                if (!fr_cond_assert(found)) goto done;
 
-               ROPTIONAL(RDEBUG2, DEBUG2, "Closing connection (%" PRIu64 "), from %d unused connections",
-                         found->number, extra);
+               ROPTIONAL(RDEBUG2, DEBUG2, "Closing connection (%" PRIu64 ") as we have too many unused connections",
+                         found->number);
                connection_close_internal(pool, request, found);
 
                /*
-                *      Decrease the delay for the next time we clean up.
+                *      Decrease the delay for the next time we clean
+                *      up.
                 */
                pool->state.next_delay >>= 1;
                if (pool->state.next_delay == 0) pool->state.next_delay = 1;
                pool->delay_interval += pool->state.next_delay;
+
+               goto manage_connections;
+       }
+
+       /*
+        *      Too few connections, open some more.
+        */
+       if (spare < pool->spare) {
+               /*
+                *      Don't open too many pending connections.
+                */
+               if (pool->state.pending >= pool->max_pending) goto manage_connections;
+
+               /*
+                *      Don't open too many connections, even if we
+                *      need more spares.
+                */
+               if (num >= pool->max) goto manage_connections;
+
+               /*
+                *      Too few spares, go add one.
+                */
+               ROPTIONAL(RINFO, INFO, "Need %i more connections to reach %i spares", pool->spare - spare, pool->spare);
+
+       add_connection:
+               /*
+                *      Only try to open spares if we're not already attempting to open
+                *      a connection. Avoids spurious log messages.
+                */
+               pthread_mutex_unlock(&pool->mutex);
+               (void) connection_spawn(pool, request, now, false, true);
+               pthread_mutex_lock(&pool->mutex);
+               goto manage_connections;
        }
 
        /*
         *      Pass over all of the connections in the pool, limiting
         *      lifetime, idle time, max requests, etc.
         */
+manage_connections:
        for (this = pool->head; this != NULL; this = next) {
                next = this->next;
                connection_manage(pool, request, this, now);
        }
 
        pool->state.last_checked = now;
+
 done:
        pthread_mutex_unlock(&pool->mutex);