From: Alan T. DeKok Date: Thu, 5 Aug 2021 20:24:16 +0000 (-0400) Subject: rework connection management. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dd858f934e05efc67830d5f5801f8177ccf536aa;p=thirdparty%2Ffreeradius-server.git rework connection management. --- diff --git a/src/lib/server/pool.c b/src/lib/server/pool.c index bb93a3b131a..daed1615fa6 100644 --- 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);