From 0f6d45fddc44506857a285637099e49f53b67913 Mon Sep 17 00:00:00 2001
From: Jon Bratseth <bratseth@gmail.com>
Date: Wed, 24 Jun 2020 00:07:29 +0200
Subject: Prefer hosts at least twice the size of the node

---
 .../provision/provisioning/PrioritizableNode.java  | 14 ++++++++++++
 .../provision/provisioning/ProvisioningTest.java   | 25 +++++++++++++++++++++-
 2 files changed, 38 insertions(+), 1 deletion(-)

(limited to 'node-repository/src')

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 0c1b396c40c..8633c1ca325 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
@@ -103,6 +103,11 @@ class PrioritizableNode implements Comparable<PrioritizableNode> {
                                                                           other.parent.get().flavor().resources().storageType());
             if (storageCostDifference != 0)
                 return storageCostDifference;
+
+            // Prefer hosts that are at least twice the size of this node
+            // (utilization is more even if one application does not dominate the host)
+            if ( lessThanHalfTheHost(this) && ! lessThanHalfTheHost(other)) return -1;
+            if ( ! lessThanHalfTheHost(this) && lessThanHalfTheHost(other)) return 1;
         }
 
         int hostPriority = Double.compare(this.skewWithThis() - this.skewWithoutThis(),
@@ -128,6 +133,15 @@ class PrioritizableNode implements Comparable<PrioritizableNode> {
     /** Returns the allocation skew of the parent of this after adding this node to it */
     double skewWithThis() { return skewWith(node.resources()); }
 
+    private boolean lessThanHalfTheHost(PrioritizableNode node) {
+        var n = node.node.resources();
+        var h = node.parent.get().resources();
+        if (h.vcpu()     < n.vcpu()     * 2) return false;
+        if (h.memoryGb() < n.memoryGb() * 2) return false;
+        if (h.diskGb()   < n.diskGb()   * 2) return false;
+        return true;
+    }
+
     private double skewWith(NodeResources resources) {
         if (parent.isEmpty()) return 0;
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
index 607ca963cef..3eb379b0914 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
@@ -265,7 +265,6 @@ public class ProvisioningTest {
         ApplicationId application1 = tester.makeApplicationId();
 
         tester.makeReadyHosts(12, small);
-        tester.makeReadyHosts(16, large);
         tester.deployZoneApp();
 
         // deploy
@@ -276,6 +275,9 @@ public class ProvisioningTest {
         SystemState state2 = prepare(application1, 2, 2, 3, 3, small, tester);
         tester.activate(application1, state2.allHosts);
 
+        tester.makeReadyHosts(16, large);
+        tester.deployZoneApp();
+
         // redeploy with increased sizes and new flavor
         SystemState state3 = prepare(application1, 3, 4, 4, 5, large, tester);
         assertEquals("New nodes are reserved", 16, tester.nodeRepository().getNodes(application1, Node.State.reserved).size());
@@ -288,6 +290,27 @@ public class ProvisioningTest {
                      0, tester.getNodes(application1, Node.State.active).retired().resources(large).size());
     }
 
+    @Test
+    public void host_flavors_at_least_twice_as_large_as_preferred() {
+        NodeResources small = new NodeResources(1, 4, 10, 0.3);
+        NodeResources large = new NodeResources(8, 8, 40, 0.3);
+
+        ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build();
+
+        ApplicationId application1 = tester.makeApplicationId();
+
+        tester.makeReadyHosts(12, small);
+        tester.makeReadyHosts(12, large);
+        tester.deployZoneApp();
+
+        // deploy
+        SystemState state1 = prepare(application1, 2, 2, 4, 4, small, tester);
+        tester.activate(application1, state1.allHosts);
+
+        tester.nodeRepository().getNodes(application1)
+              .forEach(n -> assertEquals(large, tester.nodeRepository().getNode(n.parentHostname().get()).get().resources()));
+    }
+
     @Test
     public void application_deployment_above_then_at_capacity_limit() {
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build();
-- 
cgit v1.2.3