From 5049f5123beec8d1d89c54aedd2598c0c8aa613d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20=C5=BDivkov?= Date: Mon, 19 Jan 2015 17:04:43 +0100 Subject: [PATCH] Fix LDAP connection pool configuration. Commit cd04bbc1 introduced LDAP connection pooling but it made a wrong assumption that connection pool settings can be provided as env variables. According to [1] and also [2] the LDAP connection pool configuration is done via JVM system properties. Only "com.sun.jndi.ldap.connect.pool" is specified as an env variable. [1] http://docs.oracle.com/javase/7/docs/technotes/guides/jndi/jndi-ldap.html#POOL [2] http://stackoverflow.com/questions/22411967/which-ldap-jndi-provider-pool-settings-are-system-properties-and-which-are-envi Change-Id: I71eb1934a23d658a1801afcd125895c59b69581e --- Documentation/config-gerrit.txt | 70 ++++--------------- .../gerrit/server/auth/ldap/Helper.java | 42 ++--------- 2 files changed, 20 insertions(+), 92 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 75c19ca413..437fc8363c 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2505,64 +2505,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 diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java index 795d6d8eeb..b8226fc71c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java +++ b/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 getPoolProperties(Config config) { - if (LdapRealm.optional(config, "useConnectionPooling", false)) { - Map 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> 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 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)) {