Use advertised resources when making allocations

6 files changed, 76 insertions, 53 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java
new file mode 100644
index 00000000000..0b56a49e106
--- /dev/null
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java
@@ -0,0 +1,40 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.provision.autoscale;
+
+import com.yahoo.config.provision.NodeResources;
+
+/**
+ * @author bratseth
+ */
+public class AllocatableClusterResources {
+
+    private final ClusterResources realResources;
+    private final ClusterResources advertisedResources;
+    private final double cost;
+
+    public AllocatableClusterResources(ClusterResources realResources, double cost, NodeResources advertisedResources) {
+        this.realResources = realResources;
+        this.cost = cost;
+        this.advertisedResources = realResources.with(advertisedResources);
+    }
+
+    /**
+     * Returns the resources which will actually be available in this cluster with this allocation.
+     * These should be used for reasoning about allocation to meet measured demand.
+     */
+    public ClusterResources realResources() { return realResources; }
+
+    /**
+     * Returns the resources advertised by the cloud provider, which are the basis for charging
+     * and which must be used in resource allocation requests
+     */
+    public ClusterResources advertisedResources() { return advertisedResources; }
+
+    public double cost() { return cost; }
+
+    @Override
+    public String toString() {
+        return "$" + cost + ": " + realResources();
+    }
+
+}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
index f1d209b2ade..443fd0574d4 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
@@ -31,9 +31,8 @@ public class Autoscaler {
     /*
      TODO:
      - Scale group size
-     - Have a better idea about whether we have sufficient information to make decisions
      - Consider taking spikes/variance into account
-     - Measure observed regulation lag (startup+redistribution) into account when deciding regulation observation window
+     - Measure observed regulation lag (startup+redistribution) and take it into account when deciding regulation observation window
      - Test AutoscalingMaintainer
      - Scale by performance not just load+cost
      */
@@ -78,11 +77,11 @@ public class Autoscaler {
             return Optional.empty();
         }
 
-        Optional<ClusterResourcesWithCost> bestAllocation = findBestAllocation(cpuLoad.get(),
-                                                                               memoryLoad.get(),
-                                                                               diskLoad.get(),
-                                                                               currentAllocation,
-                                                                               cluster);
+        Optional<AllocatableClusterResources> bestAllocation = findBestAllocation(cpuLoad.get(),
+                                                                                  memoryLoad.get(),
+                                                                                  diskLoad.get(),
+                                                                                  currentAllocation,
+                                                                                  cluster);
         if (bestAllocation.isEmpty()) {
             log.fine("Autoscaling " + applicationId + " " + cluster + ": Could not find a better allocation");
             return Optional.empty();
@@ -95,15 +94,15 @@ public class Autoscaler {
             log.fine("Autoscaling " + applicationId + " " + cluster + ": Resources are almost ideal and price difference is small");
             return Optional.empty(); // Avoid small, unnecessary changes
         }
-        return bestAllocation.map(a -> a.clusterResources());
+        return bestAllocation.map(a -> a.advertisedResources());
     }
 
-    private Optional<ClusterResourcesWithCost> findBestAllocation(double cpuLoad, double memoryLoad, double diskLoad,
-                                                                  ClusterResources currentAllocation, ClusterSpec cluster) {
-        Optional<ClusterResourcesWithCost> bestAllocation = Optional.empty();
+    private Optional<AllocatableClusterResources> findBestAllocation(double cpuLoad, double memoryLoad, double diskLoad,
+                                                                     ClusterResources currentAllocation, ClusterSpec cluster) {
+        Optional<AllocatableClusterResources> bestAllocation = Optional.empty();
         for (ResourceIterator i = new ResourceIterator(cpuLoad, memoryLoad, diskLoad, currentAllocation); i.hasNext(); ) {
             ClusterResources allocation = i.next();
-            Optional<ClusterResourcesWithCost> allocatableResources = toAllocatableResources(allocation, cluster);
+            Optional<AllocatableClusterResources> allocatableResources = toAllocatableResources(allocation, cluster);
             if (allocatableResources.isEmpty()) continue;
             if (bestAllocation.isEmpty() || allocatableResources.get().cost() < bestAllocation.get().cost())
                 bestAllocation = allocatableResources;
@@ -127,14 +126,16 @@ public class Autoscaler {
      * Returns the smallest allocatable node resources larger than the given node resources,
      * or empty if none available.
      */
-    private Optional<ClusterResourcesWithCost> toAllocatableResources(ClusterResources resources, ClusterSpec cluster) {
+    private Optional<AllocatableClusterResources> toAllocatableResources(ClusterResources resources, ClusterSpec cluster) {
         if (allowsHostSharing(nodeRepository.zone().cloud())) {
             // Return the requested resources, adjusted to be legal or empty if they cannot fit on existing hosts
             NodeResources nodeResources = nodeResourceLimits.enlargeToLegal(resources.nodeResources(), cluster.type());
             for (Flavor flavor : nodeRepository.getAvailableFlavors().getFlavors())
                 if (flavor.resources().satisfies(nodeResources))
-                    return Optional.of(new ClusterResourcesWithCost(resources.with(nodeResources),
-                                                                    costOf(nodeResources) * resources.nodes()));
+                    return Optional.of(new AllocatableClusterResources(resources.with(nodeResources),
+                                                                       costOf(nodeResources) * resources.nodes(),
+                                                                       nodeResources
+                                                                       ));
             return Optional.empty();
         }
         else {
@@ -153,8 +154,9 @@ public class Autoscaler {
             if (bestFlavor.isEmpty())
                 return Optional.empty();
             else
-                return Optional.of(new ClusterResourcesWithCost(resources.with(bestFlavor.get().resources()),
-                                                                bestCost * resources.nodes()));
+                return Optional.of(new AllocatableClusterResources(resources.with(bestFlavor.get().resources()),
+                                                                   bestCost * resources.nodes(),
+                                                                   hostResourcesCalculator.availableCapacityOf(bestFlavor.get().name(), bestFlavor.get().resources())));
         }
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterResourcesWithCost.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterResourcesWithCost.java
deleted file mode 100644
index 55b28ef3ce1..00000000000
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterResourcesWithCost.java
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.provision.autoscale;
-
-/**
- * @author bratseth
- */
-public class ClusterResourcesWithCost {
-
-    private final ClusterResources resources;
-    private final double cost;
-
-    public ClusterResourcesWithCost(ClusterResources resources, double cost) {
-        this.resources = resources;
-        this.cost = cost;
-    }
-
-    public ClusterResources clusterResources() { return resources;}
-
-    public double cost() { return cost; }
-
-    @Override
-    public String toString() {
-        return "$" + cost + ": " + clusterResources();
-    }
-
-}
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostResourcesCalculator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostResourcesCalculator.java
index a5570dbf169..8bdbfb15418 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostResourcesCalculator.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostResourcesCalculator.java
@@ -8,7 +8,14 @@ import com.yahoo.config.provision.NodeResources;
  */
 public interface HostResourcesCalculator {
 
-    /** Calculates the resources that are reserved for host level processes and returns the remainder. */
+    /**
+     * Returns the advertised resources for this flavor, which may be more than the actual resources
+     *
+     * @param flavorName the name of the flavor
+     * @param hostResources the real resources of the flavor
+     * @return the advertised resources of this flavor, or the host resources if this flavor is not a host
+     *         flavor with a difference between advertised and real resources
+     */
     NodeResources availableCapacityOf(String flavorName, NodeResources hostResources);
 
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
index 9c1e8dd00cf..dccedd36bfd 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
@@ -124,12 +124,12 @@ public class AutoscalingTest {
         ApplicationId application1 = tester.applicationId("application1");
         ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1");
 
-        // deploy
-        tester.deploy(application1, cluster1, 5, 1, new NodeResources(3, 100, 100, 1));
+        // deploy (Why 83 Gb memory? See AutoscalingTester.MockHostResourcesCalculator
+        tester.deploy(application1, cluster1, 5, 1, new NodeResources(3, 103, 100, 1));
 
         tester.addMeasurements(Resource.memory, 0.9f, 0.6f, 120, application1);
         ClusterResources scaledResources = tester.assertResources("Scaling up since resource usage is too high.",
-                                                                  8, 1, 3,  80, 34.3,
+                                                                  8, 1, 3,  83, 34.3,
                                                                   tester.autoscale(application1, cluster1));
 
         tester.deploy(application1, cluster1, scaledResources);
@@ -137,7 +137,7 @@ public class AutoscalingTest {
 
         tester.addMeasurements(Resource.memory, 0.3f, 0.6f, 1000, application1);
         tester.assertResources("Scaling down since resource usage has gone down",
-                               5, 1, 3, 80, 36,
+                               5, 1, 3, 83, 36,
                                tester.autoscale(application1, cluster1));
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
index 1b0ccc3611c..07fb39c5f3d 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
@@ -53,15 +53,15 @@ class AutoscalingTester {
 
     public AutoscalingTester(Zone zone, List<Flavor> flavors) {
         this(zone,
-             new MockHostProvisioner(flavors),
+             flavors,
              new InMemoryFlagSource().withBooleanFlag(Flags.ENABLE_DYNAMIC_PROVISIONING.id(), true),
              asConfig(flavors));
     }
 
-    private AutoscalingTester(Zone zone, MockHostProvisioner hostProvisioner, FlagSource flagSource, FlavorsConfig flavorsConfig) {
+    private AutoscalingTester(Zone zone, List<Flavor> flavors, FlagSource flagSource, FlavorsConfig flavorsConfig) {
         provisioningTester = new ProvisioningTester.Builder().zone(zone)
                                                              .flavorsConfig(flavorsConfig)
-                                                             .hostProvisioner(hostProvisioner)
+                                                             .hostProvisioner(new MockHostProvisioner(flavors))
                                                              .flagSource(flagSource)
                                                              .build();
 
@@ -210,7 +210,7 @@ class AutoscalingTester {
 
     }
 
-    private static class MockHostProvisioner implements HostProvisioner {
+    private class MockHostProvisioner implements HostProvisioner {
 
         private final List<Flavor> hostFlavors;
 
@@ -245,7 +245,7 @@ class AutoscalingTester {
         }
 
         private boolean matches(Flavor flavor, NodeResources resources) {
-            NodeResources flavorResources = flavor.resources();
+            NodeResources flavorResources = hostResourcesCalculator.availableCapacityOf(flavor.name(), flavor.resources());
             if (flavorResources.storageType() == NodeResources.StorageType.remote
                 && resources.diskGb() <= flavorResources.diskGb())
                 flavorResources = flavorResources.withDiskGb(resources.diskGb());