Merge branch 'wip-loadstats-refactor'

core/src/main/java/hudson/model/Label.java

@@ -39,6 +39,7 @@ import hudson.model.labels.LabelExpressionLexer;
 import hudson.model.labels.LabelExpressionParser;
 import hudson.model.labels.LabelOperatorPrecedence;
 import hudson.model.labels.LabelVisitor;
+import hudson.model.queue.SubTask;
 import hudson.security.ACL;
 import hudson.slaves.NodeProvisioner;
 import hudson.slaves.Cloud;
@@ -107,6 +108,16 @@ public abstract class Label extends Actionable implements Comparable<Label>, Mod
             public int computeQueueLength() {
                 return Jenkins.getInstance().getQueue().countBuildableItemsFor(Label.this);
             }
+
+            @Override
+            protected Set<Node> getNodes() {
+                return Label.this.getNodes();
+            }
+
+            @Override
+            protected boolean matches(SubTask item) {
+                return item.getAssignedLabel() == Label.this;
+            }
         };
         this.nodeProvisioner = new NodeProvisioner(this, loadStatistics);
     }

core/src/main/java/hudson/model/OverallLoadStatistics.java

@@ -25,6 +25,7 @@ package hudson.model;
 
 import hudson.model.MultiStageTimeSeries.TimeScale;
 import hudson.model.MultiStageTimeSeries.TrendChart;
+import hudson.model.queue.SubTask;
 import jenkins.model.Jenkins;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
@@ -68,11 +69,21 @@ public class OverallLoadStatistics extends LoadStatistics {
         return Jenkins.getInstance().getQueue().countBuildableItems();
     }
 
+    @Override
+    protected Iterable<Node> getNodes() {
+        return Jenkins.getActiveInstance().getNodes();
+    }
+
+    @Override
+    protected boolean matches(SubTask item) {
+        return true;
+    }
+
     /**
      * When drawing the overall load statistics, use the total queue length,
      * not {@link #queueLength}, which just shows jobs that are to be run on the master.
      */
     protected TrendChart createOverallTrendChart(TimeScale timeScale) {
-        return MultiStageTimeSeries.createTrendChart(timeScale,busyExecutors,totalExecutors,queueLength);
+        return MultiStageTimeSeries.createTrendChart(timeScale,busyExecutors,onlineExecutors,queueLength,availableExecutors);
     }
 }

core/src/main/java/hudson/slaves/NodeProvisioner.java

@@ -258,19 +258,18 @@ public class NodeProvisioner {
                     float plannedCapacity = plannedCapacitySnapshot;
                     plannedCapacitiesEMA.update(plannedCapacity);
 
-                    int idleSnapshot = stat.computeIdleExecutors();
-                    int queueLengthSnapshot = stat.computeQueueLength();
+                    final LoadStatistics.LoadStatisticsSnapshot snapshot = stat.computeSnapshot();
 
-                    if (queueLengthSnapshot <= idleSnapshot) {
+                    int availableSnapshot = snapshot.getAvailableExecutors();
+                    int queueLengthSnapshot = snapshot.getQueueLength();
+
+                    if (queueLengthSnapshot <= availableSnapshot) {
                         LOGGER.log(Level.FINE,
-                                "Queue length {0} is less than the idle capacity {1}. No provisioning strategy "
-                                        + "required",
-                                new Object[]{queueLengthSnapshot, idleSnapshot});
+                                "Queue length {0} is less than the available capacity {1}. No provisioning strategy required",
+                                new Object[]{queueLengthSnapshot, availableSnapshot});
                         provisioningState = null;
                     } else {
-                        provisioningState = new StrategyState(queueLengthSnapshot, label, idleSnapshot,
-                                stat.computeTotalExecutors(),
-                                plannedCapacitySnapshot);
+                        provisioningState = new StrategyState(snapshot, label, plannedCapacitySnapshot);;
                     }
                 }
             });
@@ -355,13 +354,9 @@ public class NodeProvisioner {
          */
         private final int plannedCapacitySnapshot;
         /**
-         * The number of idle executors for this {@link #label}
-         */
-        private final int idleSnapshot;
-        /**
-         * The total number of executors for this {@link #label}
+         * The current statistics snapshot for this {@link #label}.
          */
-        private final int totalSnapshot;
+        private final LoadStatistics.LoadStatisticsSnapshot snapshot;
         private final List<PlannedNode> pendingLaunches;
         /**
          * The additional planned capacity for this {@link #label} and provisioned by previous strategies during the
@@ -372,18 +367,12 @@ public class NodeProvisioner {
 
         /**
          * Should only be instantiated by {@link NodeProvisioner#update()}
-         * @param queueLengthSnapshot the queue length.
          * @param label the label.
-         * @param idleSnapshot the idle executor count.
-         * @param totalSnapshot the totoal executor count.
          * @param plannedCapacitySnapshot the planned executor count.
          */
-        private StrategyState(int queueLengthSnapshot, Label label, int idleSnapshot, int totalSnapshot,
-                              int plannedCapacitySnapshot) {
-            this.queueLengthSnapshot = queueLengthSnapshot;
+        private StrategyState(LoadStatistics.LoadStatisticsSnapshot snapshot, Label label, int plannedCapacitySnapshot) {
+            this.snapshot = snapshot;
             this.label = label;
-            this.idleSnapshot = idleSnapshot;
-            this.totalSnapshot = totalSnapshot;
             this.plannedCapacitySnapshot = plannedCapacitySnapshot;
             pendingLaunches = NodeProvisioner.this.pendingLaunches;
         }
@@ -395,11 +384,20 @@ public class NodeProvisioner {
             return label;
         }
 
+        /**
+         * The current snapshot of the load statistics for this {@link #getLabel()}.
+         * @since 1.FIXME
+         */
+        public LoadStatistics.LoadStatisticsSnapshot getSnapshot() {
+            return snapshot;
+        }
+
         /**
          * The number of items in the queue requiring this {@link #getLabel()}.
+         * @deprecated use {@link #getSnapshot()}, {@link LoadStatistics.LoadStatisticsSnapshot#getQueueLength()}
          */
         public int getQueueLengthSnapshot() {
-            return queueLengthSnapshot;
+            return snapshot.getQueueLength();
         }
 
         /**
@@ -411,16 +409,18 @@ public class NodeProvisioner {
 
         /**
          * The number of idle executors for this {@link #getLabel()}
+         * @deprecated use {@link #getSnapshot()}, {@link LoadStatistics.LoadStatisticsSnapshot#getAvailableExecutors()}
          */
         public int getIdleSnapshot() {
-            return idleSnapshot;
+            return snapshot.getAvailableExecutors();
         }
 
         /**
          * The total number of executors for this {@link #getLabel()}
+         * @deprecated use {@link #getSnapshot()}, {@link LoadStatistics.LoadStatisticsSnapshot#getOnlineExecutors()}
          */
         public int getTotalSnapshot() {
-            return totalSnapshot;
+            return snapshot.getOnlineExecutors();
         }
 
         /**
@@ -452,16 +452,66 @@ public class NodeProvisioner {
 
         /**
          * The time series average number of idle executors for this {@link #getLabel()}
+         * @deprecated use {@link #getAvailableExecutorsLatest()}
          */
         public float getIdleLatest() {
-            return stat.getLatestIdleExecutors(TIME_SCALE);
+            return getAvailableExecutorsLatest();
         }
 
         /**
          * The time series average total number of executors for this {@link #getLabel()}
+         * @deprecated use {@link #getOnlineExecutorsLatest()}
          */
         public float getTotalLatest() {
-            return stat.totalExecutors.getLatest(TIME_SCALE);
+            return getOnlineExecutorsLatest();
+        }
+
+        /**
+         * The time series average number of defined executors for this {@link #getLabel()}
+         * @since 1.FIXME
+         */
+        public float getDefinedExecutorsLatest() {
+            return stat.definedExecutors.getLatest(TIME_SCALE);
+        }
+
+        /**
+         * The time series average number of online executors for this {@link #getLabel()}
+         * @since 1.FIXME
+         */
+        public float getOnlineExecutorsLatest() {
+            return stat.onlineExecutors.getLatest(TIME_SCALE);
+        }
+
+        /**
+         * The time series average number of connecting executors for this {@link #getLabel()}
+         * @since 1.FIXME
+         */
+        public float getConnectingExecutorsLatest() {
+            return stat.connectingExecutors.getLatest(TIME_SCALE);
+        }
+
+        /**
+         * The time series average number of busy executors for this {@link #getLabel()}
+         * @since 1.FIXME
+         */
+        public float getBusyExecutorsLatest() {
+            return stat.busyExecutors.getLatest(TIME_SCALE);
+        }
+
+        /**
+         * The time series average number of idle executors for this {@link #getLabel()}
+         * @since 1.FIXME
+         */
+        public float getIdleExecutorsLatest() {
+            return stat.idleExecutors.getLatest(TIME_SCALE);
+        }
+
+        /**
+         * The time series average number of available executors for this {@link #getLabel()}
+         * @since 1.FIXME
+         */
+        public float getAvailableExecutorsLatest() {
+            return stat.availableExecutors.getLatest(TIME_SCALE);
         }
 
         /**
@@ -517,10 +567,8 @@ public class NodeProvisioner {
         public String toString() {
             final StringBuilder sb = new StringBuilder("StrategyState{");
             sb.append("label=").append(label);
-            sb.append(", queueLengthSnapshot=").append(queueLengthSnapshot);
+            sb.append(", snapshot=").append(snapshot);
             sb.append(", plannedCapacitySnapshot=").append(plannedCapacitySnapshot);
-            sb.append(", idleSnapshot=").append(idleSnapshot);
-            sb.append(", totalSnapshot=").append(totalSnapshot);
             sb.append(", additionalPlannedCapacity=").append(additionalPlannedCapacity);
             sb.append('}');
             return sb.toString();
@@ -571,21 +619,24 @@ public class NodeProvisioner {
             estimate won't create a starvation.
          */
 
-            boolean needSomeWhenNoneAtAll = (state.getIdleSnapshot() == 0)
-                    && (state.getTotalSnapshot() + state.getPlannedCapacitySnapshot() + state.getAdditionalPlannedCapacity() == 0)
-                    && (state.getQueueLengthSnapshot() > 0);
-            float idle = Math.max(state.getIdleLatest(), state.getIdleSnapshot());
-            if (idle < MARGIN || needSomeWhenNoneAtAll) {
+            final LoadStatistics.LoadStatisticsSnapshot snapshot = state.getSnapshot();
+            boolean needSomeWhenNoneAtAll = (snapshot.getAvailableExecutors() + snapshot.getConnectingExecutors() == 0)
+                    && (snapshot.getOnlineExecutors() + state.getPlannedCapacitySnapshot() + state.getAdditionalPlannedCapacity() == 0)
+                    && (snapshot.getQueueLength() > 0);
+            float available = Math.max(snapshot.getAvailableExecutors(), state.getAvailableExecutorsLatest());
+            if (available < MARGIN || needSomeWhenNoneAtAll) {
                 // make sure the system is fully utilized before attempting any new launch.
 
                 // this is the amount of work left to be done
-                float qlen = Math.min(state.getQueueLengthLatest(), state.getQueueLengthSnapshot());
+                float qlen = Math.min(state.getQueueLengthLatest(), snapshot.getQueueLength());
+
+                float connectingCapacity = Math.min(state.getConnectingExecutorsLatest(), snapshot.getConnectingExecutors());
 
                 // ... and this is the additional executors we've already provisioned.
                 float plannedCapacity = Math.max(state.getPlannedCapacityLatest(), state.getPlannedCapacitySnapshot())
                         + state.getAdditionalPlannedCapacity();
 
-                float excessWorkload = qlen - plannedCapacity;
+                float excessWorkload = qlen - plannedCapacity - connectingCapacity;
                 if (needSomeWhenNoneAtAll && excessWorkload < 1) {
                     // in this specific exceptional case we should just provision right now
                     // the exponential smoothing will delay the build unnecessarily
@@ -594,12 +645,12 @@ public class NodeProvisioner {
                 float m = calcThresholdMargin(state.getTotalSnapshot());
                 if (excessWorkload > 1 - m) {// and there's more work to do...
                     LOGGER.log(Level.FINE, "Excess workload {0,number,#.###} detected. "
-                                    + "(planned capacity={1,number,#.###},"
-                                    + "Qlen={2,number,#.###},idle={3,number,#.###}&{4,number,integer},"
-                                    + "total={5,number,integer},m={6,number,#.###})",
+                                    + "(planned capacity={1,number,#.###},connecting capacity={7,number,#.###},"
+                                    + "Qlen={2,number,#.###},available={3,number,#.###}&{4,number,integer},"
+                                    + "online={5,number,integer},m={6,number,#.###})",
                             new Object[]{
-                                    excessWorkload, plannedCapacity, qlen, idle, state.getIdleSnapshot(),
-                                    state.getTotalSnapshot(), m
+                                    excessWorkload, plannedCapacity, qlen, available, snapshot.getAvailableExecutors(),
+                                    snapshot.getOnlineExecutors(), m , snapshot.getConnectingExecutors()
                             });
 
                     CLOUD:

core/src/main/java/jenkins/model/UnlabeledLoadStatistics.java

@@ -29,6 +29,14 @@ import hudson.model.Node;
 import hudson.model.Node.Mode;
 import hudson.model.OverallLoadStatistics;
 import hudson.model.Queue.Task;
+import hudson.model.queue.SubTask;
+import hudson.util.Iterators;
+
+import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Set;
 
 /**
  * {@link LoadStatistics} that track the "free roam" jobs (whose {@link Task#getAssignedLabel()} is null)
@@ -41,6 +49,8 @@ import hudson.model.Queue.Task;
  */
 public class UnlabeledLoadStatistics extends LoadStatistics {
 
+    private final Iterable<Node> nodes = new UnlabeledNodesIterable();
+
     UnlabeledLoadStatistics() {
         super(0, 0);
     }
@@ -73,4 +83,42 @@ public class UnlabeledLoadStatistics extends LoadStatistics {
     public int computeQueueLength() {
         return Jenkins.getInstance().getQueue().countBuildableItemsFor(null);
     }
+
+    @Override
+    protected Iterable<Node> getNodes() {
+        return nodes;
+    }
+
+    @Override
+    protected boolean matches(SubTask item) {
+        return true;
+    }
+
+    private static class UnlabeledNodesIterable implements Iterable<Node> {
+
+        @Override
+        public Iterator<Node> iterator() {
+            return new UnlabeledNodesIterator();
+        }
+    }
+
+    private static class UnlabeledNodesIterator extends Iterators.FilterIterator<Node> {
+
+        protected UnlabeledNodesIterator() {
+            super(Jenkins.getActiveInstance().getNodes().iterator());
+        }
+
+        @Override
+        protected boolean filter(Node n) {
+            return n != null && n.getMode() == Mode.NORMAL;
+        }
+
+        public void remove() {
+            // why does Iterators.FilterIterator do the stupid thing and allow remove?
+            // (remove should remove the object last returned by next(), but it won't if hasNext() is called
+            // the way Iterators.FilterIterator is written... it should just return a read-only
+            // view... which is what we do!
+            throw new UnsupportedOperationException("remove");
+        }
+    }
 }

core/src/main/resources/hudson/model/Messages.properties

@@ -325,8 +325,13 @@ ListView.DisplayName=List View
 
 MyView.DisplayName=My View
 
+LoadStatistics.Legends.DefinedExecutors=Defined executors
+LoadStatistics.Legends.OnlineExecutors=Online executors
+LoadStatistics.Legends.ConnectingExecutors=Connecting executors
 LoadStatistics.Legends.TotalExecutors=Total executors
 LoadStatistics.Legends.BusyExecutors=Busy executors
+LoadStatistics.Legends.IdleExecutors=Idle executors
+LoadStatistics.Legends.AvailableExecutors=Available executors
 LoadStatistics.Legends.QueueLength=Queue length
 
 Cause.LegacyCodeCause.ShortDescription=Legacy code started this job.  No cause information is available

core/src/test/java/hudson/model/LoadStatisticsTest.java

@@ -24,6 +24,7 @@
 package hudson.model;
 
 import hudson.model.MultiStageTimeSeries.TimeScale;
+import hudson.model.queue.SubTask;
 
 import org.apache.commons.io.IOUtils;
 import org.jfree.chart.JFreeChart;
@@ -35,6 +36,9 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
 /**
  * @author Kohsuke Kawaguchi
  */
@@ -54,17 +58,28 @@ public class LoadStatisticsTest {
             public int computeQueueLength() {
                 throw new UnsupportedOperationException();
             }
+            @Override
+            protected Iterable<Node> getNodes() {
+                throw new UnsupportedOperationException();
+            }
+
+            @Override
+            protected boolean matches(SubTask item) {
+                throw new UnsupportedOperationException();
+            }
         };
 
         for (int i = 0; i < 50; i++) {
-            ls.totalExecutors.update(4);
+            ls.onlineExecutors.update(4);
             ls.busyExecutors.update(3);
+            ls.availableExecutors.update(1);
             ls.queueLength.update(3);
         }
 
         for (int i = 0; i < 50; i++) {
-            ls.totalExecutors.update(0);
+            ls.onlineExecutors.update(0);
             ls.busyExecutors.update(0);
+            ls.availableExecutors.update(0);
             ls.queueLength.update(1);
         }
 
@@ -80,4 +95,42 @@ public class LoadStatisticsTest {
             tempFile.delete();
         }
     }
+
+    @Test
+    public void isModernWorks() throws Exception {
+        assertThat(LoadStatistics.isModern(Modern.class), is(true));
+        assertThat(LoadStatistics.isModern(LoadStatistics.class), is(false));
+    }
+
+    private class Modern extends LoadStatistics {
+
+        protected Modern(int initialOnlineExecutors, int initialBusyExecutors) {
+            super(initialOnlineExecutors, initialBusyExecutors);
+        }
+
+        @Override
+        public int computeIdleExecutors() {
+            return 0;
+        }
+
+        @Override
+        public int computeTotalExecutors() {
+            return 0;
+        }
+
+        @Override
+        public int computeQueueLength() {
+            return 0;
+        }
+
+        @Override
+        protected Iterable<Node> getNodes() {
+            return null;
+        }
+
+        @Override
+        protected boolean matches(SubTask item) {
+            return false;
+        }
+    }
 }