[FIXED JENKINS-38784] Update ActiveDirectoryUserDetail on a different…

… thread (#50) [JENKINS-38784] Update ActiveDirectoryUserDetail on a different thread so don't need to wait
jenkinsci · Mar 2, 2017 · fac8df0 · fac8df0
1 parent b5cbbd4
commit fac8df0
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 1 deletion.
diff --git a/src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java b/src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java
@@ -30,6 +30,7 @@
 import groovy.lang.Binding;
 import hudson.Extension;
 import hudson.Functions;
+import hudson.init.Terminator;
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.Descriptor;
 import hudson.model.Hudson;
@@ -38,8 +39,10 @@
 import hudson.security.GroupDetails;
 import hudson.security.SecurityRealm;
 import hudson.security.TokenBasedRememberMeServices2;
+import hudson.util.DaemonThreadFactory;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
+import hudson.util.NamingThreadFactory;
 import hudson.util.Secret;
 import hudson.util.spring.BeanBuilder;
 import org.acegisecurity.Authentication;
@@ -53,6 +56,7 @@
 import org.apache.commons.io.IOUtils;
 import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;
 import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.DataBoundSetter;
@@ -89,6 +93,10 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -189,12 +197,37 @@ public class ActiveDirectorySecurityRealm extends AbstractPasswordBasedSecurityR
      */
     protected List<EnvironmentProperty> environmentProperties;
 
+    /**
+     * The threadPool to update the cache on background
+     */
+    protected transient ExecutorService threadPoolExecutor;
+
     public transient String testDomain;
 
     public transient String testDomainControllers;
 
     public transient String testSite;
 
+    /**
+     * The core pool size for the {@link ExecutorService}
+     */
+    private static final int corePoolSize = Integer.parseInt(System.getProperty("hudson.plugins.active_directory.threadPoolExecutor.corePoolSize", "4"));
+
+    /**
+     * The max pool size for the {@link ExecutorService}
+     */
+    private static final int maxPoolSize = Integer.parseInt(System.getProperty("hudson.plugins.active_directory.threadPoolExecutor.maxPoolSize", "8"));
+
+    /**
+     * The keep alive time for the {@link ExecutorService}
+     */
+    private static final long keepAliveTime = Long.parseLong(System.getProperty("hudson.plugins.active_directory.threadPoolExecutor.keepAliveTime", "10000"));
+
+    /**
+     * The queue size for the {@link ExecutorService}
+     */
+    private static final int queueSize = Integer.parseInt(System.getProperty("hudson.plugins.active_directory.threadPoolExecutor.queueSize", "25"));
+
     public ActiveDirectorySecurityRealm(String domain, String site, String bindName, String bindPassword, String server) {
         this(domain, site, bindName, bindPassword, server, GroupLookupStrategy.AUTO, false);
     }
@@ -228,6 +261,15 @@ public ActiveDirectorySecurityRealm(String domain, List<ActiveDirectoryDomain> d
         this.groupLookupStrategy = groupLookupStrategy;
         this.removeIrrelevantGroups = removeIrrelevantGroups;
         this.cache = cache;
+        this.threadPoolExecutor = new ThreadPoolExecutor(
+                corePoolSize,
+                maxPoolSize,
+                keepAliveTime,
+                TimeUnit.MILLISECONDS,
+                new ArrayBlockingQueue<Runnable>(queueSize),
+                new NamingThreadFactory(new DaemonThreadFactory(), "ActiveDirectory.updateUserCache"),
+                new ThreadPoolExecutor.DiscardPolicy()
+        );
     }
 
     @DataBoundSetter
@@ -397,6 +439,12 @@ public void doAuthTest(StaplerRequest req, StaplerResponse rsp, @QueryParameter
         req.getView(this, "test.jelly").forward(req, rsp);
     }
 
+    @Restricted(DoNotUse.class)
+    @Terminator
+    public void shutDownthreadPoolExecutors() {
+        threadPoolExecutor.shutdown();
+    }
+
     @Extension
     public static final class DescriptorImpl extends Descriptor<SecurityRealm> {
         public String getDisplayName() {

diff --git a/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java b/src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java
@@ -56,14 +56,19 @@
 import javax.naming.directory.SearchResult;
 import javax.naming.ldap.LdapName;
 import java.util.ArrayList;
+import java.util.Date;
 import java.util.HashSet;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.Stack;
+import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -100,6 +105,11 @@ public class ActiveDirectoryUnixAuthenticationProvider extends AbstractActiveDir
      */
     private final Cache<String, ActiveDirectoryGroupDetails> groupCache;
 
+    /**
+     * The threadPool to update the cache on background
+     */
+    private final ExecutorService threadPoolExecutor;
+
     /**
      * Properties to be passed to the current LDAP context
      */
@@ -147,6 +157,8 @@ public ActiveDirectoryUnixAuthenticationProvider(ActiveDirectorySecurityRealm re
         this.userCache = cache.getUserCache();
         this.groupCache = cache.getGroupCache();
 
+        this.threadPoolExecutor = realm.threadPoolExecutor;
+
         Map<String, String> extraEnvVarsMap = ActiveDirectorySecurityRealm.EnvironmentProperty.toMap(realm.environmentProperties);
         // TODO In JDK 8u65 I am facing JDK-8139721, JDK-8139942 which makes the plugin to break. Uncomment line once it is fixed.
         //props.put(LDAP_CONNECT_TIMEOUT, System.getProperty(LDAP_CONNECT_TIMEOUT, DEFAULT_LDAP_CONNECTION_TIMEOUT));
@@ -364,7 +376,24 @@ public UserDetails call() throws AuthenticationException {
                 }
             });
             if (cacheMiss[0] != null) {
-                cacheMiss[0].updateUserInfo();
+                threadPoolExecutor.execute(new Runnable() {
+                    @Override
+                    public void run() {
+                        final String threadName = Thread.currentThread().getName();
+                        Thread.currentThread().setName(threadName + " updating-cache-for-user-" + cacheMiss[0].getUsername());
+                        LOGGER.log(Level.FINEST, "Starting the cache update {0}", new Date());
+                        try {
+                            long t0 = System.currentTimeMillis();
+                            cacheMiss[0].updateUserInfo();
+                            LOGGER.log(Level.FINEST, "Finished the cache update {0}", new Date());
+                            long t1 = System.currentTimeMillis();
+                            LOGGER.log(Level.FINE, "The cache for user {0} took {1} msec", new Object[]{cacheMiss[0].getUsername(), String.valueOf(t1-t0)});
+                        } finally {
+                            Thread.currentThread().setName(threadName);
+                        }
+                    }
+                });
+
             }
         } catch (UncheckedExecutionException e) {
            Throwable t = e.getCause();