From 4c85018009650ea810ed1617f3852fd48b99d2a5 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 8 Dec 2021 15:04:08 +0100 Subject: Don't propagate unspecified resources --- .../autoscale/AllocatableClusterResources.java | 6 ++++-- .../provision/autoscale/AllocationOptimizer.java | 2 -- .../persistence/NodeResourcesSerializer.java | 2 ++ .../hosted/provision/provisioning/Activator.java | 3 ++- .../provision/provisioning/CapacityPolicies.java | 1 + .../provision/provisioning/GroupPreparer.java | 2 +- .../provision/provisioning/NodeCandidate.java | 1 - .../provisioning/NodeRepositoryProvisioner.java | 13 +++++++++++-- .../hosted/provision/provisioning/Preparer.java | 10 ++++------ .../hosted/provision/autoscale/AutoscalingTest.java | 21 +++++++++++++++++++++ .../maintenance/AutoscalingMaintainerTest.java | 16 ++++++++++++++++ .../DynamicProvisioningMaintainerTest.java | 2 +- .../provision/provisioning/ProvisioningTester.java | 1 + 13 files changed, 64 insertions(+), 16 deletions(-) (limited to 'node-repository') 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 index 2755030e2b3..849ea03665b 100644 --- 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 @@ -13,6 +13,7 @@ import com.yahoo.vespa.hosted.provision.provisioning.NodeResourceLimits; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; /** * @author bratseth @@ -141,7 +142,6 @@ public class AllocatableClusterResources { Limits applicationLimits, NodeList hosts, NodeRepository nodeRepository) { - var capacityPolicies = new CapacityPolicies(nodeRepository); var systemLimits = new NodeResourceLimits(nodeRepository); boolean exclusive = clusterSpec.isExclusive(); if ( !clusterSpec.isExclusive() && !nodeRepository.zone().getCloud().dynamicProvisioning()) { @@ -150,7 +150,9 @@ public class AllocatableClusterResources { advertisedResources = systemLimits.enlargeToLegal(advertisedResources, clusterSpec.type(), exclusive); // Ask for something legal advertisedResources = applicationLimits.cap(advertisedResources); // Overrides other conditions, even if it will then fail var realResources = nodeRepository.resourcesCalculator().requestToReal(advertisedResources, exclusive); // What we'll really get - if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type())) return Optional.empty(); + if ( ! systemLimits.isWithinRealLimits(realResources, clusterSpec.type())) + return Optional.empty(); + if (matchesAny(hosts, advertisedResources)) return Optional.of(new AllocatableClusterResources(wantedResources.with(realResources), advertisedResources, 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 d727757b07e..30432c1c078 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 @@ -67,13 +67,11 @@ public class AllocationOptimizer { limits, target, current, clusterModel)); var allocatableResources = AllocatableClusterResources.from(next, current.clusterSpec(), limits, hosts, nodeRepository); - if (allocatableResources.isEmpty()) continue; if (bestAllocation.isEmpty() || allocatableResources.get().preferableTo(bestAllocation.get())) bestAllocation = allocatableResources; } } - return bestAllocation; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java index 8c421443a65..1c3d3f5c489 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeResourcesSerializer.java @@ -20,6 +20,7 @@ public class NodeResourcesSerializer { private static final String storageTypeKey = "storageType"; static void toSlime(NodeResources resources, Cursor resourcesObject) { + if (resources.isUnspecified()) return; resourcesObject.setDouble(vcpuKey, resources.vcpu()); resourcesObject.setDouble(memoryKey, resources.memoryGb()); resourcesObject.setDouble(diskKey, resources.diskGb()); @@ -29,6 +30,7 @@ public class NodeResourcesSerializer { } static NodeResources resourcesFromSlime(Inspector resources) { + if ( ! resources.field(vcpuKey).valid()) return NodeResources.unspecified(); return new NodeResources(resources.field(vcpuKey).asDouble(), resources.field(memoryKey).asDouble(), resources.field(diskKey).asDouble(), diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java index 0d32b21016c..8c358301b85 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java @@ -113,7 +113,8 @@ class Activator { var cluster = modified.cluster(clusterEntry.getKey()).get(); var previousResources = oldNodes.cluster(clusterEntry.getKey()).toResources(); var currentResources = clusterEntry.getValue().toResources(); - if ( ! previousResources.justNumbers().equals(currentResources.justNumbers())) { + if ( previousResources.nodeResources().isUnspecified() + || ! previousResources.justNumbers().equals(currentResources.justNumbers())) { cluster = cluster.with(ScalingEvent.create(previousResources, currentResources, generation, at)); } if (cluster.targetResources().isPresent() diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java index 2b9c5396724..4088d717a67 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java @@ -57,6 +57,7 @@ public class CapacityPolicies { private NodeResources decideNodeResources(NodeResources target, boolean required) { if (required) return target; + if (target.isUnspecified()) return target; // Cannot be modified // Dev does not cap the cpu or network of containers since usage is spotty: Allocate just a small amount exclusively if (zone.environment() == Environment.dev && !zone.getCloud().dynamicProvisioning()) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index ba46f0a9535..2d93763c631 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -71,7 +71,7 @@ public class GroupPreparer { // Try preparing in memory without global unallocated lock. Most of the time there should be no changes and we // can return nodes previously allocated. NodeAllocation probeAllocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, - indices::probeNext, wantedGroups, allNodesAndHosts); + indices::probeNext, wantedGroups, allNodesAndHosts); if (probeAllocation.fulfilledAndNoChanges()) { List acceptedNodes = probeAllocation.finalNodes(); surplusActiveNodes.removeAll(acceptedNodes); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java index 4f0ae688b1c..62ac1f0d0e6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java @@ -238,7 +238,6 @@ public abstract class NodeCandidate implements Nodelike, Comparable loadBalancerProvisioner; public Preparer(NodeRepository nodeRepository, Optional hostProvisioner, Optional loadBalancerProvisioner) { - this.nodeRepository = nodeRepository; this.loadBalancerProvisioner = loadBalancerProvisioner; this.groupPreparer = new GroupPreparer(nodeRepository, hostProvisioner); } @@ -69,9 +66,10 @@ class Preparer { for (int groupIndex = 0; groupIndex < wantedGroups; groupIndex++) { ClusterSpec clusterGroup = cluster.with(Optional.of(ClusterSpec.Group.from(groupIndex))); - GroupPreparer.PrepareResult result = groupPreparer.prepare( - application, clusterGroup, requestedNodes.fraction(wantedGroups), - surplusNodes, indices, wantedGroups, allNodesAndHosts); + GroupPreparer.PrepareResult result = groupPreparer.prepare(application, clusterGroup, + requestedNodes.fraction(wantedGroups), + surplusNodes, indices, wantedGroups, + allNodesAndHosts); allNodesAndHosts = result.allNodesAndHosts; // Might have changed List accepted = result.prepared; if (requestedNodes.rejectNonActiveParent()) { 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 7ade2cdf8c4..1eebcff0903 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 @@ -779,6 +779,27 @@ public class AutoscalingTest { tester.autoscale(application1, cluster1, capacity).target()); } + @Test + public void test_autoscaling_in_dev_with_required_unspecified_resources() { + NodeResources resources = NodeResources.unspecified(); + ClusterResources min = new ClusterResources( 1, 1, resources); + ClusterResources max = new ClusterResources(3, 1, resources); + Capacity capacity = Capacity.from(min, max, true, true); + + AutoscalingTester tester = new AutoscalingTester(Environment.dev, + new NodeResources(10, 16, 100, 2)); + ApplicationId application1 = tester.applicationId("application1"); + ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); + + tester.deploy(application1, cluster1, capacity); + tester.addQueryRateMeasurements(application1, cluster1.id(), + 500, t -> 100.0); + tester.addCpuMeasurements(1.0f, 1f, 10, application1); + tester.assertResources("We scale up even in dev because resources are required", + 3, 1, 1.5, 8, 50, + tester.autoscale(application1, cluster1, capacity).target()); + } + /** * This calculator subtracts the memory tax when forecasting overhead, but not when actually * returning information about nodes. This is allowed because the forecast is a *worst case*. diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java index 6d5677d0911..ac5eec27a9d 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainerTest.java @@ -230,6 +230,22 @@ public class AutoscalingMaintainerTest { .size()); } + @Test + public void test_cd_test_not_specifying_node_resources() { + ApplicationId app1 = AutoscalingMaintainerTester.makeApplicationId("app1"); + ClusterSpec cluster1 = AutoscalingMaintainerTester.containerClusterSpec(); + ClusterResources resources = new ClusterResources( 2, 1, NodeResources.unspecified()); + var capacity = Capacity.from(resources); + var tester = new AutoscalingMaintainerTester(new Zone(SystemName.cd, Environment.prod, RegionName.from("us-east3")), + new MockDeployer.ApplicationContext(app1, cluster1, capacity)); + tester.deploy(app1, cluster1, capacity); // Deploy should succeed and allocate the nodes + assertEquals(2, + tester.nodeRepository().nodes().list(Node.State.active) + .owner(app1) + .cluster(cluster1.id()) + .size()); + } + private void autoscale(boolean down, Duration completionTime, Duration expectedWindow, ManualClock clock, ApplicationId application, ClusterSpec cluster, AutoscalingMaintainerTester tester) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java index 316655e11fb..7ce26354739 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/DynamicProvisioningMaintainerTest.java @@ -458,7 +458,7 @@ public class DynamicProvisioningMaintainerTest { // Provision config servers for (int i = 0; i < provisionedHosts.size(); i++) { - tester.makeReadyChildren(1, i + 1, NodeResources.unspecified(), hostType.childNodeType(), + tester.makeReadyChildren(1, i + 1, new NodeResources(1.5, 8, 50, 0.3), hostType.childNodeType(), provisionedHosts.get(i).hostname(), (nodeIndex) -> "cfg" + nodeIndex); } tester.prepareAndActivateInfraApplication(configSrvApp, hostType.childNodeType()); 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 d1ec1018023..c478840780f 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 @@ -493,6 +493,7 @@ public class ProvisioningTester { public List makeReadyNodes(int n, Flavor flavor, Optional reservedTo, NodeType type, int ipAddressPoolSize, boolean dualStack) { List nodes = makeProvisionedNodes(n, flavor, reservedTo, type, ipAddressPoolSize, dualStack); nodes = nodeRepository.nodes().deallocate(nodes, Agent.system, getClass().getSimpleName()); + nodes.forEach(node -> { if (node.resources().isUnspecified()) throw new IllegalArgumentException(); }); return nodeRepository.nodes().setReady(nodes, Agent.system, getClass().getSimpleName()); } -- cgit v1.2.3