diff --git a/core/src/main/java/hudson/model/LoadBalancer.java b/core/src/main/java/hudson/model/LoadBalancer.java index 1aa33195b1609705d996fd50c3c20b84255934c5..19f1eee0105b969691cdf01b2c9d0921c5ed8394 100644 --- a/core/src/main/java/hudson/model/LoadBalancer.java +++ b/core/src/main/java/hudson/model/LoadBalancer.java @@ -140,7 +140,7 @@ public abstract class LoadBalancer implements ExtensionPoint { return new LoadBalancer() { @Override public Mapping map(Task task, MappingWorksheet worksheet) { - if (Queue.ifBlockedByHudsonShutdown(task)) { + if (Queue.isBlockedByShutdown(task)) { // if we are quieting down, don't start anything new so that // all executors will be eventually free. return null; diff --git a/core/src/main/java/hudson/model/Queue.java b/core/src/main/java/hudson/model/Queue.java index 65058e65f339a6dc929e5e38ff43f4b9d1c89202..a7f25bde273e197f1e0db27dd8b391fc816b2cad 100644 --- a/core/src/main/java/hudson/model/Queue.java +++ b/core/src/main/java/hudson/model/Queue.java @@ -106,7 +106,6 @@ import javax.servlet.ServletException; import jenkins.model.Jenkins; import jenkins.security.QueueItemAuthenticator; -import jenkins.security.QueueItemAuthenticatorConfiguration; import jenkins.util.AtmostOneTaskExecutor; import org.acegisecurity.AccessDeniedException; import org.acegisecurity.Authentication; @@ -1098,8 +1097,11 @@ public class Queue extends ResourceController implements Saveable { for (BlockedItem p : new ArrayList(blockedProjects.values())) {// copy as we'll mutate the list if (!isBuildBlocked(p) && allowNewBuildableTask(p.task)) { // ready to be executed - p.leave(this); - makeBuildable(new BuildableItem(p)); + Runnable r = makeBuildable(new BuildableItem(p)); + if (r != null) { + p.leave(this); + r.run(); + } } } } @@ -1115,7 +1117,12 @@ public class Queue extends ResourceController implements Saveable { Task p = top.task; if (!isBuildBlocked(top) && allowNewBuildableTask(p)) { // ready to be executed immediately - makeBuildable(new BuildableItem(top)); + Runnable r = makeBuildable(new BuildableItem(top)); + if (r != null) { + r.run(); + } else { + new BlockedItem(top).enter(this); + } } else { // this can't be built now because another build is in progress // set this project aside. @@ -1164,10 +1171,14 @@ public class Queue extends ResourceController implements Saveable { } } - private void makeBuildable(BuildableItem p) { - if(Jenkins.FLYWEIGHT_SUPPORT && p.task instanceof FlyweightTask && !ifBlockedByHudsonShutdown(p.task)) { - - + /** + * Tries to make an item ready to build. + * @param p a proposed buildable item + * @return a thunk to actually prepare it (after leaving an earlier list), or null if it cannot be run now + */ + private @CheckForNull Runnable makeBuildable(final BuildableItem p) { + if (p.task instanceof FlyweightTask) { + if (!isBlockedByShutdown(p.task)) { Jenkins h = Jenkins.getInstance(); Map hashSource = new HashMap(h.getNodes().size()); @@ -1183,19 +1194,27 @@ public class Queue extends ResourceController implements Saveable { Label lbl = p.getAssignedLabel(); for (Node n : hash.list(p.task.getFullDisplayName())) { - Computer c = n.toComputer(); + final Computer c = n.toComputer(); if (c==null || c.isOffline()) continue; if (lbl!=null && !lbl.contains(n)) continue; if (n.canTake(p) != null) continue; - c.startFlyWeightTask(new WorkUnitContext(p).createWorkUnit(p.task)); - makePending(p); - return; + return new Runnable() { + @Override public void run() { + c.startFlyWeightTask(new WorkUnitContext(p).createWorkUnit(p.task)); + makePending(p); + } + }; + } } // if the execution get here, it means we couldn't schedule it anywhere. - // so do the scheduling like other normal jobs. + return null; + } else { // regular heavyweight task + return new Runnable() { + @Override public void run() { + p.enter(Queue.this); + } + }; } - - p.enter(this); } @@ -1211,7 +1230,18 @@ public class Queue extends ResourceController implements Saveable { return pendings.add(p); } + /** @deprecated Use {@link #isBlockedByShutdown} instead. */ public static boolean ifBlockedByHudsonShutdown(Task task) { + return isBlockedByShutdown(task); + } + + /** + * Checks whether a task should not be scheduled because {@link Jenkins#isQuietingDown()}. + * @param task some queue task + * @return true if {@link Jenkins#isQuietingDown()} unless this is a {@link NonBlockingTask} + * @since TODO + */ + public static boolean isBlockedByShutdown(Task task) { return Jenkins.getInstance().isQuietingDown() && !(task instanceof NonBlockingTask); } @@ -1235,7 +1265,7 @@ public class Queue extends ResourceController implements Saveable { /** * Marks {@link Task}s that are not affected by the {@linkplain Jenkins#isQuietingDown()} quieting down}, * because these tasks keep other tasks executing. - * + * @see #isBlockedByShutdown * @since 1.336 */ public interface NonBlockingTask extends Task {} @@ -1926,7 +1956,7 @@ public class Queue extends ResourceController implements Saveable { public CauseOfBlockage getCauseOfBlockage() { Jenkins jenkins = Jenkins.getInstance(); - if(ifBlockedByHudsonShutdown(task)) + if(isBlockedByShutdown(task)) return CauseOfBlockage.fromMessage(Messages._Queue_HudsonIsAboutToShutDown()); Label label = getAssignedLabel(); diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index f2862872f2f777d4980ff6c1f642ea8a8d5e05a4..16e0b42beba24a44c6441102b164a4af088699c2 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -4179,9 +4179,9 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve public static boolean PARALLEL_LOAD = Configuration.getBooleanConfigParameter("parallelLoad", true); public static boolean KILL_AFTER_LOAD = Configuration.getBooleanConfigParameter("killAfterLoad", false); /** - * Enabled by default as of 1.337. Will keep it for a while just in case we have some serious problems. + * @deprecated No longer used. */ - public static boolean FLYWEIGHT_SUPPORT = Configuration.getBooleanConfigParameter("flyweightSupport", true); + public static boolean FLYWEIGHT_SUPPORT = true; /** * Tentative switch to activate the concurrent build behavior. diff --git a/test/src/test/java/hudson/model/QueueTest.java b/test/src/test/java/hudson/model/QueueTest.java index 93bcb5ba052c1dd73e3144030844f86c2d601dd3..37d9fa82f9cf5aad1a830e6e9df2da6f922a7ca0 100644 --- a/test/src/test/java/hudson/model/QueueTest.java +++ b/test/src/test/java/hudson/model/QueueTest.java @@ -353,6 +353,49 @@ public class QueueTest extends HudsonTestCase { assertEquals(1, runs.size()); assertEquals("slave0", runs.get(0).getBuiltOnStr()); } + + @Issue("JENKINS-10944") + @Test public void flyweightTasksBlockedByShutdown() throws Exception { + r.jenkins.doQuietDown(true, 0); + AtomicInteger cnt = new AtomicInteger(); + TestFlyweightTask task = new TestFlyweightTask(cnt, null); + assertTrue(Queue.isBlockedByShutdown(task)); + r.jenkins.getQueue().schedule2(task, 0); + r.jenkins.getQueue().maintain(); + r.jenkins.doCancelQuietDown(); + assertFalse(Queue.isBlockedByShutdown(task)); + r.waitUntilNoActivity(); + assertEquals(1, cnt.get()); + assert task.exec instanceof OneOffExecutor : task.exec; + } + + @Issue("JENKINS-24519") + @Test public void flyweightTasksBlockedBySlave() throws Exception { + Label label = Label.get("myslave"); + AtomicInteger cnt = new AtomicInteger(); + TestFlyweightTask task = new TestFlyweightTask(cnt, label); + r.jenkins.getQueue().schedule2(task, 0); + r.jenkins.getQueue().maintain(); + r.createSlave(label); + r.waitUntilNoActivity(); + assertEquals(1, cnt.get()); + assert task.exec instanceof OneOffExecutor : task.exec; + } + + private static class TestFlyweightTask extends TestTask implements Queue.FlyweightTask { + Executor exec; + private final Label assignedLabel; + TestFlyweightTask(AtomicInteger cnt, Label assignedLabel) { + super(cnt); + this.assignedLabel = assignedLabel; + } + @Override protected void doRun() { + exec = Executor.currentExecutor(); + } + @Override public Label getAssignedLabel() { + return assignedLabel; + } + } public void testTaskEquality() throws Exception { AtomicInteger cnt = new AtomicInteger(); @@ -364,7 +407,7 @@ public class QueueTest extends HudsonTestCase { waitUntilNoActivity(); assertEquals(1, cnt.get()); } - private static final class TestTask extends AbstractQueueTask { + private static class TestTask extends AbstractQueueTask { private final AtomicInteger cnt; TestTask(AtomicInteger cnt) { this.cnt = cnt; @@ -387,11 +430,13 @@ public class QueueTest extends HudsonTestCase { @Override public Node getLastBuiltOn() {return null;} @Override public long getEstimatedDuration() {return -1;} @Override public ResourceList getResourceList() {return new ResourceList();} + protected void doRun() {} @Override public Executable createExecutable() throws IOException { return new Executable() { @Override public SubTask getParent() {return TestTask.this;} @Override public long getEstimatedDuration() {return -1;} @Override public void run() { + doRun(); cnt.incrementAndGet(); } };