Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-28926] Block while upstream/downstream building cycles…
… never complete

- One could argue that without this change the system is functioning correctly and that previous behaviour
  was a bug. On the other hand, people have come to rely on the previous behaviour.
- The issue really centeres around state changes in the blocked tasks. Since blocking on upstream/downstream
  relies on checking the building projects and the queued (excluding blocked) tasks we need any change in
  the blocked task list to be visible immediately (i.e. update the snapshot)
- I was able to reliably reproduce this behaviour with a convoluted set of manually configured projects
  but turning this into a test case has not proved quite as easy. Manual testing confirms that the issue is
  fixed for my manual test case
- I have also added a sorting of the blocked list when probing for tasks to unblock. This should prioritise
  tasks as intended by the QueueSorter

(cherry picked from commit de87736)
  • Loading branch information
stephenc authored and olivergondza committed Jul 7, 2015
1 parent c3dcaeb commit 4f4a64a
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 2 deletions.
20 changes: 18 additions & 2 deletions core/src/main/java/hudson/model/Queue.java
Expand Up @@ -1403,15 +1403,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();
}
}
}
Expand Down Expand Up @@ -1441,7 +1455,6 @@ public void maintain() {
}
}

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

Expand All @@ -1456,6 +1469,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;
}

Expand Down
28 changes: 28 additions & 0 deletions core/src/main/java/hudson/model/queue/QueueSorter.java
Expand Up @@ -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;

Expand All @@ -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.
Expand All @@ -28,6 +44,18 @@ public abstract class QueueSorter implements ExtensionPoint {
*/
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)}.
Expand Down

0 comments on commit 4f4a64a

Please sign in to comment.