Merge pull request #1645 from stephenc/jenkins-27708

JENKINS-27708, JENKINS-27871 Ensure that identification of blocked tasks is using the live state.
jenkinsci · Apr 15, 2015 · 152d00a · 152d00a
2 parents b688047 + 57efbea
commit 152d00a
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 2 deletions.
diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java
@@ -1335,7 +1335,10 @@ public void maintain() {
             final QueueSorter s = sorter;
             if (s != null)
                 s.sortBuildableItems(buildables);
-
+
+            // Ensure that identification of blocked tasks is using the live state: JENKINS-27708 & JENKINS-27871
+            updateSnapshot();
+
             // allocate buildable jobs to executors
             for (BuildableItem p : new ArrayList<BuildableItem>(
                     buildables)) {// copy as we'll mutate the list in the loop
@@ -1372,6 +1375,18 @@ public void maintain() {
                     makePending(p);
                 else
                     LOGGER.log(Level.FINE, "BuildableItem {0} with empty work units!?", p);
+
+                // Ensure that identification of blocked tasks is using the live state: JENKINS-27708 & JENKINS-27871
+                // The creation of a snapshot itself should be relatively cheap given the expected rate of
+                // job execution. You probably would need 100's of jobs starting execution every iteration
+                // of maintain() before this could even start to become an issue and likely the calculation
+                // of isBuildBlocked(p) will become a bottleneck before updateSnapshot() will. Additionally
+                // since the snapshot itself only ever has at most one reference originating outside of the stack
+                // it should remain in the eden space and thus be cheap to GC.
+                // See https://issues.jenkins-ci.org/browse/JENKINS-27708?focusedCommentId=225819&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-225819
+                // or https://issues.jenkins-ci.org/browse/JENKINS-27708?focusedCommentId=225906&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-225906
+                // for alternative fixes of this issue.
+                updateSnapshot();
             }
         } finally { updateSnapshot(); } } finally {
             lock.unlock();

diff --git a/test/src/test/java/hudson/model/QueueTest.java b/test/src/test/java/hudson/model/QueueTest.java
@@ -50,6 +50,7 @@
 import hudson.slaves.DumbSlave;
 import hudson.slaves.DummyCloudImpl;
 import hudson.slaves.NodeProvisionerRule;
+import hudson.tasks.BuildTrigger;
 import hudson.tasks.Shell;
 import hudson.triggers.SCMTrigger.SCMTriggerCause;
 import hudson.triggers.TimerTrigger.TimerTriggerCause;
@@ -91,6 +92,7 @@
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.MockQueueItemAuthenticator;
 import org.jvnet.hudson.test.SequenceLock;
+import org.jvnet.hudson.test.SleepBuilder;
 import org.jvnet.hudson.test.TestBuilder;
 import org.jvnet.hudson.test.recipes.LocalData;
 import org.mortbay.jetty.Server;
@@ -707,5 +709,38 @@ public void run() {
             fail("Expected an CancellationException to be thrown");
         } catch (CancellationException e) {}
     }
-
+
+    @Issue("JENKINS-27871")
+    @Test public void testBlockBuildWhenUpstreamBuildingLock() throws Exception {
+        final String prefix = "JENKINS-27871";
+        r.getInstance().setNumExecutors(4);
+        r.getInstance().save();
+
+        final FreeStyleProject projectA = r.createFreeStyleProject(prefix+"A");
+        projectA.getBuildersList().add(new SleepBuilder(5000));
+
+        final FreeStyleProject projectB = r.createFreeStyleProject(prefix+"B");
+        projectB.getBuildersList().add(new SleepBuilder(10000));     
+        projectB.setBlockBuildWhenUpstreamBuilding(true);
+
+        final FreeStyleProject projectC = r.createFreeStyleProject(prefix+"C");
+        projectC.getBuildersList().add(new SleepBuilder(10000));
+        projectC.setBlockBuildWhenUpstreamBuilding(true);
+
+        projectA.getPublishersList().add(new BuildTrigger(Arrays.asList(projectB), Result.SUCCESS));
+        projectB.getPublishersList().add(new BuildTrigger(Arrays.asList(projectC), Result.SUCCESS));
+
+        final QueueTaskFuture<FreeStyleBuild> taskA = projectA.scheduleBuild2(0, new TimerTriggerCause());
+        Thread.sleep(1000);
+        final QueueTaskFuture<FreeStyleBuild> taskB = projectB.scheduleBuild2(0, new TimerTriggerCause());
+        final QueueTaskFuture<FreeStyleBuild> taskC = projectC.scheduleBuild2(0, new TimerTriggerCause());
+
+        final FreeStyleBuild buildA = taskA.get(60, TimeUnit.SECONDS);       
+        final FreeStyleBuild buildB = taskB.get(60, TimeUnit.SECONDS);     
+        final FreeStyleBuild buildC = taskC.get(60, TimeUnit.SECONDS);
+        long buildBEndTime = buildB.getStartTimeInMillis() + buildB.getDuration();
+        assertTrue("Project B build should be finished before the build of project C starts. " +
+                "B finished at " + buildBEndTime + ", C started at " + buildC.getStartTimeInMillis(), 
+                buildC.getStartTimeInMillis() >= buildBEndTime);
+    }
 }