[JENKINS-40284] - Fix blocked weights with FQ strategies (#32)

[JENKINS-40284] - Fix blocked weights with FQ strategies
jenkinsci · Jan 16, 2017 · 5e5657d · 5e5657d
2 parents 5d333f6 + 16f901b
commit 5e5657d
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 24 deletions.
diff --git a/src/main/java/jenkins/advancedqueue/sorter/strategy/FQBaseStrategy.java b/src/main/java/jenkins/advancedqueue/sorter/strategy/FQBaseStrategy.java
@@ -32,16 +32,18 @@
 import jenkins.advancedqueue.sorter.SorterStrategyCallback;
 
 /**
+ * Scheduler based on Fair Queuing algorithm.
  * @author Magnus Sandberg
  * @since 2.0
  */
 abstract public class FQBaseStrategy extends MultiBucketStrategy {
-	//
+	// The equivalent of a packet size for a network scheduler.
 	static final protected float MIN_STEP_SIZE = 0.00001F;
 	// Keeps track on the last assigned weight for a given priority
-	static Map<Integer, Float> prio2weight = new HashMap<Integer, Float>();
+	static final protected Map<Integer, Float> prio2weight = new HashMap<Integer, Float>();
+	static final private float MIN_STARTED_WEIGHT = 1F;
 	// Keeps track on the max weight of started jobs
-	transient private float maxStartedWeight = 1F;
+	static protected float maxStartedWeight = MIN_STARTED_WEIGHT;
 
 	public FQBaseStrategy() {
 	}
@@ -72,23 +74,18 @@ protected float getMinimumWeightToAssign(int priority) {
 	}
 
 	protected float getWeightToUse(int priority, float minimumWeightToAssign) {
-		float stepSize = getStepSize(priority);
-		double weight = Math.ceil(minimumWeightToAssign / stepSize) * stepSize;
-		// Cannot be smaller but maybe rounding problems (?)
-		if (weight <= minimumWeightToAssign) {
-			weight += stepSize;
-		}
+		float weight = minimumWeightToAssign * (1F + getStepSize(priority));
 		// Protect us from values going through the roof if we run for a very
 		// long time
 		// This below might leave some jobs in the queue with very large weight
 		// this probably improbable to happen so let's do it like this for now
 		// ...
-		if (Double.POSITIVE_INFINITY == weight) {
-			maxStartedWeight = 1F;
+		if (Float.POSITIVE_INFINITY == weight) {
+			maxStartedWeight = MIN_STARTED_WEIGHT;
 			prio2weight.clear();
-			return getWeightToUse(priority, minimumWeightToAssign);
+			return MIN_STARTED_WEIGHT;
 		}
-		return (float) weight;
+		return weight;
 	}
 
 	abstract float getStepSize(int priority);

diff --git a/src/test/java/jenkins/advancedqueue/sorter/strategy/FQStrategyTest.java b/src/test/java/jenkins/advancedqueue/sorter/strategy/FQStrategyTest.java
@@ -1,8 +1,5 @@
 package jenkins.advancedqueue.sorter.strategy;
 
-import jenkins.advancedqueue.sorter.strategy.FQBaseStrategy;
-import jenkins.advancedqueue.sorter.strategy.FQStrategy;
-
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -19,7 +16,18 @@ public void testStepSize() {
 	@Test
 	public void testGetWeightToUse() {
 		Assert.assertEquals(1.00000F + FQBaseStrategy.MIN_STEP_SIZE, new FQStrategy().getWeightToUse(1, 1.00000F), 0F);
-		Assert.assertEquals(4.56456F + FQBaseStrategy.MIN_STEP_SIZE, new FQStrategy().getWeightToUse(2, 4.56456F), 0F);
+		Assert.assertEquals(1.00001F + FQBaseStrategy.MIN_STEP_SIZE, new FQStrategy().getWeightToUse(1, 1.00001F), 0F);
+		Assert.assertEquals(1F, new FQStrategy().getWeightToUse(1, Float.MAX_VALUE), 0F);
+		assertIncreasingWeight(1F);
+		assertIncreasingWeight(100000F);
 	}
 
+	private void assertIncreasingWeight(float initialWeight) {
+		float previousWeight = initialWeight;
+		for (int i = 0; i < 10; ++i) {
+			float newWeight = new FQStrategy().getWeightToUse(1, previousWeight);
+			Assert.assertTrue(String.format("New weight %s should be greater than previous weight %s", newWeight, previousWeight), newWeight > previousWeight);
+			previousWeight = newWeight;
+		}
+	}
 }
diff --git a/src/test/java/jenkins/advancedqueue/sorter/strategy/WFQStrategyTest.java b/src/test/java/jenkins/advancedqueue/sorter/strategy/WFQStrategyTest.java
@@ -1,8 +1,5 @@
 package jenkins.advancedqueue.sorter.strategy;
 
-import jenkins.advancedqueue.sorter.strategy.FQBaseStrategy;
-import jenkins.advancedqueue.sorter.strategy.WFQStrategy;
-
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -19,10 +16,10 @@ public void testStepSize() {
 
 	@Test
 	public void testGetWeightToUse() {
-		Assert.assertEquals(FQBaseStrategy.MIN_STEP_SIZE * 1 * 456456, new WFQStrategy().getWeightToUse(1, 4.56455F), 0F);
-		Assert.assertEquals(FQBaseStrategy.MIN_STEP_SIZE * 2 * 228228, new WFQStrategy().getWeightToUse(2, 4.56455F), 0F);
-		Assert.assertEquals(FQBaseStrategy.MIN_STEP_SIZE * 3 * 152152, new WFQStrategy().getWeightToUse(3, 4.56455F), 0F);
+		Assert.assertEquals(1.00000F + FQBaseStrategy.MIN_STEP_SIZE, new WFQStrategy().getWeightToUse(1, 1.00000F), 0F);
+		Assert.assertEquals(1.00001F + FQBaseStrategy.MIN_STEP_SIZE, new WFQStrategy().getWeightToUse(1, 1.00001F), 0F);
+		Assert.assertEquals(1.00000F + 2*FQBaseStrategy.MIN_STEP_SIZE, new WFQStrategy().getWeightToUse(2, 1.00000F), 0F);
+		Assert.assertEquals(1.00001F + 2*FQBaseStrategy.MIN_STEP_SIZE, new WFQStrategy().getWeightToUse(2, 1.00001F), 0F);
 	}
-
 
 }