Make host prioritization explicit

8 files changed, 116 insertions, 78 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java
index 187fb639ac8..08e4f559e32 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java
@@ -216,12 +216,12 @@ public class MetricsReporter extends Maintainer {
     private void updateDockerMetrics(List<Node> nodes) {
         // Capacity flavors for docker
         DockerHostCapacity capacity = new DockerHostCapacity(nodes);
-        metric.set("hostedVespa.docker.totalCapacityCpu", capacity.getCapacityTotal().getCpu(), null);
-        metric.set("hostedVespa.docker.totalCapacityMem", capacity.getCapacityTotal().getMemory(), null);
-        metric.set("hostedVespa.docker.totalCapacityDisk", capacity.getCapacityTotal().getDisk(), null);
-        metric.set("hostedVespa.docker.freeCapacityCpu", capacity.getFreeCapacityTotal().getCpu(), null);
-        metric.set("hostedVespa.docker.freeCapacityMem", capacity.getFreeCapacityTotal().getMemory(), null);
-        metric.set("hostedVespa.docker.freeCapacityDisk", capacity.getFreeCapacityTotal().getDisk(), null);
+        metric.set("hostedVespa.docker.totalCapacityCpu", capacity.getCapacityTotal().vcpu(), null);
+        metric.set("hostedVespa.docker.totalCapacityMem", capacity.getCapacityTotal().memoryGb(), null);
+        metric.set("hostedVespa.docker.totalCapacityDisk", capacity.getCapacityTotal().diskGb(), null);
+        metric.set("hostedVespa.docker.freeCapacityCpu", capacity.getFreeCapacityTotal().vcpu(), null);
+        metric.set("hostedVespa.docker.freeCapacityMem", capacity.getFreeCapacityTotal().memoryGb(), null);
+        metric.set("hostedVespa.docker.freeCapacityDisk", capacity.getFreeCapacityTotal().diskGb(), null);
 
         List<Flavor> dockerFlavors = nodeRepository().getAvailableFlavors().getFlavors().stream()
                 .filter(f -> f.getType().equals(Flavor.Type.DOCKER_CONTAINER))
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java
index 95b8bd24ba5..4c557c60802 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java
@@ -33,13 +33,14 @@ public class DockerHostCapacity {
 
     /**
      * Compare hosts on free capacity.
-     * <p>
      * Used in prioritizing hosts for allocation in <b>descending</b> order.
      */
     int compare(Node hostA, Node hostB) {
-        int comp = freeCapacityOf(hostB, false).compare(freeCapacityOf(hostA, false));
+        int comp = compare(freeCapacityOf(hostB, false),
+                           freeCapacityOf(hostA, false));
         if (comp == 0) {
-            comp = freeCapacityOf(hostB, false).compare(freeCapacityOf(hostA, false));
+            comp = compare(freeCapacityOf(hostB, false),
+                           freeCapacityOf(hostA, false));
             if (comp == 0) {
                 // If resources are equal - we want to assign to the one with the most IPaddresses free
                 comp = freeIPs(hostB) - freeIPs(hostA);
@@ -49,9 +50,11 @@ public class DockerHostCapacity {
     }
 
     int compareWithoutInactive(Node hostA, Node hostB) {
-        int comp = freeCapacityOf(hostB,  true).compare(freeCapacityOf(hostA, true));
+        int comp = compare(freeCapacityOf(hostB,  true),
+                           freeCapacityOf(hostA, true));
         if (comp == 0) {
-            comp = freeCapacityOf(hostB, true).compare(freeCapacityOf(hostA, true));
+            comp = compare(freeCapacityOf(hostB, true),
+                           freeCapacityOf(hostA, true));
             if (comp == 0) {
                 // If resources are equal - we want to assign to the one with the most IPaddresses free
                 comp = freeIPs(hostB) - freeIPs(hostA);
@@ -60,6 +63,10 @@ public class DockerHostCapacity {
         return comp;
     }
 
+    private int compare(ResourceCapacity a, ResourceCapacity b) {
+        return ResourceCapacityComparator.defaultOrder().compare(a, b);
+    }
+
     /**
      * Checks the node capacity and free ip addresses to see
      * if we could allocate a flavor on the docker host.
@@ -105,11 +112,22 @@ public class DockerHostCapacity {
     }
 
     private int canFitNumberOf(Node node, Flavor flavor) {
-        int capacityFactor = freeCapacityOf(node, false).freeCapacityInFlavorEquivalence(flavor);
+        ResourceCapacity freeCapacity = freeCapacityOf(node, false);
+        int capacityFactor = freeCapacityInFlavorEquivalence(freeCapacity, flavor);
         int ips = freeIPs(node);
         return Math.min(capacityFactor, ips);
     }
 
+    int freeCapacityInFlavorEquivalence(ResourceCapacity freeCapacity, Flavor flavor) {
+        if ( ! freeCapacity.hasCapacityFor(ResourceCapacity.of(flavor))) return 0;
+
+        double cpuFactor = Math.floor(freeCapacity.vcpu() / flavor.getMinCpuCores());
+        double memoryFactor = Math.floor(freeCapacity.memoryGb() / flavor.getMinMainMemoryAvailableGb());
+        double diskFactor =  Math.floor(freeCapacity.diskGb() / flavor.getMinDiskAvailableGb());
+
+        return (int) Math.min(Math.min(memoryFactor, cpuFactor), diskFactor);
+    }
+
     /**
      * Calculate the remaining capacity for the dockerHost.
      * @param dockerHost The host to find free capacity of.
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
index d7fe7301b3a..3cac1d843ad 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java
@@ -272,7 +272,8 @@ class NodePrioritizer {
 
     private static int compareForRelocation(Node a, Node b) {
         // Choose smallest node
-        int capacity = ResourceCapacity.of(a).compare(ResourceCapacity.of(b));
+        int capacity = ResourceCapacityComparator.defaultOrder().compare(ResourceCapacity.of(a),
+                                                                         ResourceCapacity.of(b));
         if (capacity != 0) return capacity;
 
         // Choose unallocated over allocated (this case is when we have ready docker nodes)
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java
index f98de7ac579..7abb43374bf 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java
@@ -90,7 +90,7 @@ class PrioritizableNode implements Comparable<PrioritizableNode> {
         if (other.parent.isPresent() && !this.parent.isPresent()) return 1;
 
         // Choose the node with parent node with the least capacity (TODO parameterize this as this is pretty much the core of the algorithm)
-        int freeCapacity = this.freeParentCapacity.compare(other.freeParentCapacity);
+        int freeCapacity = ResourceCapacityComparator.defaultOrder().compare(this.freeParentCapacity, other.freeParentCapacity);
         if (freeCapacity != 0) return freeCapacity;
 
         // Choose cheapest node
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacity.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacity.java
index 8a87d66f72b..3839d66c8de 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacity.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacity.java
@@ -17,14 +17,14 @@ public class ResourceCapacity {
 
     public static final ResourceCapacity NONE = new ResourceCapacity(0, 0, 0);
 
-    private final double memory;
-    private final double cpu;
-    private final double disk;
-
-    private ResourceCapacity(double memory, double cpu, double disk) {
-        this.memory = memory;
-        this.cpu = cpu;
-        this.disk = disk;
+    private final double memoryGb;
+    private final double vcpu;
+    private final double diskGb;
+
+    private ResourceCapacity(double memoryGb, double vcpu, double diskGb) {
+        this.memoryGb = memoryGb;
+        this.vcpu = vcpu;
+        this.diskGb = diskGb;
     }
 
     static ResourceCapacity of(Flavor flavor) {
@@ -32,68 +32,46 @@ public class ResourceCapacity {
                 flavor.getMinMainMemoryAvailableGb(), flavor.getMinCpuCores(), flavor.getMinDiskAvailableGb());
     }
 
-    static ResourceCapacity of(NodeResources flavor) {
-        return new ResourceCapacity(flavor.memoryGb(), flavor.vcpu(), flavor.diskGb());
+    static ResourceCapacity of(NodeResources resources) {
+        return new ResourceCapacity(resources.memoryGb(), resources.vcpu(), resources.diskGb());
     }
 
     static ResourceCapacity of(Node node) {
         return ResourceCapacity.of(node.flavor());
     }
 
-    public double getMemory() {
-        return memory;
+    public double memoryGb() {
+        return memoryGb;
     }
 
-    public double getCpu() {
-        return cpu;
+    public double vcpu() {
+        return vcpu;
     }
 
-    public double getDisk() {
-        return disk;
+    public double diskGb() {
+        return diskGb;
     }
 
     public ResourceCapacity subtract(ResourceCapacity other) {
-        return new ResourceCapacity(memory - other.memory,
-                cpu - other.cpu,
-                disk - other.disk);
+        return new ResourceCapacity(memoryGb - other.memoryGb,
+                                    vcpu - other.vcpu,
+                                    diskGb - other.diskGb);
     }
 
     public ResourceCapacity add(ResourceCapacity other) {
-        return new ResourceCapacity(memory + other.memory,
-                cpu + other.cpu,
-                disk + other.disk);
+        return new ResourceCapacity(memoryGb + other.memoryGb,
+                                    vcpu + other.vcpu,
+                                    diskGb + other.diskGb);
     }
 
     boolean hasCapacityFor(ResourceCapacity capacity) {
-        return memory >= capacity.memory &&
-                cpu >= capacity.cpu &&
-                disk >= capacity.disk;
+        return memoryGb >= capacity.memoryGb &&
+               vcpu >= capacity.vcpu &&
+               diskGb >= capacity.diskGb;
     }
 
     boolean hasCapacityFor(Flavor flavor) {
         return hasCapacityFor(ResourceCapacity.of(flavor));
     }
 
-    int freeCapacityInFlavorEquivalence(Flavor flavor) {
-        if (!hasCapacityFor(ResourceCapacity.of(flavor))) return 0;
-
-        double memoryFactor = Math.floor(memory/flavor.getMinMainMemoryAvailableGb());
-        double cpuFactor = Math.floor(cpu/flavor.getMinCpuCores());
-        double diskFactor =  Math.floor(disk/flavor.getMinDiskAvailableGb());
-
-        return (int) Math.min(Math.min(memoryFactor, cpuFactor), diskFactor);
-    }
-
-    /**
-     * Normal compare implementation where -1 if this is less than that.
-     */
-    int compare(ResourceCapacity that) {
-        if (memory > that.memory) return 1;
-        if (memory < that.memory) return -1;
-        if (disk > that.disk) return 1;
-        if (disk < that.disk) return -1;
-        if (cpu > that.cpu) return 1;
-        if (cpu < that.cpu) return -1;
-        return 0;
-    }
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacityComparator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacityComparator.java
new file mode 100644
index 00000000000..c4f605bb9fc
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacityComparator.java
@@ -0,0 +1,36 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.provisioning;
+
+import java.util.Comparator;
+
+/**
+ * Resource comparator
+ *
+ * @author bratseth
+ */
+public class ResourceCapacityComparator {
+
+    private static final MemoryDiskCpu memoryDiskCpuComparator = new MemoryDiskCpu();
+
+    /** Returns the default ordering */
+    public static Comparator<ResourceCapacity> defaultOrder() { return memoryDiskCpuOrder(); }
+
+    /** Returns a comparator comparing by memory, disk, vcpu */
+    public static Comparator<ResourceCapacity> memoryDiskCpuOrder() { return memoryDiskCpuComparator; }
+
+    private static class MemoryDiskCpu implements Comparator<ResourceCapacity> {
+
+        @Override
+        public int compare(ResourceCapacity a, ResourceCapacity b) {
+            if (a.memoryGb() > b.memoryGb()) return 1;
+            if (a.memoryGb() < b.memoryGb()) return -1;
+            if (a.diskGb() > b.diskGb()) return 1;
+            if (a.diskGb() < b.diskGb()) return -1;
+            if (a.vcpu() > b.vcpu()) return 1;
+            if (a.vcpu() < b.vcpu()) return -1;
+            return 0;
+        }
+
+    }
+
+}
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java
index 4ab9beacf15..9e61cbe98ab 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java
@@ -97,17 +97,17 @@ public class DockerHostCapacityTest {
     @Test
     public void getCapacityTotal() {
         ResourceCapacity total = capacity.getCapacityTotal();
-        assertEquals(21.0, total.getCpu(), 0.1);
-        assertEquals(30.0, total.getMemory(), 0.1);
-        assertEquals(36.0, total.getDisk(), 0.1);
+        assertEquals(21.0, total.vcpu(), 0.1);
+        assertEquals(30.0, total.memoryGb(), 0.1);
+        assertEquals(36.0, total.diskGb(), 0.1);
     }
 
     @Test
     public void getFreeCapacityTotal() {
         ResourceCapacity totalFree = capacity.getFreeCapacityTotal();
-        assertEquals(15.0, totalFree.getCpu(), 0.1);
-        assertEquals(14.0, totalFree.getMemory(), 0.1);
-        assertEquals(24.0, totalFree.getDisk(), 0.1);
+        assertEquals(15.0, totalFree.vcpu(), 0.1);
+        assertEquals(14.0, totalFree.memoryGb(), 0.1);
+        assertEquals(24.0, totalFree.diskGb(), 0.1);
     }
 
     @Test
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacityTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacityTest.java
index 7883d9e58ed..5a021466e58 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacityTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ResourceCapacityTest.java
@@ -51,29 +51,34 @@ public class ResourceCapacityTest {
         assertFalse(capacityOfHostSmall.hasCapacityFor(d3CPUFlavor));
 
         // Compare it to various flavors
-        assertEquals(1, capacityOfHostSmall.compare(nodeCapacity(d1Flavor)));
-        assertEquals(1, capacityOfHostSmall.compare(nodeCapacity(d2Flavor)));
-        assertEquals(0, capacityOfHostSmall.compare(nodeCapacity(d3Flavor)));
-        assertEquals(-1, capacityOfHostSmall.compare(nodeCapacity(d3DiskFlavor)));
-        assertEquals(-1, capacityOfHostSmall.compare(nodeCapacity(d3CPUFlavor)));
-        assertEquals(-1, capacityOfHostSmall.compare(nodeCapacity(d3MemFlavor)));
+        assertEquals(1, compare(capacityOfHostSmall, nodeCapacity(d1Flavor)));
+        assertEquals(1, compare(capacityOfHostSmall, nodeCapacity(d2Flavor)));
+        assertEquals(0, compare(capacityOfHostSmall, nodeCapacity(d3Flavor)));
+        assertEquals(-1, compare(capacityOfHostSmall, nodeCapacity(d3DiskFlavor)));
+        assertEquals(-1, compare(capacityOfHostSmall, nodeCapacity(d3CPUFlavor)));
+        assertEquals(-1, compare(capacityOfHostSmall, nodeCapacity(d3MemFlavor)));
 
         // Change free capacity and assert on rest capacity
         capacityOfHostSmall = capacityOfHostSmall.subtract(ResourceCapacity.of(d1Flavor));
-        assertEquals(0, capacityOfHostSmall.compare(nodeCapacity(d2Flavor)));
+        assertEquals(0, compare(capacityOfHostSmall, nodeCapacity(d2Flavor)));
 
         // Assert on rest capacity
         assertTrue(capacityOfHostSmall.hasCapacityFor(d1Flavor));
         assertFalse(capacityOfHostSmall.hasCapacityFor(d3Flavor));
 
         // At last compare the disk and cpu and mem variations
-        assertEquals(-1, nodeCapacity(d3Flavor).compare(nodeCapacity(d3DiskFlavor)));
-        assertEquals(1, nodeCapacity(d3DiskFlavor).compare(nodeCapacity(d3CPUFlavor)));
-        assertEquals(-1, nodeCapacity(d3CPUFlavor).compare(nodeCapacity(d3MemFlavor)));
-        assertEquals(1, nodeCapacity(d3MemFlavor).compare(nodeCapacity(d3DiskFlavor)));
+        assertEquals(-1, compare(nodeCapacity(d3Flavor), nodeCapacity(d3DiskFlavor)));
+        assertEquals(1, compare(nodeCapacity(d3DiskFlavor), nodeCapacity(d3CPUFlavor)));
+        assertEquals(-1, compare(nodeCapacity(d3CPUFlavor), nodeCapacity(d3MemFlavor)));
+        assertEquals(1, compare(nodeCapacity(d3MemFlavor), nodeCapacity(d3DiskFlavor)));
     }
 
     private ResourceCapacity nodeCapacity(Flavor flavor) {
         return ResourceCapacity.of(flavor);
     }
+
+    private int compare(ResourceCapacity a, ResourceCapacity b) {
+        return ResourceCapacityComparator.defaultOrder().compare(a, b);
+    }
+
 }