Skip to content
Permalink
Browse files

Merge pull request #1709 from oleg-nenashev/JENKINS-28446-fix

[FIXED JENKINS-28446] - proper calculation of queue length in UnlabeledLoadStatistics
  • Loading branch information...
oleg-nenashev committed May 21, 2015
2 parents 048ea9c + 7697bdb commit 7205da0205a67324919cb9aae024cdd45a00792b
@@ -124,6 +124,7 @@
import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.converters.basic.AbstractSingleValueConverter;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnegative;
import jenkins.model.queue.AsynchronousExecution;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.interceptor.RequirePOST;
@@ -942,8 +943,12 @@ public boolean isPending(Task t) {

/**
* How many {@link BuildableItem}s are assigned for the given label?
* @param l Label to be checked. If null, any label will be accepted.
* If you want to count {@link BuildableItem}s without assigned labels,
* use {@link #strictCountBuildableItemsFor(hudson.model.Label)}.
* @return Number of {@link BuildableItem}s for the specified label.
*/
public int countBuildableItemsFor(Label l) {
public @Nonnegative int countBuildableItemsFor(@CheckForNull Label l) {
Snapshot snapshot = this.snapshot;
int r = 0;
for (BuildableItem bi : snapshot.buildables)
@@ -956,6 +961,30 @@ public int countBuildableItemsFor(Label l) {
r++;
return r;
}

/**
* How many {@link BuildableItem}s are assigned for the given label?
* <p/>
* The implementation is quite similar to {@link #countBuildableItemsFor(hudson.model.Label)},
* but it has another behavior for null parameters.
* @param l Label to be checked. If null, only jobs without assigned labels
* will be taken into the account.
* @return Number of {@link BuildableItem}s for the specified label.
* @since TODO
*/
public @Nonnegative int strictCountBuildableItemsFor(@CheckForNull Label l) {
Snapshot _snapshot = this.snapshot;
int r = 0;
for (BuildableItem bi : _snapshot.buildables)
for (SubTask st : bi.task.getSubTasks())
if (bi.getAssignedLabelFor(st) == l)
r++;
for (BuildableItem bi : _snapshot.pendings)
for (SubTask st : bi.task.getSubTasks())
if (bi.getAssignedLabelFor(st) == l)
r++;
return r;
}

/**
* Counts all the {@link BuildableItem}s currently in the queue.
@@ -1808,13 +1837,16 @@ public Label getAssignedLabel() {
}

/**
* Test if the specified {@link SubTask} needs to be run on a node with a particular label, and
* return that {@link Label}. Otherwise null, indicating it can run on anywhere.
*
* Test if the specified {@link SubTask} needs to be run on a node with a particular label.
* <p>
* This code takes {@link LabelAssignmentAction} into account, then falls back to {@link SubTask#getAssignedLabel()}
* This method takes {@link LabelAssignmentAction} into account, the first
* non-null assignment will be returned.
* Otherwise falls back to {@link SubTask#getAssignedLabel()}
* @param st {@link SubTask} to be checked.
* @return Required {@link Label}. Otherwise null, indicating it can run on anywhere.
*/
public Label getAssignedLabelFor(SubTask st) {
public @CheckForNull Label getAssignedLabelFor(@Nonnull SubTask st) {
for (LabelAssignmentAction laa : getActions(LabelAssignmentAction.class)) {
Label l = laa.getAssignedLabel(st);
if (l!=null) return l;
@@ -7,6 +7,7 @@
import hudson.model.Queue.QueueDecisionHandler;
import hudson.model.Queue.Task;
import hudson.model.queue.SubTask;
import javax.annotation.Nonnull;

/**
* {@link Action} that can be submitted to {@link Queue} that controls where
@@ -35,5 +36,5 @@
* null to let other {@link LabelAssignmentAction}s take control, eventually to {@code SubTask#getAssignedLabel()}.
* If non-null value is returned, that label will be authoritative.
*/
Label getAssignedLabel(SubTask task);
Label getAssignedLabel(@Nonnull SubTask task);
}
@@ -31,9 +31,11 @@
import hudson.model.Queue;
import hudson.model.Queue.Task;
import hudson.model.queue.SubTask;
import hudson.model.queue.Tasks;
import hudson.util.Iterators;

import java.util.Iterator;
import java.util.List;

/**
* {@link LoadStatistics} that track the "free roam" jobs (whose {@link Task#getAssignedLabel()} is null)
@@ -78,7 +80,11 @@ public int computeTotalExecutors() {

@Override
public int computeQueueLength() {
return Jenkins.getInstance().getQueue().countBuildableItemsFor(null);
final Jenkins j = Jenkins.getInstance();
if (j == null) { // Consider queue as empty when Jenkins is inactive
return 0;
}
return j.getQueue().strictCountBuildableItemsFor(null);
}

@Override
@@ -0,0 +1,96 @@
/*
* The MIT License
*
* Copyright 2015 CloudBees Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package jenkins.model;

import hudson.model.FreeStyleProject;
import hudson.model.LoadStatistics;
import hudson.model.Node;
import hudson.model.ParametersAction;
import hudson.model.Queue;
import hudson.model.StringParameterValue;
import hudson.model.labels.LabelAtom;
import hudson.slaves.DumbSlave;
import org.junit.After;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import static org.junit.Assert.*;

/**
* Tests for {@link UnlabeledLoadStatistics} class.
* @author Oleg Nenashev
*/
public class UnlabeledLoadStatisticsTest {

@Rule
public JenkinsRule j = new JenkinsRule();

private final LoadStatistics unlabeledLoad = new UnlabeledLoadStatistics();

@After
public void clearQueue() {
j.getInstance().getQueue().clear();
}

@Test
@Issue("JENKINS-28446")
public void computeQueueLength() throws Exception {
final Queue queue = j.jenkins.getQueue();
assertEquals("Queue must be empty when the test starts", 0, queue.getBuildableItems().size());
assertEquals("Statistics must return 0 when the test starts", 0, unlabeledLoad.computeQueueLength());

// Disable builds by default, create a slave to prevent assigning of "master" labels
j.jenkins.setNumExecutors(0);
DumbSlave slave = j.createOnlineSlave(new LabelAtom("testLabel"));
slave.setMode(Node.Mode.EXCLUSIVE);

// Init project
FreeStyleProject unlabeledProject = j.createFreeStyleProject("UnlabeledProject");
unlabeledProject.setConcurrentBuild(true);
FreeStyleProject labeledProject = j.createFreeStyleProject("LabeledProject");
labeledProject.setAssignedLabel(new LabelAtom("foo"));

// Put unlabeled build into the queue
unlabeledProject.scheduleBuild2(0, new ParametersAction(new StringParameterValue("FOO", "BAR1")));
queue.maintain();
assertEquals("Unlabeled build must be taken into account", 1, unlabeledLoad.computeQueueLength());
unlabeledProject.scheduleBuild2(0, new ParametersAction(new StringParameterValue("FOO", "BAR2")));
queue.maintain();
assertEquals("Second Unlabeled build must be taken into account", 2, unlabeledLoad.computeQueueLength());

// Put labeled build into the queue
labeledProject.scheduleBuild2(0);
queue.maintain();
assertEquals("Labeled builds must be ignored", 2, unlabeledLoad.computeQueueLength());

// Allow executions of unlabeled builds on master, all unlabeled builds should pass
j.jenkins.setNumExecutors(1);
j.buildAndAssertSuccess(unlabeledProject);
queue.maintain();
assertEquals("Queue must contain the labeled project build", 1, queue.getBuildableItems().size());
assertEquals("Statistics must return 0 after all builds", 0, unlabeledLoad.computeQueueLength());
}

}

0 comments on commit 7205da0

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