Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-10944] [FIXED JENKINS-24519] If makeBuildable fails on…
… a FlyweightTask, keep it in queue.
  • Loading branch information
jglick committed Jan 5, 2015
1 parent 64cb70e commit 3e344a9
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 16 deletions.
49 changes: 34 additions & 15 deletions core/src/main/java/hudson/model/Queue.java
Expand Up @@ -106,7 +106,6 @@

import jenkins.model.Jenkins;
import jenkins.security.QueueItemAuthenticator;
import jenkins.security.QueueItemAuthenticatorConfiguration;
import jenkins.util.AtmostOneTaskExecutor;
import org.acegisecurity.AccessDeniedException;
import org.acegisecurity.Authentication;
Expand Down Expand Up @@ -1098,8 +1097,11 @@ public synchronized void maintain() {
for (BlockedItem p : new ArrayList<BlockedItem>(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();
}
}
}
}
Expand All @@ -1115,7 +1117,12 @@ public synchronized void maintain() {
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.
Expand Down Expand Up @@ -1164,10 +1171,14 @@ public synchronized void maintain() {
}
}

private void makeBuildable(BuildableItem p) {
if (p.task instanceof FlyweightTask && !isBlockedByShutdown(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<Node,Integer> hashSource = new HashMap<Node, Integer>(h.getNodes().size());

Expand All @@ -1183,19 +1194,27 @@ private void makeBuildable(BuildableItem p) {

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);
}


Expand Down
47 changes: 46 additions & 1 deletion test/src/test/java/hudson/model/QueueTest.java
Expand Up @@ -345,6 +345,49 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
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;
}
}

@Test public void taskEquality() throws Exception {
AtomicInteger cnt = new AtomicInteger();
Expand All @@ -356,7 +399,7 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
r.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;
Expand All @@ -379,11 +422,13 @@ private static final class TestTask extends AbstractQueueTask {
@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();
}
};
Expand Down

0 comments on commit 3e344a9

Please sign in to comment.