A Suggestion for rlm_ldap.
A problem we have had and will continue to have without a small change to rlm_ldap's connection handling is that it defeats the ability of a load balancer to do its job. The issue being that if there is a temporary problem or maintenance on a bunch of LDAP servers all the active connections can end up on a tiny proportion of the servers. I understand that doing 1 query per connection would be terrible for performance so how about have 'max-requests-per-connection' in the code and drop the current connection after doing that many searches. Roughly (and untested) something like this. dogmatix:rlm_ldap alister$ git diff diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c index 450279b..5eb1834 100644 --- a/src/modules/rlm_ldap/rlm_ldap.c +++ b/src/modules/rlm_ldap/rlm_ldap.c @@ -78,6 +78,10 @@ RCSID("$Id$") #define MAX_FAILED_CONNS_RESTART 4 #define MAX_FAILED_CONNS_START 5 +/* When connecting to a server farm its a good idea to reconnect regularly + * to allow a load balancer to do its job correctly.*/ +#define MAX_REQUESTS_PER_CONNECTION 2000 + #ifdef NOVELL_UNIVERSAL_PASSWORD /* Universal Password Length */ @@ -116,6 +120,7 @@ typedef struct ldap_conn { char bound; char locked; int failed_conns; + int query_count; #ifdef HAVE_PTHREAD_H pthread_mutex_t mutex; #endif @@ -856,12 +861,21 @@ retry: } conn->bound = 1; conn->failed_conns = 0; + conn->query_count = 0; + } + /* If we have performed greater than a fixed number of queries on this connection force a reconnect */ + if (conn->query_count > MAX_REQUESTS_PER_CONNECTION) { + conn->bound=0; + conn->query_count=0; + goto retry; } tv.tv_sec = inst->timeout; tv.tv_usec = 0; DEBUG2(" [%s] performing search in %s, with filter %s", inst->xlat_name, search_basedn ? search_basedn : "(null)" , filter); + + conn->query_count++; switch (ldap_search_st(conn->ld, search_basedn, scope, filter, attrs, 0, &tv, result)) { case LDAP_SUCCESS: @@ -1939,7 +1953,6 @@ static int ldap_authenticate(void *instance, REQUEST * request) if (conn1->failed_conns >= MAX_FAILED_CONNS_END){ conn1->failed_conns = MAX_FAILED_CONNS_RESTART; conn1->bound = 0; - } } retry: if (!conn1->bound || conn1->ld == NULL) { @@ -1955,6 +1968,7 @@ retry: } conn1->bound = 1; conn1->failed_conns = 0; + conn1->query_count = 0; } RDEBUG("Performing NMAS Authentication for user: %s, seq: %s \n", user_dn,seq);
participants (1)
-
Alister Winfield