From 270d9fefe61c3780f36764c225a7d19e9115e057 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 20 Aug 2020 11:25:05 +0200 Subject: Preserve allocation when possible We want to avoid unnecessary small adjustments to capacity. Since the worst case real resources we'll get is generally worse than the actual allocation, we cannot guarantee that we compute the same allocation to request as the one leading to the current allocation. So instead of relying on that we explicitly reuse the current (requested) resources when they are still legal instead of recomputing each time. --- .../provision/autoscale/AllocationOptimizer.java | 2 +- .../hosted/provision/autoscale/ResourceTarget.java | 2 +- .../provisioning/NodeRepositoryProvisioner.java | 21 ++- ...ckerProvisioningCompleteHostCalculatorTest.java | 149 +++++++++++++++++++++ .../provisioning/DockerProvisioningTest.java | 34 +++++ .../provision/provisioning/ProvisioningTester.java | 8 +- 6 files changed, 206 insertions(+), 10 deletions(-) create mode 100644 node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningCompleteHostCalculatorTest.java (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java index d2589b15421..e57011b0e4a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java @@ -30,7 +30,7 @@ public class AllocationOptimizer { } /** - * An AllocationSearcher searches the space of possible allocations given a target + * Searches the space of possible allocations given a target * and (optionally) cluster limits and returns the best alternative. * * @return the best allocation, if there are any possible legal allocations, fulfilling the target diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java index f7c2a51436a..bf5e53d823b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ResourceTarget.java @@ -11,7 +11,7 @@ public class ResourceTarget { private final boolean adjustForRedundancy; - /** The target resources per node, assuming the node assignment where this was decided */ + /** The target real resources per node, assuming the node assignment where this was decided */ private final double cpu, memory, disk; private ResourceTarget(double cpu, double memory, double disk, boolean adjustForRedundancy) { 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 227e0744314..b246793096a 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 @@ -158,19 +158,26 @@ public class NodeRepositoryProvisioner implements Provisioner { .not().retired() .not().removable() .asList(); + boolean firstDeployment = nodes.isEmpty(); AllocatableClusterResources currentResources = - nodes.isEmpty() ? new AllocatableClusterResources(requested.minResources(), - clusterSpec.type(), - nodeRepository) // new deployment: Use min - : new AllocatableClusterResources(nodes, nodeRepository); - return within(Limits.of(requested), clusterSpec.isExclusive(), currentResources); + firstDeployment // start at min, preserve current resources otherwise + ? new AllocatableClusterResources(requested.minResources(), clusterSpec.type(), nodeRepository) + : new AllocatableClusterResources(nodes, nodeRepository); + return within(Limits.of(requested), clusterSpec.isExclusive(), currentResources, firstDeployment); } /** Make the minimal adjustments needed to the current resources to stay within the limits */ - private ClusterResources within(Limits limits, boolean exclusive, AllocatableClusterResources current) { - if (limits.isEmpty()) return current.toAdvertisedClusterResources(); + private ClusterResources within(Limits limits, + boolean exclusive, + AllocatableClusterResources current, + boolean firstDeployment) { if (limits.min().equals(limits.max())) return limits.min(); + // Don't change current deployments that are still legal + var currentAsAdvertised = current.toAdvertisedClusterResources(); + if (! firstDeployment && currentAsAdvertised.isWithin(limits.min(), limits.max())) return currentAsAdvertised; + + // Otherwise, find an allocation that preserves the current resources as well as possible return allocationOptimizer.findBestAllocation(ResourceTarget.preserve(current), current, limits, exclusive) .orElseThrow(() -> new IllegalArgumentException("No allocation possible within " + limits)) .toAdvertisedClusterResources(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningCompleteHostCalculatorTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningCompleteHostCalculatorTest.java new file mode 100644 index 00000000000..24cdc5c8fd0 --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningCompleteHostCalculatorTest.java @@ -0,0 +1,149 @@ +// 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.provisioning; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.Capacity; +import com.yahoo.config.provision.ClusterResources; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.Flavor; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.Zone; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeRepository; +import org.junit.Test; + +import java.util.List; + +import static org.junit.Assert.assertEquals; + +/** + * @author bratseth + */ +public class DockerProvisioningCompleteHostCalculatorTest { + + @Test + public void changing_to_different_range_preserves_allocation() { + Flavor hostFlavor = new Flavor(new NodeResources(20, 40, 1000, 4)); + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))) + .resourcesCalculator(new CompleteResourcesCalculator(hostFlavor)) + .flavors(List.of(hostFlavor)) + .build(); + tester.makeReadyHosts(9, hostFlavor.resources()).deployZoneApp(); + + ApplicationId app1 = tester.makeApplicationId("app1"); + ClusterSpec cluster1 = ClusterSpec.request(ClusterSpec.Type.content, new ClusterSpec.Id("cluster1")).vespaVersion("7").build(); + + var initialResources = new NodeResources(2, 16, 50, 1); + tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, initialResources), + new ClusterResources(2, 1, initialResources))); + tester.assertNodes("Initial allocation", + 2, 1, 2, 16, 50, 1.0, + app1, cluster1); + + var newMinResources = new NodeResources(0.5, 4, 11, 1); + var newMaxResources = new NodeResources(2.0, 10, 30, 1); + tester.activate(app1, cluster1, Capacity.from(new ClusterResources(7, 1, newMinResources), + new ClusterResources(7, 1, newMaxResources))); + tester.assertNodes("New allocation preserves total resources", + 7, 1, 0.7, 4.6, 14.3, 1.0, + app1, cluster1); + + System.out.println("---------------------redeploying the same---------------------"); + tester.activate(app1, cluster1, Capacity.from(new ClusterResources(7, 1, newMinResources), + new ClusterResources(7, 1, newMaxResources))); + tester.assertNodes("Redeploying the same ranges does not cause changes", + 7, 1, 0.7, 4.6, 14.3, 1.0, + app1, cluster1); + } + + @Test + public void testResourcesCalculator() { + Flavor hostFlavor = new Flavor(new NodeResources(20, 40, 1000, 4)); + var calculator = new CompleteResourcesCalculator(hostFlavor); + var originalReal = new NodeResources(0.7, 6.0, 12.9, 1.0); + var realToRequest = calculator.realToRequest(originalReal); + var requestToReal = calculator.requestToReal(realToRequest); + var realResourcesOf = calculator.realResourcesOf(realToRequest); + assertEquals(originalReal, requestToReal); + assertEquals(originalReal, realResourcesOf); + } + + private static class CompleteResourcesCalculator implements HostResourcesCalculator { + + private final Flavor hostFlavor; // Has the real resources + private final double memoryOverhead = 1; + private final double diskOverhead = 100; + + public CompleteResourcesCalculator(Flavor hostFlavor) { + this.hostFlavor = hostFlavor; + } + + @Override + public NodeResources realResourcesOf(Node node, NodeRepository nodeRepository) { + if (node.parentHostname().isEmpty()) return node.flavor().resources(); // hosts use configured flavors + return realResourcesOf(node.resources()); + } + + NodeResources realResourcesOf(NodeResources advertisedResources) { + var r = advertisedResources.withMemoryGb(advertisedResources.memoryGb() - + memoryOverhead(advertisedResourcesOf(hostFlavor).memoryGb(), advertisedResources, false)) + .withDiskGb(advertisedResources.diskGb() - + diskOverhead(advertisedResourcesOf(hostFlavor).diskGb(), advertisedResources, false)); + System.out.println(" real given " + advertisedResources + ": " + r); + System.out.println(" adv. given those: " + realToRequest(r)); + return advertisedResources.withMemoryGb(advertisedResources.memoryGb() - + memoryOverhead(advertisedResourcesOf(hostFlavor).memoryGb(), advertisedResources, false)) + .withDiskGb(advertisedResources.diskGb() - + diskOverhead(advertisedResourcesOf(hostFlavor).diskGb(), advertisedResources, false)); + } + + @Override + public NodeResources requestToReal(NodeResources advertisedResources) { + double memoryOverhead = memoryOverhead(advertisedResourcesOf(hostFlavor).memoryGb(), advertisedResources, false); + double diskOverhead = diskOverhead(advertisedResourcesOf(hostFlavor).diskGb(), advertisedResources, false); + return advertisedResources.withMemoryGb(advertisedResources.memoryGb() - memoryOverhead) + .withDiskGb(advertisedResources.diskGb() - diskOverhead); + } + + @Override + public NodeResources advertisedResourcesOf(Flavor flavor) { + if ( ! flavor.equals(hostFlavor)) return flavor.resources(); // Node 'flavors' just wrap the advertised resources + return hostFlavor.resources().withMemoryGb(hostFlavor.resources().memoryGb() + memoryOverhead) + .withDiskGb(hostFlavor.resources().diskGb() + diskOverhead); + } + + @Override + public NodeResources realToRequest(NodeResources realResources) { + double memoryOverhead = memoryOverhead(advertisedResourcesOf(hostFlavor).memoryGb(), realResources, true); + double diskOverhead = diskOverhead(advertisedResourcesOf(hostFlavor).diskGb(), realResources, true); + return realResources.withMemoryGb(realResources.memoryGb() + memoryOverhead) + .withDiskGb(realResources.diskGb() + diskOverhead); + } + + /** + * Returns the memory overhead resulting if the given advertised resources are placed on the given node + * + * @param real true if the given resources are in real values, false if they are in advertised + */ + private double memoryOverhead(double hostAdvertisedMemoryGb, NodeResources resources, boolean real) { + double memoryShare = resources.memoryGb() / + ( hostAdvertisedMemoryGb - (real ? memoryOverhead : 0)); + return memoryOverhead * memoryShare; + } + + /** + * Returns the disk overhead resulting if the given advertised resources are placed on the given node + * + * @param real true if the resources are in real values, false if they are in advertised + */ + private double diskOverhead(double hostAdvertisedDiskGb, NodeResources resources, boolean real) { + double diskShare = resources.diskGb() / + ( hostAdvertisedDiskGb - (real ? diskOverhead : 0) ); + return diskOverhead * diskShare; + } + + } + +} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java index 0c5a682c3c5..cddc1fcb253 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java @@ -322,6 +322,40 @@ public class DockerProvisioningTest { app1, cluster1); } + @Test + public void changing_to_different_range_preserves_allocation() { + Flavor hostFlavor = new Flavor(new NodeResources(20, 40, 100, 4)); + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))) + .resourcesCalculator(3, 0) + .flavors(List.of(hostFlavor)) + .build(); + tester.makeReadyHosts(9, hostFlavor.resources()).deployZoneApp(); + + ApplicationId app1 = tester.makeApplicationId("app1"); + ClusterSpec cluster1 = ClusterSpec.request(ClusterSpec.Type.content, new ClusterSpec.Id("cluster1")).vespaVersion("7").build(); + + var initialResources = new NodeResources(2, 16, 50, 1); + tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, initialResources), + new ClusterResources(2, 1, initialResources))); + tester.assertNodes("Initial allocation", + 2, 1, 2, 16, 50, 1.0, + app1, cluster1); + + var newMinResources = new NodeResources(0.5, 6, 11, 1); + var newMaxResources = new NodeResources(2.0, 10, 30, 1); + tester.activate(app1, cluster1, Capacity.from(new ClusterResources(7, 1, newMinResources), + new ClusterResources(7, 1, newMaxResources))); + tester.assertNodes("New allocation preserves total resources", + 7, 1, 0.7, 6.7, 14.3, 1.0, + app1, cluster1); + + tester.activate(app1, cluster1, Capacity.from(new ClusterResources(7, 1, newMinResources), + new ClusterResources(7, 1, newMaxResources))); + tester.assertNodes("Redeploying does not cause changes", + 7, 1, 0.7, 6.7, 14.3, 1.0, + app1, cluster1); + } + @Test public void too_few_real_resources_causes_failure() { try { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index b814bec683a..d56cae799b2 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -266,7 +266,13 @@ public class ProvisioningTester { for (Node node : nodeList) { var expected = new NodeResources(vcpu, memory, disk, bandwidth, diskSpeed, storageType); assertTrue(explanation + ": Resources: Expected " + expected + " but was " + node.resources(), - expected.compatibleWith(node.resources())); + expected.justNonNumbers().compatibleWith(node.resources().justNonNumbers())); + assertEquals(explanation + ": Vcpu: Expected " + expected.vcpu() + " but was " + node.resources().vcpu(), + expected.vcpu(), node.resources().vcpu(), 0.05); + assertEquals(explanation + ": Memory: Expected " + expected.memoryGb() + " but was " + node.resources().memoryGb(), + expected.memoryGb(), node.resources().memoryGb(), 0.05); + assertEquals(explanation + ": Disk: Expected " + expected.diskGb() + " but was " + node.resources().diskGb(), + expected.diskGb(), node.resources().diskGb(), 0.05); } } -- cgit v1.2.3