Skip to content
Permalink
Browse files

Merge pull request #1743 from stephenc/jenkins-28926

[FIXED JENKINS-28926] Block while upstream/downstream building cycles never complete
  • Loading branch information
stephenc committed Jun 17, 2015
2 parents 482bffa + c44c088 commit 7929412037ff75f60791cfb23631521f8726c23d
@@ -1408,15 +1408,29 @@ public void maintain() {
}
}

final QueueSorter s = sorter;

{// blocked -> buildable
for (BlockedItem p : new ArrayList<BlockedItem>(blockedProjects.values())) {// copy as we'll mutate the list
// copy as we'll mutate the list and we want to process in a potentially different order
// TODO replace with <> operator after backporting to 1.609.3
List<BlockedItem> blockedItems = new ArrayList<BlockedItem>(blockedProjects.values());
// if facing a cycle of blocked tasks, ensure we process in the desired sort order
if (s != null) {
s.sortBlockedItems(blockedItems);
} else {
Collections.sort(blockedItems, QueueSorter.DEFAULT_BLOCKED_ITEM_COMPARATOR);
}
for (BlockedItem p : blockedItems) {
if (!isBuildBlocked(p) && allowNewBuildableTask(p.task)) {
// ready to be executed
Runnable r = makeBuildable(new BuildableItem(p));
if (r != null) {
p.leave(this);
r.run();
// JENKINS-28926 we have removed a task from the blocked projects and added to building
// thus we should update the snapshot so that subsequent blocked projects can correctly
// determine if they are blocked by the lucky winner
updateSnapshot();
}
}
}
@@ -1446,7 +1460,6 @@ public void maintain() {
}
}

final QueueSorter s = sorter;
if (s != null)
s.sortBuildableItems(buildables);

@@ -1461,6 +1474,9 @@ public void maintain() {
p.leave(this);
new BlockedItem(p).enter(this);
LOGGER.log(Level.FINE, "Catching that {0} is blocked in the last minute", p);
// JENKINS-28926 we have moved an unblocked task into the blocked state, update snapshot
// so that other buildables which might have been blocked by this can see the state change
updateSnapshot();
continue;
}

@@ -8,6 +8,8 @@
import hudson.model.Queue;
import hudson.model.Queue.BuildableItem;

import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.logging.Logger;

@@ -19,6 +21,20 @@
* @since 1.343
*/
public abstract class QueueSorter implements ExtensionPoint {
/**
* A comparator that sorts {@link Queue.BlockedItem} instances based on how long they have been in the queue.
* (We want the time since in queue by default as blocking on upstream/downstream considers waiting items
* also and thus the blocking starts once the task is in the queue not once the task is buildable)
*
* @since 1.FIXME
*/
public static final Comparator<Queue.BlockedItem> DEFAULT_BLOCKED_ITEM_COMPARATOR = new Comparator<Queue.BlockedItem>() {
@Override
public int compare(Queue.BlockedItem o1, Queue.BlockedItem o2) {
return Long.compare(o1.getInQueueSince(), o2.getInQueueSince());
}
};

/**
* Sorts the buildable items list. The items at the beginning will be executed
* before the items at the end of the list.
@@ -28,6 +44,18 @@
*/
public abstract void sortBuildableItems(List<BuildableItem> buildables);

/**
* Sorts the blocked items list. The items at the beginning will be considered for removal from the blocked state
* before the items at the end of the list.
*
* @param blockedItems
* List of blocked items in the queue. Never null.
* @since 1.FIXME
*/
public void sortBlockedItems(List<Queue.BlockedItem> blockedItems) {
Collections.sort(blockedItems, DEFAULT_BLOCKED_ITEM_COMPARATOR);
}

/**
* All registered {@link QueueSorter}s. Only the first one will be picked up,
* unless explicitly overridden by {@link Queue#setSorter(QueueSorter)}.
@@ -75,6 +75,7 @@
import javax.servlet.http.HttpServletResponse;
import jenkins.model.Jenkins;
import jenkins.security.QueueItemAuthenticatorConfiguration;
import jenkins.triggers.ReverseBuildTrigger;
import org.acegisecurity.Authentication;
import org.acegisecurity.GrantedAuthority;
import org.acegisecurity.acls.sid.PrincipalSid;
@@ -83,6 +84,9 @@
import org.apache.commons.fileupload.disk.DiskFileItemFactory;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.apache.commons.io.FileUtils;

import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.*;

import org.junit.Assert;
@@ -442,6 +446,38 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
String tagName = queueItem.getDocumentElement().getTagName();
assertTrue(tagName.equals("blockedItem") || tagName.equals("buildableItem"));
}

@Issue("JENKINS-28926")
@Test
public void upstreamDownstreamCycle() throws Exception {
FreeStyleProject trigger = r.createFreeStyleProject();
FreeStyleProject chain1 = r.createFreeStyleProject();
FreeStyleProject chain2a = r.createFreeStyleProject();
FreeStyleProject chain2b = r.createFreeStyleProject();
FreeStyleProject chain3 = r.createFreeStyleProject();
trigger.getPublishersList().add(new BuildTrigger(String.format("%s, %s, %s, %s", chain1.getName(), chain2a.getName(), chain2b.getName(), chain3.getName()), true));
trigger.setQuietPeriod(0);
chain1.setQuietPeriod(1);
chain2a.setQuietPeriod(1);
chain2b.setQuietPeriod(1);
chain3.setQuietPeriod(1);
chain1.getPublishersList().add(new BuildTrigger(String.format("%s, %s", chain2a.getName(), chain2b.getName()), true));
chain2a.getPublishersList().add(new BuildTrigger(chain3.getName(), true));
chain2b.getPublishersList().add(new BuildTrigger(chain3.getName(), true));
chain1.setBlockBuildWhenDownstreamBuilding(true);
chain2a.setBlockBuildWhenDownstreamBuilding(true);
chain2b.setBlockBuildWhenDownstreamBuilding(true);
chain3.setBlockBuildWhenUpstreamBuilding(true);
r.jenkins.rebuildDependencyGraph();
r.buildAndAssertSuccess(trigger);
// the trigger should build immediately and schedule the cycle
r.waitUntilNoActivity();
final Queue queue = r.getInstance().getQueue();
assertThat("The cycle should have been defanged and chain1 executed", queue.getItem(chain1), nullValue());
assertThat("The cycle should have been defanged and chain2a executed", queue.getItem(chain2a), nullValue());
assertThat("The cycle should have been defanged and chain2b executed", queue.getItem(chain2b), nullValue());
assertThat("The cycle should have been defanged and chain3 executed", queue.getItem(chain3), nullValue());
}

private static class TestFlyweightTask extends TestTask implements Queue.FlyweightTask {
Executor exec;

0 comments on commit 7929412

Please sign in to comment.
You can’t perform that action at this time.