diff options
author | Jon Bratseth <bratseth@gmail.com> | 2022-11-18 18:51:52 +0100 |
---|---|---|
committer | Jon Bratseth <bratseth@gmail.com> | 2022-11-18 18:51:52 +0100 |
commit | 40ddafbd211ebf7e59346f731de37b72e35530d6 (patch) | |
tree | 48bd35930e853ebd91285ff0c15b932187347e09 /node-repository/src | |
parent | 2872e68c64605c04147fbf930d76f09078ce5550 (diff) |
Use the overhead of the exactly matching flavors if any
Diffstat (limited to 'node-repository/src')
5 files changed, 79 insertions, 26 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java index b4c01267d3b..dad1bf8b2fa 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeRepositoryProvisioner.java @@ -191,8 +191,8 @@ public class NodeRepositoryProvisioner implements Provisioner { if (limits.min().equals(limits.max())) return limits.min(); // Don't change current deployments that are still legal - var currentAsAdvertised = current.advertisedResources(); - if (! firstDeployment && currentAsAdvertised.isWithin(limits.min(), limits.max())) return currentAsAdvertised; + if (! firstDeployment && current.advertisedResources().isWithin(limits.min(), limits.max())) + return current.advertisedResources(); // Otherwise, find an allocation that preserves the current resources as well as possible return allocationOptimizer.findBestAllocation(Load.one(), 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 f833348b8dc..6c8c3c571f5 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 @@ -649,4 +649,34 @@ public class AutoscalingTest { fixture.autoscale()); } + @Test + public void test_changing_exclusivity() { + var fixture = AutoscalingTester.fixture() + .awsProdSetup(true) + .cluster(clusterSpec(true)) + .initialResources(Optional.empty()) + .build(); + fixture.tester().assertResources("Initial deployment at minimum", + 2, 1, 2, 4, 10, + fixture.currentResources().advertisedResources()); + + fixture.tester().deploy(fixture.applicationId(), clusterSpec(false), fixture.capacity()); + fixture.tester().assertResources("With non-exclusive nodes, a better solution is " + + "50% more nodes with half the cpu", + 3, 1, 1, 4, 10.2, + fixture.autoscale()); + + fixture.tester().deploy(fixture.applicationId(), clusterSpec(true), fixture.capacity()); + fixture.tester().assertResources("Reverts to the initial resources", + 2, 1, 2, 4, 10, + fixture.currentResources().advertisedResources()); + } + + private ClusterSpec clusterSpec(boolean exclusive) { + return ClusterSpec.request(ClusterSpec.Type.container, + ClusterSpec.Id.from("test")).vespaVersion("8.1.2") + .exclusive(exclusive) + .build(); + } + } 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 56f23e62f90..3134d378c1c 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 @@ -210,17 +210,15 @@ class AutoscalingTester { public static class MockHostResourcesCalculator implements HostResourcesCalculator { private final Zone zone; - private double memoryTax; - public MockHostResourcesCalculator(Zone zone, double memoryTax) { + public MockHostResourcesCalculator(Zone zone) { this.zone = zone; - this.memoryTax = memoryTax; } @Override public NodeResources realResourcesOf(Nodelike node, NodeRepository nodeRepository) { if (zone.cloud().dynamicProvisioning()) - return node.resources().withMemoryGb(node.resources().memoryGb() - memoryTax); + return node.resources().withMemoryGb(node.resources().memoryGb()); else return node.resources(); } @@ -228,19 +226,19 @@ class AutoscalingTester { @Override public NodeResources advertisedResourcesOf(Flavor flavor) { if (zone.cloud().dynamicProvisioning()) - return flavor.resources().withMemoryGb(flavor.resources().memoryGb() + memoryTax); + return flavor.resources().withMemoryGb(flavor.resources().memoryGb()); else return flavor.resources(); } @Override public NodeResources requestToReal(NodeResources resources, boolean exclusive) { - return resources.withMemoryGb(resources.memoryGb() - memoryTax); + return resources.withMemoryGb(resources.memoryGb()); } @Override public NodeResources realToRequest(NodeResources resources, boolean exclusive) { - return resources.withMemoryGb(resources.memoryGb() + memoryTax); + return resources.withMemoryGb(resources.memoryGb()); } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java index d348df00b78..d7d77a83caa 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java @@ -60,10 +60,17 @@ public class Fixture { return tester().nodeRepository().applications().get(applicationId).orElse(Application.empty(applicationId)); } + public AllocatableClusterResources currentResources() { + return new AllocatableClusterResources(tester.nodeRepository().nodes().list(Node.State.active).owner(applicationId).cluster(clusterId()), + tester.nodeRepository()); + } + public Cluster cluster() { return application().cluster(clusterId()).get(); } + public Capacity capacity() { return capacity; } + public ClusterModel clusterModel() { return new ClusterModel(application(), clusterSpec, @@ -128,10 +135,10 @@ public class Fixture { List<Flavor> hostFlavors = List.of(new Flavor(new NodeResources(100, 100, 100, 1))); Optional<ClusterResources> initialResources = Optional.of(new ClusterResources(5, 1, new NodeResources(2, 16, 75, 1))); Capacity capacity = Capacity.from(new ClusterResources(2, 1, - new NodeResources(1, 1, 1, 1, NodeResources.DiskSpeed.any)), + new NodeResources(1, 4, 10, 1, NodeResources.DiskSpeed.any)), new ClusterResources(20, 1, new NodeResources(100, 1000, 1000, 1, NodeResources.DiskSpeed.any))); - HostResourcesCalculator resourceCalculator = new AutoscalingTester.MockHostResourcesCalculator(zone, 0); + HostResourcesCalculator resourceCalculator = new AutoscalingTester.MockHostResourcesCalculator(zone); int hostCount = 0; public Fixture.Builder zone(Zone zone) { @@ -168,6 +175,11 @@ public class Fixture { return this; } + public Fixture.Builder cluster(ClusterSpec cluster) { + this.cluster = cluster; + return this; + } + public Fixture.Builder awsProdSetup(boolean allowHostSharing) { return this.awsHostFlavors() .awsResourceCalculator() @@ -215,7 +227,7 @@ public class Fixture { return this; } - public Fixture.Builder hostCount(int hostCount) { // TODO: Remove all usage of this + public Fixture.Builder hostCount(int hostCount) { this.hostCount = hostCount; return this; } 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 4609c0a4023..d148f6d3cc7 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 @@ -12,7 +12,6 @@ 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; /** * @author valerijf @@ -45,19 +44,32 @@ public class AwsHostResourcesCalculatorImpl implements HostResourcesCalculator { @Override public NodeResources requestToReal(NodeResources advertisedResources, boolean exclusive) { - double memoryOverhead = flavorsCompatibleWithAdvertised(advertisedResources, exclusive) - .mapToDouble(flavor -> resourcesCalculator.memoryOverhead(flavor, advertisedResources, false)).max().orElse(0); - double diskOverhead = flavorsCompatibleWithAdvertised(advertisedResources, exclusive) - .mapToDouble(flavor -> resourcesCalculator.diskOverhead(flavor, advertisedResources, false, exclusive)).max().orElse(0); + // Only consider exactly matched flavors if any to avoid concluding we have slightly too little resources + // on an exactly matched flavor if we move from exclusive to shared hosts + List<VespaFlavor> consideredFlavors = flavorsCompatibleWithAdvertised(advertisedResources, true); + if (consideredFlavors.isEmpty()) + consideredFlavors = flavorsCompatibleWithAdvertised(advertisedResources, false); + + double memoryOverhead = consideredFlavors.stream() + .mapToDouble(flavor -> resourcesCalculator.memoryOverhead(flavor, advertisedResources, false)) + .max().orElse(0); + double diskOverhead = consideredFlavors.stream() + .mapToDouble(flavor -> resourcesCalculator.diskOverhead(flavor, advertisedResources, false, exclusive)) + .max().orElse(0); return advertisedResources.withMemoryGb(advertisedResources.memoryGb() - memoryOverhead) .withDiskGb(advertisedResources.diskGb() - diskOverhead); } @Override public NodeResources realToRequest(NodeResources realResources, boolean exclusive) { + // Only consider exactly matched flavors if any to avoid concluding we have slightly too little resources + // on an exactly matched flavor if we move from exclusive to shared hosts + List<VespaFlavor> consideredFlavors = flavorsCompatibleWithReal(realResources, true); + if (consideredFlavors.isEmpty()) + consideredFlavors = flavorsCompatibleWithReal(realResources, false); double worstMemoryOverhead = 0; double worstDiskOverhead = 0; - for (VespaFlavor flavor : flavorsCompatibleWithReal(realResources, exclusive)) { + for (VespaFlavor flavor : consideredFlavors) { double memoryOverhead = resourcesCalculator.memoryOverhead(flavor, realResources, true); double diskOverhead = resourcesCalculator.diskOverhead(flavor, realResources, true, exclusive); NodeResources advertised = realResources.withMemoryGb(realResources.memoryGb() + memoryOverhead) @@ -78,20 +90,21 @@ public class AwsHostResourcesCalculatorImpl implements HostResourcesCalculator { } /** Returns the flavors of hosts which are eligible and matches the given advertised resources */ - private Stream<VespaFlavor> flavorsCompatibleWithAdvertised(NodeResources advertisedResources, boolean exclusive) { + private List<VespaFlavor> flavorsCompatibleWithAdvertised(NodeResources advertisedResources, boolean exactOnly) { return flavors.values().stream() - .filter(flavor -> exclusive - ? flavor.advertisedResources().compatibleWith(advertisedResources) - : flavor.advertisedResources().satisfies(advertisedResources)); + .filter(flavor -> exactOnly + ? flavor.advertisedResources().equalsWhereSpecified(advertisedResources) + : flavor.advertisedResources().satisfies(advertisedResources)) + .toList(); } /** Returns the flavors of hosts which are eligible and matches the given real resources */ - private List<VespaFlavor> flavorsCompatibleWithReal(NodeResources realResources, boolean exclusive) { + private List<VespaFlavor> flavorsCompatibleWithReal(NodeResources realResources, boolean exactOnly) { return flavors.values().stream() - .filter(flavor -> exclusive - ? flavor.realResources().compatibleWith(realResources) + .filter(flavor -> exactOnly + ? resourcesCalculator.realResourcesOfChildContainer(flavor.advertisedResources(), flavor).compatibleWith(realResources) : flavor.realResources().satisfies(realResources)) - .collect(Collectors.toList()); + .toList(); } } |