Skip to content
Permalink
Browse files

[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 Jun 17, 2015
1 parent c3dcaeb commit 4f4a64a522ec7bf31f24280827757214e6985f3d
Showing with 46 additions and 2 deletions.
  1. +18 −2 core/src/main/java/hudson/model/Queue.java
  2. +28 −0 core/src/main/java/hudson/model/queue/QueueSorter.java
@@ -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();
}
}
}
@@ -1441,7 +1455,6 @@ public void maintain() {
}
}

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

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

@@ -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)}.

0 comments on commit 4f4a64a

Please sign in to comment.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.