A Suggestion for rlm_ldap.

[Alister Winfield]

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);