Merge "Fix LDAP connection pool configuration."

Documentation/config-gerrit.txt

@@ -2500,64 +2500,22 @@ etc...
 +
 By default there is no timeout and Gerrit will wait indefinitely.
 
-[[ldap.poolAuthentication]]ldap.poolAuthentication::
-+
-_(Optional)_  A list of space-separated authentication types of
-connections that may be pooled. Valid types are "none", "simple",
-and "DIGEST-MD5".
-+
-Default is "none simple".
+[[ldap-connection-pooling]]
+==== LDAP Connection Pooling
+Once LDAP connection pooling is enabled by setting the link:#ldap.useConnectionPooling[
+ldap.useConnectionPooling] configuration property to `true`, the connection pool
+can be configured using JVM system properties as explained in the
+link:http://docs.oracle.com/javase/7/docs/technotes/guides/jndi/jndi-ldap.html#POOL[
+Java SE Documentation].
 
-[[ldap.poolDebug]]ldap.poolDebug::
-+
-_(Optional)_  A string that indicates the level of debug output
-to produce. Valid values are "fine" (trace connection creation
-and removal) and "all" (all debugging information).
+For standalone Gerrit (running with the embedded Jetty), JVM system properties
+are specified in the link:#container[container section]:
 
-[[ldap.poolInitsize]]ldap.poolInitsize::
-+
-_(Optional)_ The string representation of an integer that
-represents the number of connections per connection identity
-to create when initially creating a connection for the identity.
-+
-Default is 1.
-
-[[ldap.poolMaxsize]]ldap.poolMaxsize::
-+
-_(Optional)_ The string representation of an integer that
-represents the maximum number of connections per connection
-identity that can be maintained concurrently.
-+
-Default is 0, means that there is no maximum size: A request for
-a pooled connection will use an existing pooled idle connection
-or a newly created pooled connection.
-
-[[ldap.poolPrefsize]]ldap.poolPrefsize::
-+
-_(Optional)_ The string representation of an integer that
-represents the preferred number of connections per connection
-identity that should be maintained concurrently.
-+
-Default is 0, means that there is no preferred size: A request
-for a pooled connection will result in a newly created connection
-only if no idle ones are available.
-
-[[ldap.poolProtocol]]ldap.poolProtocol::
-+
-_(Optional)_  A list of space-separated protocol types of
-connections that may be pooled. Valid types are "plain" and "ssl".
-+
-Default is "plain".
-
-[[ldap.poolTimeout]]ldap.poolTimeout::
-+
-_(Optional)_ Specify how long an idle connection may remain
-in the pool without being closed and removed from the pool.
-+
-The value is in the usual time-unit format like "1 s", "100 ms",
-etc...
-+
-By default there is no timeout.
+----
+  javaOptions = -Dcom.sun.jndi.ldap.connect.pool.maxsize=20
+  javaOptions = -Dcom.sun.jndi.ldap.connect.pool.prefsize=10
+  javaOptions = -Dcom.sun.jndi.ldap.connect.pool.timeout=300000
+----
 
 [[mimetype]]
 === Section mimetype

gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java

@@ -60,37 +60,6 @@ import javax.security.auth.login.LoginException;
 @Singleton class Helper {
   static final String LDAP_UUID = "ldap:";
 
-  static private Map<String, String> getPoolProperties(Config config) {
-    if (LdapRealm.optional(config, "useConnectionPooling", false)) {
-      Map<String, String> r = Maps.newHashMap();
-      r.put("com.sun.jndi.ldap.connect.pool", "true");
-
-      String poolDebug = LdapRealm.optional(config, "poolDebug");
-      String poolTimeout = LdapRealm.optional(config, "poolTimeout");
-
-      r.put("com.sun.jndi.ldap.connect.pool.authentication",
-          LdapRealm.optional(config, "poolAuthentication", "none simple"));
-      if (poolDebug != null) {
-        r.put("com.sun.jndi.ldap.connect.pool.debug", poolDebug);
-      }
-      r.put("com.sun.jndi.ldap.connect.pool.initsize",
-          String.valueOf(LdapRealm.optional(config, "poolInitsize", 1)));
-      r.put("com.sun.jndi.ldap.connect.pool.maxsize",
-          String.valueOf(LdapRealm.optional(config, "poolMaxsize", 0)));
-      r.put("com.sun.jndi.ldap.connect.pool.prefsize",
-          String.valueOf(LdapRealm.optional(config, "poolPrefsize", 0)));
-      r.put("com.sun.jndi.ldap.connect.pool.protocol",
-          LdapRealm.optional(config, "poolProtocol", "plain"));
-      if (poolTimeout != null) {
-        r.put("com.sun.jndi.ldap.connect.pool.timeout", Long
-            .toString(ConfigUtil.getTimeUnit(poolTimeout, 0,
-                TimeUnit.MILLISECONDS)));
-      }
-      return r;
-    }
-    return null;
-  }
-
   private final Cache<String, ImmutableSet<String>> parentGroups;
   private final Config config;
   private final String server;
@@ -102,7 +71,7 @@ import javax.security.auth.login.LoginException;
   private volatile LdapSchema ldapSchema;
   private final String readTimeoutMillis;
   private final String connectTimeoutMillis;
-  private final Map<String, String> connectionPoolConfig;
+  private final boolean useConnectionPooling;
 
   @Inject
   Helper(@GerritServerConfig final Config config,
@@ -133,7 +102,8 @@ import javax.security.auth.login.LoginException;
       connectTimeoutMillis = null;
     }
     this.parentGroups = parentGroups;
-    this.connectionPoolConfig = getPoolProperties(config);
+    this.useConnectionPooling =
+        LdapRealm.optional(config, "useConnectionPooling", false);
   }
 
   private Properties createContextProperties() {
@@ -150,14 +120,14 @@ import javax.security.auth.login.LoginException;
     if (connectTimeoutMillis != null) {
       env.put("com.sun.jndi.ldap.connect.timeout", connectTimeoutMillis);
     }
+    if (useConnectionPooling) {
+      env.put("com.sun.jndi.ldap.connect.pool", "true");
+    }
     return env;
   }
 
   DirContext open() throws NamingException, LoginException {
     final Properties env = createContextProperties();
-    if (connectionPoolConfig != null) {
-      env.putAll(connectionPoolConfig);
-    }
     env.put(Context.SECURITY_AUTHENTICATION, authentication);
     env.put(Context.REFERRAL, referral);
     if ("GSSAPI".equals(authentication)) {