Merge pull request #24894 from vespa-engine/bratseth/dynamic-shared-autoscaling

Support autoscaling in dynamic shared zones

10 files changed, 65 insertions, 40 deletions

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 4f3a3fd5afa..1b76f0a36fd 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
@@ -34,12 +34,12 @@ public class AutoscalingTest {
 
     @Test
     public void test_autoscaling_single_content_group() {
-        var fixture = AutoscalingTester.fixture().hostCount(20).build();
+        var fixture = AutoscalingTester.fixture().awsProdSetup(true).build();
 
         fixture.loader().applyCpuLoad(0.7f, 10);
-        ClusterResources scaledResources = fixture.tester().assertResources("Scaling up since resource usage is too high",
-                                                                            9, 1, 3.6,  8, 37.5,
-                                                                            fixture.autoscale());
+        var scaledResources = fixture.tester().assertResources("Scaling up since resource usage is too high",
+                                                               9, 1, 3.6,  8.3, 37.7,
+                                                               fixture.autoscale());
 
         fixture.deploy(Capacity.from(scaledResources));
         assertTrue("Cluster in flux -> No further change", fixture.autoscale().isEmpty());
@@ -55,14 +55,22 @@ public class AutoscalingTest {
         fixture.tester().clock().advance(Duration.ofDays(2));
         fixture.loader().applyCpuLoad(0.1f, 10);
         fixture.tester().assertResources("Scaling cpu down since usage has gone down significantly",
-                                         8, 1, 1.0, 8.1, 38.1,
+                                         8, 1, 1.0, 8.5, 38.5,
                                          fixture.autoscale());
     }
 
     /** Using too many resources for a short period is proof we should scale up regardless of the time that takes. */
     @Test
-    public void test_no_autoscaling_with_no_measurements() {
+    public void test_no_autoscaling_with_no_measurements_shared_hosts() {
+        var fixture = AutoscalingTester.fixture().awsProdSetup(true).build();
+        assertTrue(fixture.autoscale().target().isEmpty());
+    }
+
+    /** Using too many resources for a short period is proof we should scale up regardless of the time that takes. */
+    @Test
+    public void test_no_autoscaling_with_no_measurements_exclusive_hosts() {
         var fixture = AutoscalingTester.fixture().awsProdSetup(false).build();
+        System.out.println(fixture.autoscale().target());
         assertTrue(fixture.autoscale().target().isEmpty());
     }
 
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 0eb65511299..56f23e62f90 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
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.autoscale;
 
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Capacity;
+import com.yahoo.config.provision.Cloud;
 import com.yahoo.config.provision.ClusterResources;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Flavor;
@@ -53,7 +54,7 @@ class AutoscalingTester {
         provisioningTester = new ProvisioningTester.Builder().zone(zone)
                                                              .flavors(flavors)
                                                              .resourcesCalculator(resourcesCalculator)
-                                                             .hostProvisioner(zone.cloud().dynamicProvisioning() ? new MockHostProvisioner(flavors) : null)
+                                                             .hostProvisioner(zone.cloud().dynamicProvisioning() ? new MockHostProvisioner(flavors, zone.cloud()) : null)
                                                              .build();
 
         hostResourcesCalculator = resourcesCalculator;
@@ -209,7 +210,7 @@ class AutoscalingTester {
     public static class MockHostResourcesCalculator implements HostResourcesCalculator {
 
         private final Zone zone;
-        private double memoryTax = 0;
+        private double memoryTax;
 
         public MockHostResourcesCalculator(Zone zone, double memoryTax) {
             this.zone = zone;
@@ -249,8 +250,8 @@ class AutoscalingTester {
 
     private class MockHostProvisioner extends com.yahoo.vespa.hosted.provision.testutils.MockHostProvisioner {
 
-        public MockHostProvisioner(List<Flavor> flavors) {
-            super(flavors);
+        public MockHostProvisioner(List<Flavor> flavors, Cloud cloud) {
+            super(flavors, cloud);
         }
 
         @Override
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/awsnodes/AwsHostResourcesCalculatorImpl.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/awsnodes/AwsHostResourcesCalculatorImpl.java
index deb6f41c383..4609c0a4023 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/awsnodes/AwsHostResourcesCalculatorImpl.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/awsnodes/AwsHostResourcesCalculatorImpl.java
@@ -9,6 +9,7 @@ import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.Nodelike;
 import com.yahoo.vespa.hosted.provision.provisioning.HostResourcesCalculator;
 
+import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -44,9 +45,9 @@ public class AwsHostResourcesCalculatorImpl implements HostResourcesCalculator {
 
     @Override
     public NodeResources requestToReal(NodeResources advertisedResources, boolean exclusive) {
-        double memoryOverhead = compatibleFlavors(advertisedResources, exclusive)
+        double memoryOverhead = flavorsCompatibleWithAdvertised(advertisedResources, exclusive)
                                         .mapToDouble(flavor -> resourcesCalculator.memoryOverhead(flavor, advertisedResources, false)).max().orElse(0);
-        double diskOverhead   = compatibleFlavors(advertisedResources, exclusive)
+        double diskOverhead   = flavorsCompatibleWithAdvertised(advertisedResources, exclusive)
                                         .mapToDouble(flavor -> resourcesCalculator.diskOverhead(flavor, advertisedResources, false, exclusive)).max().orElse(0);
         return advertisedResources.withMemoryGb(advertisedResources.memoryGb() - memoryOverhead)
                                   .withDiskGb(advertisedResources.diskGb() - diskOverhead);
@@ -56,7 +57,7 @@ public class AwsHostResourcesCalculatorImpl implements HostResourcesCalculator {
     public NodeResources realToRequest(NodeResources realResources, boolean exclusive) {
         double worstMemoryOverhead = 0;
         double worstDiskOverhead = 0;
-        for (VespaFlavor flavor : flavors.values()) {
+        for (VespaFlavor flavor : flavorsCompatibleWithReal(realResources, exclusive)) {
             double memoryOverhead = resourcesCalculator.memoryOverhead(flavor, realResources, true);
             double diskOverhead = resourcesCalculator.diskOverhead(flavor, realResources, true, exclusive);
             NodeResources advertised = realResources.withMemoryGb(realResources.memoryGb() + memoryOverhead)
@@ -77,11 +78,20 @@ public class AwsHostResourcesCalculatorImpl implements HostResourcesCalculator {
     }
 
     /** Returns the flavors of hosts which are eligible and matches the given advertised resources */
-    private Stream<VespaFlavor> compatibleFlavors(NodeResources advertisedResources, boolean exclusive) {
+    private Stream<VespaFlavor> flavorsCompatibleWithAdvertised(NodeResources advertisedResources, boolean exclusive) {
         return flavors.values().stream()
                       .filter(flavor -> exclusive
                                         ? flavor.advertisedResources().compatibleWith(advertisedResources)
                                         : flavor.advertisedResources().satisfies(advertisedResources));
     }
 
+    /** Returns the flavors of hosts which are eligible and matches the given real resources */
+    private List<VespaFlavor> flavorsCompatibleWithReal(NodeResources realResources, boolean exclusive) {
+        return flavors.values().stream()
+                      .filter(flavor -> exclusive
+                                        ? flavor.realResources().compatibleWith(realResources)
+                                        : flavor.realResources().satisfies(realResources))
+                      .collect(Collectors.toList());
+    }
+
 }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/awsnodes/AwsResourcesCalculator.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/awsnodes/AwsResourcesCalculator.java
index b16fcfc2739..63f6d50ab2e 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/awsnodes/AwsResourcesCalculator.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/awsnodes/AwsResourcesCalculator.java
@@ -27,7 +27,7 @@ public class AwsResourcesCalculator {
     }
 
     /**
-     * Returns the memory overhead resulting if the given advertised resources are placed on the given node
+     * Returns the memory overhead resulting if the given resources are placed on the given node
      *
      * @param real true if the given resources are in real values, false if they are in advertised
      */
@@ -37,10 +37,11 @@ public class AwsResourcesCalculator {
                 + hostMemory; // Approximate cost of host administration processes
 
         if (hostMemoryOverhead > hostFlavor.advertisedResources().memoryGb()) // An unusably small flavor,
-            hostMemoryOverhead = hostFlavor.advertisedResources().memoryGb(); // overhead cannot exceed what's available
-
+            return resources.memoryGb(); // all will be overhead
         double memoryShare = resources.memoryGb() /
                              ( hostFlavor.advertisedResources().memoryGb() - ( real ? hostMemoryOverhead : 0));
+        if (memoryShare > 1) // The real resources of the host cannot fit the requested real resources after overhead
+            memoryShare = 1;
         return hostMemoryOverhead * memoryShare;
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirerTest.java
index ac20b9164f8..8bdb0fb2daf 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DirtyExpirerTest.java
@@ -39,7 +39,7 @@ public class DirtyExpirerTest {
     private void assertAllocationAfterExpiry(boolean dynamicProvisioning) {
         Zone zone = new Zone(Cloud.builder().dynamicProvisioning(dynamicProvisioning).build(), SystemName.main, Environment.prod, RegionName.from("us-east"));
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(zone)
-                .hostProvisioner(dynamicProvisioning ? new MockHostProvisioner(List.of()) : null)
+                .hostProvisioner(dynamicProvisioning ? new MockHostProvisioner(List.of(), zone.cloud()) : null)
                 .build();
 
         Node node = Node.create("id", "node1.domain.tld", new Flavor(NodeResources.unspecified()), Node.State.dirty, NodeType.tenant)
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DiskReplacerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DiskReplacerTest.java
index ef0c524c48e..599303bb098 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DiskReplacerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DiskReplacerTest.java
@@ -1,5 +1,6 @@
 package com.yahoo.vespa.hosted.provision.maintenance;
 
+import com.yahoo.config.provision.Cloud;
 import com.yahoo.config.provision.NodeResources;
 import com.yahoo.vespa.hosted.provision.node.Agent;
 import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester;
@@ -17,7 +18,7 @@ import static org.junit.Assert.assertEquals;
 public class DiskReplacerTest {
 
     private final ProvisioningTester tester = new ProvisioningTester.Builder().build();
-    private final MockHostProvisioner hostProvisioner = new MockHostProvisioner(List.of());
+    private final MockHostProvisioner hostProvisioner = new MockHostProvisioner(List.of(), Cloud.defaultCloud());
     private final DiskReplacer diskReplacer = new DiskReplacer(tester.nodeRepository(), Duration.ofDays(1), new TestMetric(), hostProvisioner);
 
     @Test
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java
index 4b90923efc2..409dc4dac1a 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainerTest.java
@@ -274,6 +274,7 @@ public class HostCapacityMaintainerTest {
         tester.maintain();
 
         // Hosts are provisioned
+        // TODO: Not thread safe as HostCapacityMaintainer may count hoists before we are done provisioning
         assertEquals(2, tester.provisionedHostsMatching(resources1));
         assertEquals(0, tester.hostProvisioner.deprovisionedHosts());
 
@@ -623,10 +624,11 @@ public class HostCapacityMaintainerTest {
         }
 
         public DynamicProvisioningTester(Cloud cloud, MockNameResolver nameResolver) {
-            this.hostProvisioner = new MockHostProvisioner(flavors.getFlavors(), nameResolver, 0);
-            this.provisioningTester = new ProvisioningTester.Builder().zone(new Zone(cloud, SystemName.defaultSystem(),
-                                                                                     Environment.defaultEnvironment(),
-                                                                                     RegionName.defaultName()))
+            Zone zone = new Zone(cloud, SystemName.defaultSystem(),
+                                 Environment.defaultEnvironment(),
+                                 RegionName.defaultName());
+            this.hostProvisioner = new MockHostProvisioner(flavors.getFlavors(), nameResolver, 0, cloud);
+            this.provisioningTester = new ProvisioningTester.Builder().zone(zone)
                                                                       .flavors(flavors.getFlavors())
                                                                       .nameResolver(nameResolver)
                                                                       .flagSource(flagSource)
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisionerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisionerTest.java
index bf51d80877c..64b32276ca2 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisionerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostResumeProvisionerTest.java
@@ -38,12 +38,13 @@ public class HostResumeProvisionerTest {
 
     private final List<Flavor> flavors = FlavorConfigBuilder.createDummies("default").getFlavors();
     private final MockNameResolver nameResolver = new MockNameResolver();
-    private final MockHostProvisioner hostProvisioner = new MockHostProvisioner(flavors, nameResolver, 0);
+    private final Zone zone = new Zone(Cloud.builder().dynamicProvisioning(true).allowHostSharing(false).build(),
+                                       SystemName.defaultSystem(),
+                                       Environment.dev,
+                                       RegionName.defaultName());
+    private final MockHostProvisioner hostProvisioner = new MockHostProvisioner(flavors, nameResolver, 0, zone.cloud());
     private final ProvisioningTester tester = new ProvisioningTester.Builder()
-            .zone(new Zone(Cloud.builder().dynamicProvisioning(true).allowHostSharing(false).build(),
-                           SystemName.defaultSystem(),
-                           Environment.dev,
-                           RegionName.defaultName()))
+            .zone(zone)
             .hostProvisioner(hostProvisioner)
             .nameResolver(nameResolver)
             .flavors(flavors)
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostRetirerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostRetirerTest.java
index a46946b7cee..c7e06676a2e 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostRetirerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/HostRetirerTest.java
@@ -30,14 +30,15 @@ public class HostRetirerTest {
     @Test
     public void retire_hosts() {
         NodeFlavors flavors = FlavorConfigBuilder.createDummies("default");
-        MockHostProvisioner hostProvisioner = new MockHostProvisioner(flavors.getFlavors());
+        Zone zone = new Zone(Cloud.builder()
+                                  .dynamicProvisioning(true)
+                                  .build(), SystemName.defaultSystem(),
+                             Environment.defaultEnvironment(),
+                             RegionName.defaultName());
+        MockHostProvisioner hostProvisioner = new MockHostProvisioner(flavors.getFlavors(), zone.cloud());
         ProvisioningTester tester = new ProvisioningTester.Builder().hostProvisioner(hostProvisioner)
                                                                     .flavors(flavors.getFlavors())
-                                                                    .zone(new Zone(Cloud.builder()
-                                                                                        .dynamicProvisioning(true)
-                                                                                        .build(), SystemName.defaultSystem(),
-                                                                                   Environment.defaultEnvironment(),
-                                                                                   RegionName.defaultName()))
+                                                                    .zone(zone)
                                                                     .build();
         HostRetirer retirer = new HostRetirer(tester.nodeRepository(), Duration.ofDays(1), new MockMetric(), hostProvisioner);
         tester.makeReadyHosts(3, new NodeResources(24, 48, 1000, 10))
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
index 55c41cae1d4..53ce516c072 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
@@ -226,7 +226,7 @@ public class DynamicProvisioningTest {
 
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(zone)
                                                                     .flavors(flavors)
-                                                                    .hostProvisioner(new MockHostProvisioner(flavors, memoryTax))
+                                                                    .hostProvisioner(new MockHostProvisioner(flavors, memoryTax, zone.cloud()))
                                                                     .nameResolver(nameResolver)
                                                                     .resourcesCalculator(memoryTax, 0)
                                                                     .build();
@@ -268,7 +268,7 @@ public class DynamicProvisioningTest {
         InMemoryFlagSource flagSource = new InMemoryFlagSource();
         List<Flavor> flavors = List.of(new Flavor("x86", new NodeResources(1, 4, 50, 0.1, fast, local, Architecture.x86_64)),
                                        new Flavor("arm", new NodeResources(1, 4, 50, 0.1, fast, local, Architecture.arm64)));
-        MockHostProvisioner hostProvisioner = new MockHostProvisioner(flavors);
+        MockHostProvisioner hostProvisioner = new MockHostProvisioner(flavors, zone.cloud());
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(zone)
                 .flavors(flavors)
                 .hostProvisioner(hostProvisioner)
@@ -315,7 +315,7 @@ public class DynamicProvisioningTest {
 
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(zone)
                                                                     .flavors(flavors)
-                                                                    .hostProvisioner(new MockHostProvisioner(flavors, memoryTax))
+                                                                    .hostProvisioner(new MockHostProvisioner(flavors, memoryTax, zone.cloud()))
                                                                     .nameResolver(nameResolver)
                                                                     .resourcesCalculator(memoryTax, 0)
                                                                     .build();
@@ -325,7 +325,7 @@ public class DynamicProvisioningTest {
         ApplicationId app1 = ProvisioningTester.applicationId("app1");
         ClusterSpec cluster1 = ClusterSpec.request(ClusterSpec.Type.content, new ClusterSpec.Id("cluster1")).vespaVersion("7").build();
 
-        // Limits where each number is within flavor limits but but which don't contain any flavor leads to an error
+        // Limits where each number is within flavor limits but which don't contain any flavor leads to an error
         try {
             tester.activate(app1, cluster1, Capacity.from(resources(8, 4, 3.8, 20, 40),
                                                           resources(10, 5, 5, 25, 50)));
@@ -390,7 +390,7 @@ public class DynamicProvisioningTest {
 
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(zone)
                                                                     .flavors(flavors)
-                                                                    .hostProvisioner(new MockHostProvisioner(flavors, memoryTax))
+                                                                    .hostProvisioner(new MockHostProvisioner(flavors, memoryTax, zone.cloud()))
                                                                     .nameResolver(nameResolver)
                                                                     .resourcesCalculator(memoryTax, 0)
                                                                     .build();
@@ -425,7 +425,7 @@ public class DynamicProvisioningTest {
 
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(zone)
                                                                     .flavors(flavors)
-                                                                    .hostProvisioner(new MockHostProvisioner(flavors, memoryTax))
+                                                                    .hostProvisioner(new MockHostProvisioner(flavors, memoryTax, zone.cloud()))
                                                                     .nameResolver(nameResolver)
                                                                     .resourcesCalculator(memoryTax, localDiskTax)
                                                                     .build();