Am 2020-10-07 um 22:34 schrieb r...@apache.org:
This is an automated email from the ASF dual-hosted git repository.
remm pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 50de36b7874da98591345e40b374a1e2dd52c188
Author: remm <r...@apache.org>
AuthorDate: Thu Jan 30 17:22:51 2020 +0100
Add connection pool to JNDI realm
This implements a TODO from the class javadoc header.
As described in the javadoc, the idea is to use a pool to avoid blocking
on a single connection, which could possibly become a bottleneck in some
cases. The message formats need to be kept along with the connection
since they are not thread safe.
Preserve the default behavior: sync without pooling (using a Lock object
which is more flexible).
I may backport this since this is limited to the JNDI realm, but only
once it is confirmed to be regression free. Tested with ApacheDS but my
LDAP skills are very limited.
---
java/org/apache/catalina/realm/JNDIRealm.java | 442 ++++++++++++---------
.../apache/catalina/realm/LocalStrings.properties | 1 +
test/org/apache/catalina/realm/TestJNDIRealm.java | 7 +-
webapps/docs/changelog.xml | 3 +
webapps/docs/config/realm.xml | 7 +
5 files changed, 276 insertions(+), 184 deletions(-)
Salut Rémy,
this is a very very nice improvement to the matter and I gave your idea
a spin my public Active Directory realm. Based on my tests with AD I see
room for improvement: (Note that I don't use the JNDIRealm because
because is it too generic and usable for me)
* You might want to consider to introduce a maxIdleTime to avoid stale
connections, e.g., AD has default value of 15 minutes. Yep, I had had
several RSTs resulting in 401s
* Validating connections optionally might be a good option to detect
stale connections or broken ones ny networks issues. This works for me
very fast:
protected boolean validate(DirContextConnection connection) {
if (logger.isDebugEnabled())
logger.debug(sm.getString("activeDirectoryRealm.validate"));
SearchControls controls = new SearchControls();
controls.setSearchScope(SearchControls.OBJECT_SCOPE);
controls.setCountLimit(1);
controls.setReturningAttributes(new String[] { "objectClass" });
controls.setTimeLimit(500);
try {
NamingEnumeration<SearchResult> results =
connection.context.search("", "objectclass=*",
controls);
if (results.hasMore()) {
close(results);
return true;
}
} catch (NamingException e) {
logger.error(sm.getString("activeDirectoryRealm.validate.namingException"), e);
return false;
}
return false;
}
I do this in the acquire() method
* The acquire(), authenticate(), fail, close(), authenticate(),
release() is brittle and proved to fail here. I have a curl script which
does 4 requests in parallel and 1200 requests in total. Pool size is 8.
4 connections in the pool. Rest for 16 minutes, connections are broken
now. Start requests. authenticate() will fail twice because top two
acquired connections from pool are broken. Requests end with 401. I'd
prefer to to modify acquire() is such a fashion:
protected DirContextConnection acquire() {
if (logger.isDebugEnabled())
logger.debug(sm.getString("activeDirectoryRealm.acquire"));
DirContextConnection connection = null;
while (connection == null) {
connection = connectionPool.pop();
if (connection != null) {
long idleTime = System.currentTimeMillis() -
connection.lastBorrowTime;
if (idleTime > maxIdleTime) {
if (logger.isDebugEnabled())
logger.debug(sm.getString("activeDirectoryRealm.exceedMaxIdleTime"));
close(connection);
connection = null;
} else {
boolean valid = validate(connection);
if (valid) {
if (logger.isDebugEnabled())
logger.debug(sm.getString("activeDirectoryRealm.reuse"));
} else {
close(connection);
connection = null;
}
}
} else {
connection = new DirContextConnection();
open(connection);
}
}
WDYT?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
