Skip to content

Commit

Permalink
[FIXED JENKINS-27708][FIXED JENKINS-27871] Ensure that identification…
Browse files Browse the repository at this point in the history
… of blocked tasks is using the live state.

- The creation of a snapshot itself should be relatively cheap given the expected rate of
  job execution. You probably would need 100's of jobs starting execution every iteration
  of maintain() before this could even start to become an issue and likely the calculation
  of isBuildBlocked(p) will become a bottleneck before updateSnapshot() will. Additionally
  since the snapshot itself only ever has at most one reference originating outside of the stack
  it should remain in the eden space and thus be cheap to GC.

- JENKINS-27708 comments 225819 and 225906 provide more complex but logically equivalent fixes of
  this issue. I am favouring this approach as it is simpler and provides less scope for error as any
  new helper methods can just rely on the snapshot being up to date whereas with the other
  two candidates if a new helper method is introduced there is the potential to miss adding support
  for the live view. The comment 225819 has the risk of introducing extra lock contention while
  the comment 225906 version forces every access to the helper methods to pass a second memory
  barrier

(cherry picked from commit 5880ed8)
  • Loading branch information
stephenc authored and olivergondza committed May 8, 2015
1 parent 1fed368 commit 11b89d9
Showing 1 changed file with 16 additions and 1 deletion.
17 changes: 16 additions & 1 deletion core/src/main/java/hudson/model/Queue.java
Expand Up @@ -1335,7 +1335,10 @@ public void maintain() {
final QueueSorter s = sorter;
if (s != null)
s.sortBuildableItems(buildables);


// Ensure that identification of blocked tasks is using the live state: JENKINS-27708 & JENKINS-27871
updateSnapshot();

// allocate buildable jobs to executors
for (BuildableItem p : new ArrayList<BuildableItem>(
buildables)) {// copy as we'll mutate the list in the loop
Expand Down Expand Up @@ -1372,6 +1375,18 @@ public void maintain() {
makePending(p);
else
LOGGER.log(Level.FINE, "BuildableItem {0} with empty work units!?", p);

// Ensure that identification of blocked tasks is using the live state: JENKINS-27708 & JENKINS-27871
// The creation of a snapshot itself should be relatively cheap given the expected rate of
// job execution. You probably would need 100's of jobs starting execution every iteration
// of maintain() before this could even start to become an issue and likely the calculation
// of isBuildBlocked(p) will become a bottleneck before updateSnapshot() will. Additionally
// since the snapshot itself only ever has at most one reference originating outside of the stack
// it should remain in the eden space and thus be cheap to GC.
// See https://issues.jenkins-ci.org/browse/JENKINS-27708?focusedCommentId=225819&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-225819
// or https://issues.jenkins-ci.org/browse/JENKINS-27708?focusedCommentId=225906&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-225906
// for alternative fixes of this issue.
updateSnapshot();
}
} finally { updateSnapshot(); } } finally {
lock.unlock();
Expand Down

0 comments on commit 11b89d9

Please sign in to comment.