From: Alan T. DeKok Date: Mon, 17 Oct 2011 19:49:08 +0000 (+0200) Subject: Release the mutex lock when trying to make a new connection X-Git-Tag: release_3_0_0_beta0~577 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f7aa54d54c5cfc346b62919140a182e99abdf1d0;p=thirdparty%2Ffreeradius-server.git Release the mutex lock when trying to make a new connection The DB might be down, and it could take a LONG time to open a new connection. Instead of holding the mutex lock for long periods of time, we set a flag saying "spawning", and release the lock. This lets other threads access the connection pool, to get open && active connections. The result is that there are fewer situations where the server blocks --- diff --git a/src/main/connection.c b/src/main/connection.c index cc77ff63434..d7ca81ded06 100644 --- a/src/main/connection.c +++ b/src/main/connection.c @@ -65,6 +65,7 @@ struct fr_connection_pool_t { int lifetime; int idle_timeout; int lazy_init; + int spawning; fr_connection_t *head, *tail; @@ -151,6 +152,10 @@ static void fr_connection_link(fr_connection_pool_t *fc, } } + +/* + * Called with the mutex free. + */ static fr_connection_t *fr_connection_spawn(fr_connection_pool_t *fc, time_t now) { @@ -158,10 +163,29 @@ static fr_connection_t *fr_connection_spawn(fr_connection_pool_t *fc, void *conn; rad_assert(fc != NULL); + + pthread_mutex_lock(&fc->mutex); rad_assert(fc->num <= fc->max); - if (fc->last_failed == now) return NULL; + if ((fc->last_failed == now) || fc->spawning) { + pthread_mutex_unlock(&fc->mutex); + return NULL; + } + + fc->spawning = TRUE; + /* + * Unlock the mutex while we try to open a new + * connection. If there are issues with the back-end, + * opening a new connection may take a LONG time. In + * that case, we want the other connections to continue + * to be used. + */ + pthread_mutex_unlock(&fc->mutex); + + DEBUG("%s: Opening additional connection (%i)", + fc->log_prefix, fc->count); + this = rad_malloc(sizeof(*this)); memset(this, 0, sizeof(*this)); @@ -175,17 +199,25 @@ static fr_connection_t *fr_connection_spawn(fr_connection_pool_t *fc, if (!conn) { fc->last_failed = now; free(this); + fc->spawning = FALSE; /* atomic, so no lock is needed */ return NULL; } this->start = now; - this->connection = conn; - this->number = fc->count; + this->connection = conn; - fr_connection_link(fc, this); + /* + * And lock the mutex again while we link the new + * connection back into the pool. + */ + pthread_mutex_lock(&fc->mutex); - fc->count++; + this->number = fc->count++; + fr_connection_link(fc, this); fc->num++; + fc->spawning = FALSE; + + pthread_mutex_unlock(&fc->mutex); return this; } @@ -239,6 +271,7 @@ fr_connection_pool_t *fr_connection_pool_init(CONF_SECTION *parent, fr_connection_t *this; CONF_SECTION *cs; const char *cs_name1, *cs_name2; + time_t now = time(NULL); if (!parent || !ctx || !c || !d) return NULL; @@ -296,10 +329,6 @@ fr_connection_pool_t *fr_connection_pool_init(CONF_SECTION *parent, * not to. */ if (!fc->lazy_init) for (i = 0; i < fc->start; i++) { - time_t now = time(NULL); - - DEBUG("%s: Spawning additional connection (%i)", fc->log_prefix, fc->count); - this = fr_connection_spawn(fc, now); if (!this) { error: @@ -361,7 +390,7 @@ static int fr_connection_manage(fr_connection_pool_t *fc, static int fr_connection_pool_check(fr_connection_pool_t *fc) { - int i, spare, spawn; + int spare, spawn; time_t now = time(NULL); fr_connection_t *this; @@ -372,20 +401,16 @@ static int fr_connection_pool_check(fr_connection_pool_t *fc) spare = fc->num - fc->active; spawn = 0; - if ((fc->num < fc->max) && - (spare < fc->spare)) { + if ((fc->num < fc->max) && (spare < fc->spare)) { spawn = fc->spare - spare; if ((spawn + fc->num) > fc->max) { spawn = fc->max - fc->num; } + if (fc->spawning) spawn = 0; - for (i = 0; i < spawn; i++) { - DEBUG("%s: Spawning additional connection (%i): Not enough free connections", - fc->log_prefix, fc->count); - if (!fr_connection_spawn(fc, now)) { - pthread_mutex_unlock(&fc->mutex); - return 0; - } + if (spawn) { + pthread_mutex_unlock(&fc->mutex); + fr_connection_spawn(fc, now); /* ignore return code */ } } @@ -474,19 +499,22 @@ void *fr_connection_get(fr_connection_pool_t *fc) } if (fc->num == fc->max) { - radlog(L_ERR, "%s: No connections available and at max connection limit", - fc->log_prefix); + /* + * Rate-limit complaints. + */ + if (fc->last_complained != now) { + radlog(L_ERR, "%s: No connections available and at max connection limit", + fc->log_prefix); + fc->last_complained = now; + } pthread_mutex_unlock(&fc->mutex); return NULL; } - DEBUG("%s: Spawning additional connection (%i)", - fc->log_prefix, fc->count); + pthread_mutex_unlock(&fc->mutex); this = fr_connection_spawn(fc, now); - if (!this) { - pthread_mutex_unlock(&fc->mutex); - return NULL; - } + if (!this) return NULL; + pthread_mutex_lock(&fc->mutex); do_return: fc->active++; @@ -533,22 +561,17 @@ void fr_connection_release(fr_connection_pool_t *fc, void *conn) } } + pthread_mutex_unlock(&fc->mutex); + + DEBUG("%s: Released connection (%i)", fc->log_prefix, this->number); /* * We mirror the "spawn on get" functionality by having * "delete on release". If there are too many spare * connections, go manage the pool && clean some up. */ - if ((time(NULL) >= (fc->last_spawned + fc->cleanup_delay)) && - ((fc->num - fc->active) > fc->spare)) { - pthread_mutex_unlock(&fc->mutex); + fr_connection_pool_check(fc); - fr_connection_pool_check(fc); - } else { - pthread_mutex_unlock(&fc->mutex); - } - - DEBUG("%s: Released connection (%i)", fc->log_prefix, this->number); } void *fr_connection_reconnect(fr_connection_pool_t *fc, void *conn)