diff options
7 files changed, 81 insertions, 28 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java index 22c9234b0c5..dd07b29c2de 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java @@ -31,7 +31,7 @@ public final class ClusterSpec { this.type = type; this.id = id; this.groupId = groupId; - this.vespaVersion = Objects.requireNonNull(vespaVersion); + this.vespaVersion = Objects.requireNonNull(vespaVersion, "vespaVersion cannot be null"); this.exclusive = exclusive; if (type == Type.combined) { if (combinedId.isEmpty()) throw new IllegalArgumentException("combinedId must be set for cluster of type " + type); diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java index e03ebb4d8da..d9a723d69f1 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/NodeResources.java @@ -399,7 +399,7 @@ public class NodeResources { public boolean compatibleWith(NodeResources requested) { if ( ! equal(this.vcpu, requested.vcpu)) return false; if ( ! equal(this.memoryGb, requested.memoryGb)) return false; - if (requested.storageType == StorageType.local) { + if (this.storageType == StorageType.local || requested.storageType == StorageType.local) { if ( ! equal(this.diskGb, requested.diskGb)) return false; } else { 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(); } } |