Add a parameter to control LDAP read timeout time
This is to solve b/13730776 (by allowing a user to add a line to
the adaptor-config.properties file to increase the timeout time).
diff --git a/src/com/google/enterprise/adaptor/ad/AdAdaptor.java b/src/com/google/enterprise/adaptor/ad/AdAdaptor.java
index 104b569..b18118f 100644
--- a/src/com/google/enterprise/adaptor/ad/AdAdaptor.java
+++ b/src/com/google/enterprise/adaptor/ad/AdAdaptor.java
@@ -65,6 +65,7 @@
private Map<String, String> localizedStrings;
private boolean feedBuiltinGroups;
private GroupCatalog lastCompleteGroupCatalog = null;
+ private String ldapTimeoutInMillis;
@Override
public void initConfig(Config config) {
@@ -78,6 +79,7 @@
config.addKey("ad.localized.AuthenticatedUsers", "Authenticated Users");
config.addKey("ad.localized.Builtin", "BUILTIN");
config.addKey("ad.feedBuiltinGroups", "false");
+ config.addKey("ad.ldapReadTimeoutSecs", "90");
}
@Override
@@ -90,6 +92,8 @@
config.getValue("ad.defaultPassword"));
feedBuiltinGroups = Boolean.parseBoolean(
config.getValue("ad.feedBuiltinGroups"));
+ ldapTimeoutInMillis = parseLdapTimeoutInMillis(
+ config.getValue("ad.ldapReadTimeoutSecs"));
// register for incremental pushes
context.setPollingIncrementalLister(this);
List<Map<String, String>> serverConfigs
@@ -126,7 +130,8 @@
throw new IllegalStateException("password not specified for host "
+ host);
}
- AdServer adServer = newAdServer(method, host, port, principal, passwd);
+ AdServer adServer = newAdServer(method, host, port, principal, passwd,
+ ldapTimeoutInMillis);
adServer.initialize();
servers.add(adServer);
Map<String, String> dup = new TreeMap<String, String>(singleServerConfig);
@@ -142,8 +147,22 @@
*/
@VisibleForTesting
AdServer newAdServer(Method method, String host, int port,
- String principal, String passwd) {
- return new AdServer(method, host, port, principal, passwd);
+ String principal, String passwd, String ldapTimeoutInMillis) {
+ return new AdServer(method, host, port, principal, passwd,
+ ldapTimeoutInMillis);
+ }
+
+ private static String parseLdapTimeoutInMillis(String timeInSeconds) {
+ if (timeInSeconds.equals("0") || timeInSeconds.trim().equals("")) {
+ timeInSeconds = "90";
+ log.log(Level.CONFIG, "ad.ldapReadTimeoutSecs set to default of 90 sec.");
+ }
+ try {
+ return String.valueOf(1000L * Integer.parseInt(timeInSeconds));
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException("invalid ad.ldapReadTimeoutSecs: " +
+ timeInSeconds);
+ }
}
/** This adaptor does not serve documents. */
diff --git a/src/com/google/enterprise/adaptor/ad/AdServer.java b/src/com/google/enterprise/adaptor/ad/AdServer.java
index 4c9480c..39a0c90 100644
--- a/src/com/google/enterprise/adaptor/ad/AdServer.java
+++ b/src/com/google/enterprise/adaptor/ad/AdServer.java
@@ -62,15 +62,17 @@
private long highestCommittedUSN;
private String invocationID;
private String dnsRoot;
+ private String ldapTimeoutInMillis;
public AdServer(Method connectMethod, String hostName,
- int port, String principal, String password) {
+ int port, String principal, String password, String ldapTimeoutInMillis) {
this(hostName, createLdapContext(connectMethod, hostName, port,
- principal, password));
+ principal, password, ldapTimeoutInMillis));
this.connectMethod = connectMethod;
this.port = port;
this.principal = principal;
this.password = password;
+ this.ldapTimeoutInMillis = ldapTimeoutInMillis;
}
@VisibleForTesting
@@ -85,7 +87,8 @@
* Called (only) by public constructor
*/
private static LdapContext createLdapContext(Method connectMethod,
- String hostName, int port, String principal, String password) {
+ String hostName, int port, String principal, String password,
+ String ldapTimeoutInMillis) {
Hashtable<String, String> env = new Hashtable<String, String>();
if (null == connectMethod || null == hostName
|| null == principal || null == password) {
@@ -106,8 +109,7 @@
"com.sun.jndi.ldap.LdapCtxFactory");
// Connecting to configuration naming context is very slow for crawl users
// in large multidomain environment, which belong to thousands of groups
- // TODO(pjo or myk): make this configurable
- env.put("com.sun.jndi.ldap.read.timeout", "90000");
+ env.put("com.sun.jndi.ldap.read.timeout", ldapTimeoutInMillis);
env.put(Context.SECURITY_AUTHENTICATION, "simple");
env.put(Context.SECURITY_PRINCIPAL, principal);
env.put(Context.SECURITY_CREDENTIALS, password);
@@ -125,7 +127,7 @@
@VisibleForTesting
void recreateLdapContext() {
ldapContext = createLdapContext(connectMethod, hostName, port, principal,
- password);
+ password, ldapTimeoutInMillis);
}
/**
@@ -146,6 +148,15 @@
"Reconnecting to AdServer after detecting issue", ce);
recreateLdapContext();
attributes = ldapContext.getAttributes("");
+ } catch (NamingException ne) {
+ if (ne.getMessage() != null
+ && ne.getMessage().contains("read timed out")) {
+ LOGGER.log(Level.WARNING, "Read timeout insufficient", ne);
+ LOGGER.log(Level.WARNING, "Consider increasing the value of "
+ + "``ad.ldapReadTimeoutSeconds'' in the config file.");
+ }
+ // rethrow the exception, whether or not we were able to give advice.
+ throw(ne);
}
dn = attributes.get("defaultNamingContext").get(0).toString();
dsServiceName = attributes.get("dsServiceName").get(0).toString();
diff --git a/test/com/google/enterprise/adaptor/ad/AdAdaptorTest.java b/test/com/google/enterprise/adaptor/ad/AdAdaptorTest.java
index f5cc466..14b4228 100644
--- a/test/com/google/enterprise/adaptor/ad/AdAdaptorTest.java
+++ b/test/com/google/enterprise/adaptor/ad/AdAdaptorTest.java
@@ -756,6 +756,32 @@
configEntries.put("ad.servers.server2.method", "standard");
configEntries.put("ad.defaultUser", "defaultUser");
configEntries.put("ad.defaultPassword", "password");
+ configEntries.put("ad.ldapReadTimeoutSecs", "");
+ configEntries.put("server.port", "5680");
+ configEntries.put("server.dashboardPort", "5681");
+ pushGroupDefinitions(adAdaptor, configEntries, pusher, /*fullPush=*/ true);
+ Map<GroupPrincipal, Collection<Principal>> results = pusher.getGroups();
+ // the above (eventually) calls AdAdaptor.init() with the specified config.
+ }
+
+ @Test
+ public void testFakeAdaptorInitZeroTimeout() throws Exception {
+ AdAdaptor adAdaptor = new FakeAdaptor();
+ AccumulatingDocIdPusher pusher = new AccumulatingDocIdPusher();
+ Map<String, String> configEntries = new HashMap<String, String>();
+ configEntries.put("gsa.hostname", "localhost");
+ configEntries.put("ad.servers", "server1,server2");
+ configEntries.put("ad.servers.server1.host", "localhost");
+ configEntries.put("ad.servers.server1.port", "1234");
+ configEntries.put("ad.servers.server1.user", "user-override");
+ configEntries.put("ad.servers.server1.method", "ssl");
+ configEntries.put("ad.servers.server2.host", "localhost");
+ configEntries.put("ad.servers.server2.port", "1234");
+ configEntries.put("ad.servers.server2.password", "password-override");
+ configEntries.put("ad.servers.server2.method", "standard");
+ configEntries.put("ad.defaultUser", "defaultUser");
+ configEntries.put("ad.defaultPassword", "password");
+ configEntries.put("ad.ldapReadTimeoutSecs", "0");
configEntries.put("server.port", "5680");
configEntries.put("server.dashboardPort", "5681");
pushGroupDefinitions(adAdaptor, configEntries, pusher, /*fullPush=*/ true);
@@ -824,6 +850,27 @@
}
@Test
+ public void testFakeAdaptorInitBadTimeout() throws Exception {
+ AdAdaptor adAdaptor = new FakeAdaptor();
+ Map<String, String> configEntries = new HashMap<String, String>();
+ configEntries.put("gsa.hostname", "localhost");
+ configEntries.put("ad.servers", "server1");
+ configEntries.put("ad.servers.server1.host", "localhost");
+ configEntries.put("ad.servers.server1.port", "1234");
+ configEntries.put("ad.defaultUser", "defaultUser");
+ configEntries.put("ad.defaultPassword", "password");
+ configEntries.put("ad.ldapReadTimeoutSecs", "bogus");
+ configEntries.put("server.port", "5680");
+ configEntries.put("server.dashboardPort", "5681");
+ try {
+ initializeAdaptorConfig(adAdaptor, configEntries);
+ fail("Did not catch expected exception");
+ } catch (IllegalArgumentException iae) {
+ assertTrue(iae.toString().contains("invalid ad.ldapReadTimeoutSecs"));
+ }
+ }
+
+ @Test
public void testFakeAdaptorGetDocIds() throws Exception {
AdAdaptor adAdaptor = new FakeAdaptor();
AccumulatingDocIdPusher pusher = new AccumulatingDocIdPusher();
@@ -864,7 +911,7 @@
+ "(&(objectClass=user)(objectCategory=person)))";
@Override
AdServer newAdServer(Method method, String host, int port,
- String principal, String passwd) {
+ String principal, String passwd, String ldapTimeoutInMillis) {
MockLdapContext ldapContext = null;
try {
ldapContext = mockLdapContextForMakeDefs(false);
@@ -1150,7 +1197,7 @@
public class FakeAdaptor extends AdAdaptor {
@Override
AdServer newAdServer(Method method, String host, int port,
- String principal, String passwd) {
+ String principal, String passwd, String ldapTimeoutInMillis) {
MockLdapContext ldapContext = null;
try {
ldapContext = mockLdapContextForMakeDefs(false);
diff --git a/test/com/google/enterprise/adaptor/ad/AdServerTest.java b/test/com/google/enterprise/adaptor/ad/AdServerTest.java
index 0d95a6c..bb6b5ab 100644
--- a/test/com/google/enterprise/adaptor/ad/AdServerTest.java
+++ b/test/com/google/enterprise/adaptor/ad/AdServerTest.java
@@ -40,56 +40,71 @@
@Test
public void testNPEOnNullConnectMethod() {
thrown.expect(NullPointerException.class);
- AdServer adServer = new AdServer(null, "hostname", 1234, "principal", "pw");
+ AdServer adServer = new AdServer(null, "hostname", 1234, "principal", "pw",
+ "90000");
}
@Test
public void testNPEOnNullHostname() {
thrown.expect(NullPointerException.class);
- AdServer adServer = new AdServer(Method.SSL, null, 1234, "principal", "pw");
+ AdServer adServer = new AdServer(Method.SSL, null, 1234, "principal", "pw",
+ "90000");
}
@Test
public void testIAEOnEmptyHostname() {
thrown.expect(IllegalArgumentException.class);
- AdServer adServer = new AdServer(Method.SSL, "", 1234, "principal", "pw");
+ AdServer adServer = new AdServer(Method.SSL, "", 1234, "principal", "pw",
+ "90000");
}
@Test
public void testNPEOnNullPrincipal() {
thrown.expect(NullPointerException.class);
- AdServer adServer = new AdServer(Method.SSL, "hostname", 1234, null, "pw");
+ AdServer adServer = new AdServer(Method.SSL, "hostname", 1234, null, "pw",
+ "90000");
}
@Test
public void testIAEOnEmptyPrincipal() throws Exception {
thrown.expect(IllegalArgumentException.class);
- AdServer adServer = new AdServer(Method.SSL, "hostname", 1234, "", "pw");
+ AdServer adServer = new AdServer(Method.SSL, "hostname", 1234, "", "pw",
+ "90000");
}
@Test
public void testNPEOnNullPassword() {
thrown.expect(NullPointerException.class);
- AdServer adServer = new AdServer(Method.SSL, "host", 1234, "princ", null);
+ AdServer adServer = new AdServer(Method.SSL, "host", 1234, "princ", null,
+ "90000");
}
@Test
public void testIAEOnEmptyPassword() {
thrown.expect(IllegalArgumentException.class);
- AdServer adServer = new AdServer(Method.SSL, "hostname", 1234, "princ", "");
+ AdServer adServer = new AdServer(Method.SSL, "hostname", 1234, "princ", "",
+ "90000");
+ }
+
+ @Test
+ public void testIAEOnBogusTimeout() {
+ thrown.expect(IllegalArgumentException.class);
+ AdServer adServer = new AdServer(Method.SSL, "", 1234, "principal", "pw",
+ "bogusTimeout");
}
@Test
public void testPublicSSLConstructor() {
thrown.expect(AssertionError.class);
- AdServer adServer = new AdServer(Method.SSL, "localhost", 389, " ", " ");
+ AdServer adServer = new AdServer(Method.SSL, "localhost", 389, " ", " ",
+ "90000");
}
@Test
public void testPublicStandardConstructor() {
thrown.expect(AssertionError.class);
AdServer adServer =
- new AdServer(Method.STANDARD, "localhost", 389, " ", " ");
+ new AdServer(Method.STANDARD, "localhost", 389, " ", " ", "90000");
}
@Test
@@ -179,6 +194,44 @@
}
@Test
+ public void testEnsureOnetimeException() throws Exception {
+ MockLdapContext ldapContext = new MockLdapContext() {
+ boolean firstTime = true;
+ @Override
+ public Attributes getAttributes(String name) throws NamingException {
+ if (firstTime) {
+ firstTime = false;
+ throw new CommunicationException("testing");
+ } else {
+ return super.getAttributes(name);
+ }
+ }
+ };
+ addStandardKeysAndResults(ldapContext);
+ AdServer adServer = new AdServer("localhost", ldapContext) {
+ @Override
+ void recreateLdapContext() {
+ // do nothing
+ }
+ };
+ adServer.initialize();
+ }
+
+ @Test
+ public void testEnsureConnectionTimesOut() throws Exception {
+ thrown.expect(RuntimeException.class);
+ MockLdapContext ldapContext = new MockLdapContext() {
+ @Override
+ public Attributes getAttributes(String name) throws NamingException {
+ throw new NamingException("read timed out");
+ }
+ };
+ addStandardKeysAndResults(ldapContext);
+ AdServer adServer = new AdServer("localhost", ldapContext);
+ adServer.initialize();
+ }
+
+ @Test
public void testSearchReturnsOneUser() throws Exception {
MockLdapContext ldapContext = new MockLdapContext();
addStandardKeysAndResults(ldapContext);
