Merge branch 'JENKINS-4756'

* JENKINS-4756: minor touch up FIXED JENKINS-4756 the executor number was not always unique. Computer.executors.size() does not provide a unique number within one Computer.
jenkinsci · Feb 5, 2011 · f88a2ba · f88a2ba
2 parents 6c52125 + 36d9b56
commit f88a2ba
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 16 deletions.
diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java
@@ -25,12 +25,10 @@
 package hudson.model;
 
 import hudson.EnvVars;
-import hudson.FilePath;
 import hudson.Util;
 import hudson.cli.declarative.CLIMethod;
 import hudson.console.AnnotatedLargeText;
 import hudson.model.Descriptor.FormException;
-import hudson.model.Hudson.MasterComputer;
 import hudson.model.queue.WorkUnit;
 import hudson.node_monitors.NodeMonitor;
 import hudson.remoting.Channel;
@@ -59,7 +57,6 @@
 import org.kohsuke.stapler.HttpResponses;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpRedirect;
-import org.kohsuke.stapler.WebMethod;
 import org.kohsuke.stapler.export.Exported;
 import org.kohsuke.stapler.export.ExportedBean;
 import org.kohsuke.args4j.Option;
@@ -69,11 +66,7 @@
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.StringWriter;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Enumeration;
+import java.util.*;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -596,11 +589,22 @@ private synchronized void setNumExecutors(int n) {
                     e.interrupt();
         } else {
             // if the number is increased, add new ones
-            while(executors.size()<numExecutors) {
-                Executor e = new Executor(this, executors.size());
-                e.start();
-                executors.add(e);
-            }
+            addNewExecutorIfNecessary();
+        }
+    }
+
+    private void addNewExecutorIfNecessary() {
+        Set<Integer> availableNumbers  = new HashSet<Integer>();
+        for (int i = 0; i < numExecutors; i++)
+            availableNumbers.add(i);
+
+        for (Executor executor : executors)
+            availableNumbers.remove(executor.getNumber());
+
+        for (Integer number : availableNumbers) {
+            Executor e = new Executor(this, number);
+            e.start();
+            executors.add(e);
         }
     }
 
@@ -700,6 +704,7 @@ public final long getDemandStartMilliseconds() {
      */
     /*package*/ synchronized void removeExecutor(Executor e) {
         executors.remove(e);
+        addNewExecutorIfNecessary();
         if(!isAlive())
             Hudson.getInstance().removeComputer(this);
     }

diff --git a/test/src/test/java/hudson/model/ExecutorTest.java b/test/src/test/java/hudson/model/ExecutorTest.java
@@ -1,6 +1,7 @@
 package hudson.model;
 
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import org.jvnet.hudson.test.Bug;
 import org.jvnet.hudson.test.HudsonTestCase;
 
 /**
@@ -12,9 +13,7 @@ public void testYank() throws Exception {
         Executor e = c.getExecutors().get(0);
 
         // kill an executor
-        e.killHard();
-        while (e.isAlive())
-            Thread.sleep(10);
+        kill(e);
 
         // make sure it's dead
         assertTrue(c.getExecutors().contains(e));
@@ -27,6 +26,44 @@ public void testYank() throws Exception {
         submit(p.getFormByName("yank"));
 
         assertFalse(c.getExecutors().contains(e));
+        waitUntilExecutorSizeIs(c, 2);
+    }
+
+    @Bug(4756)
+    public void testWhenAnExecuterIsYankedANewExecuterTakesItsPlace() throws Exception {
+        Computer c = hudson.toComputer();
+        Executor e = getExecutorByNumber(c, 0);
+
+        kill(e);
+        e.doYank();
+
+        waitUntilExecutorSizeIs(c, 2);
+
+        assertNotNull(getExecutorByNumber(c, 0));
+        assertNotNull(getExecutorByNumber(c, 1));
+    }
 
+    private void waitUntilExecutorSizeIs(Computer c, int executorCollectionSize) throws InterruptedException {
+        int timeOut = 10;
+        while (c.getExecutors().size() != executorCollectionSize) {
+            Thread.sleep(10);
+            if (timeOut-- == 0) fail("executor collection size was not " + executorCollectionSize);
+        }
     }
+
+    private void kill(Executor e) throws InterruptedException {
+        e.killHard();
+        while (e.isAlive())
+            Thread.sleep(10);
+    }
+
+    private Executor getExecutorByNumber(Computer c, int executorNumber) {
+        for (Executor executor : c.getExecutors()) {
+            if (executor.getNumber() == executorNumber) {
+                return executor;
+            }
+        }
+        return null;
+    }
+
 }