On Wed, Oct 14, 2020 at 6:32 PM Michael Osipov <micha...@apache.org> wrote:
> Am 2020-10-14 um 12:32 schrieb Rémy Maucherat: > > On Tue, Oct 13, 2020 at 8:27 PM Michael Osipov <micha...@apache.org> > wrote: > > > >> Am 2020-10-13 um 16:05 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 > >>> > >>> > >>> The following commit(s) were added to refs/heads/8.5.x by this push: > >>> new 883600b Always retry on a new connection, even when pooling > >>> 883600b is described below > >>> > >>> commit 883600b8a77c0be93b3a8dc2404e8d1110970ee7 > >>> Author: remm <r...@apache.org> > >>> AuthorDate: Tue Oct 13 14:19:54 2020 +0200 > >>> > >>> Always retry on a new connection, even when pooling > >>> > >>> This keeps the same very simple design as for the single > connection > >>> scenario, for now. > >>> --- > >>> java/org/apache/catalina/realm/JNDIRealm.java | 22 > >> +++++++++++++++++++--- > >>> webapps/docs/changelog.xml | 5 +++++ > >>> 2 files changed, 24 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/java/org/apache/catalina/realm/JNDIRealm.java > >> b/java/org/apache/catalina/realm/JNDIRealm.java > >>> index 72087ab..98007f7 100644 > >>> --- a/java/org/apache/catalina/realm/JNDIRealm.java > >>> +++ b/java/org/apache/catalina/realm/JNDIRealm.java > >>> @@ -1311,7 +1311,7 @@ public class JNDIRealm extends RealmBase { > >>> close(connection); > >>> > >>> // open a new directory context. > >>> - connection = get(); > >>> + connection = get(true); > >>> > >>> // Try the authentication again. > >>> principal = authenticate(connection, username, > >> credentials); > >>> @@ -2389,12 +2389,28 @@ public class JNDIRealm extends RealmBase { > >>> * @exception NamingException if a directory server error occurs > >>> */ > >>> protected JNDIConnection get() throws NamingException { > >>> + return get(false); > >>> + } > >>> + > >>> + /** > >>> + * Open (if necessary) and return a connection to the configured > >>> + * directory server for this Realm. > >>> + * @param create when pooling, this forces creation of a new > >> connection, > >>> + * for example after an error > >>> + * @return the connection > >>> + * @exception NamingException if a directory server error occurs > >>> + */ > >>> + protected JNDIConnection get(boolean create) throws > NamingException > >> { > >>> JNDIConnection connection = null; > >>> // Use the pool if available, otherwise use the single > >> connection > >>> if (connectionPool != null) { > >>> - connection = connectionPool.pop(); > >>> - if (connection == null) { > >>> + if (create) { > >>> connection = create(); > >>> + } else { > >>> + connection = connectionPool.pop(); > >>> + if (connection == null) { > >>> + connection = create(); > >>> + } > >>> } > >>> } else { > >>> singleConnectionLock.lock(); > >> > >> That suitable and simple approach. > >> > > > > If you have the code for adding a max idle check on hand and tested, you > > should add it IMO, it will be more efficient. > > I will need to give this a couple more weeks of testing. This is what I > have observed today: > > 2020-10-14T16:01:47.039 147.54.155.198 w99sezx0... "GET > /x2tc-proxy-bln/rest/info/targetSystem HTTP/1.1" 200 8 92132 > > > > 20609 2020-10-14T16:00:14 FEIN [https-openssl-apr-8444-exec-166] > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Acquiring > directory server connection from pool > > 20610 2020-10-14T16:00:14 FEIN [https-openssl-apr-8444-exec-166] > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.acquire Directory server > connection from pool exceeds max idle time, closing it > > 20611 2020-10-14T16:00:14 FEIN [https-openssl-apr-8444-exec-166] > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.close Closing directory > server connection > > 20612 2020-10-14T16:00:14 FEIN [https-openssl-apr-8444-exec-166] > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.open Opening new > directory server connection > > 20613 2020-10-14T16:01:47 FEIN [https-openssl-apr-8444-exec-166] > net.sf.michaelo.tomcat.realm.ActiveDirectoryRealm.getUser Searching for > username 'w99sezx0' in base ... > > As you can see it took 90 seconds to server the request because the > connection has expired and close took way too long. In average the > request takes: > > 2020-10-14T13:57:06.730 10.81.50.232 osipovmi@... "GET > /x2tc-proxy-bln/rest/info/targetSystem HTTP/1.1" 200 8 70 > > when the connection is healthy. > Ok, so there's some real incentive to avoid reusing a connection that was idle for too long. Rémy
