From 7a8359cda8165694b614b5d12b8335ab388ef07d Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 01:01:39 +0200 Subject: Pull in groupSize --- .../yahoo/vespa/hosted/provision/provisioning/GroupIndices.java | 8 ++++---- .../yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java | 2 +- .../com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java | 3 +++ 3 files changed, 8 insertions(+), 5 deletions(-) (limited to 'node-repository/src') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupIndices.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupIndices.java index 44f371be293..27b31d3bea3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupIndices.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupIndices.java @@ -78,7 +78,7 @@ class GroupIndices { private NodeCandidate moveNodeInSurplusGroup(NodeCandidate node, int[] countInGroup) { var currentGroup = node.allocation().get().membership().cluster().group(); - if (currentGroup.isEmpty()) return node; // Shouldn't happen + if (currentGroup.isEmpty()) return node; if (currentGroup.get().index() < requested.groups()) return node; return inFirstGroupWithDeficiency(node, countInGroup); } @@ -89,7 +89,7 @@ class GroupIndices { if (currentGroup.isEmpty()) return node; if (currentGroup.get().index() >= requested.groups()) return node; if (requested.count().isEmpty()) return node; // Can't retire - if (countInGroup[currentGroup.get().index()] <= requested.count().get() / requested.groups()) return node; + if (countInGroup[currentGroup.get().index()] <= requested.groupSize()) return node; countInGroup[currentGroup.get().index()]--; return node.withNode(node.toNode().retire(Agent.application, clock.instant())); } @@ -101,7 +101,7 @@ class GroupIndices { if (currentGroup.isEmpty()) return node; if (currentGroup.get().index() >= requested.groups()) return node; if (node.preferToRetire() || node.wantToRetire()) return node; - if (requested.count().isPresent() && countInGroup[currentGroup.get().index()] >= requested.count().get() / requested.groups()) return node; + if (requested.count().isPresent() && countInGroup[currentGroup.get().index()] >= requested.groupSize()) return node; node = unretire(node); if (node.allocation().get().membership().retired()) return node; countInGroup[currentGroup.get().index()]++; @@ -110,7 +110,7 @@ class GroupIndices { private NodeCandidate inFirstGroupWithDeficiency(NodeCandidate node, int[] countInGroup) { for (int group = 0; group < requested.groups(); group++) { - if (requested.count().isEmpty() || countInGroup[group] < requested.count().get() / requested.groups()) { + if (requested.count().isEmpty() || countInGroup[group] < requested.groupSize()) { return inGroup(group, node, countInGroup); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index 40e5909d4d9..4110540ab95 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -301,7 +301,7 @@ class NodeAllocation { var group = node.allocation().get().membership().cluster().group(); if (group.isEmpty()) return true; long nodesInGroup = nodes.values().stream().filter(n -> groupOf(n).equals(group)).count(); - return nodesInGroup < requestedNodes.count().get() / requestedNodes.groups(); + return nodesInGroup < requestedNodes.groupSize(); } private Optional groupOf(NodeCandidate candidate) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java index f4b2c4ceee0..cea0608013d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java @@ -40,6 +40,9 @@ public interface NodeSpec { int groups(); + /** Returns the group size requested if count() is present. Throws RuntimeException otherwise. */ + default int groupSize() { return count().get() / groups(); } + /** Returns whether this should throw an exception if the requested nodes are not fully available */ boolean canFail(); -- cgit v1.2.3 From 18f1d126e66e67584baa00f9f3c8817be9bdde77 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 01:14:56 +0200 Subject: Simplify --- .../vespa/hosted/provision/provisioning/Preparer.java | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) (limited to 'node-repository/src') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 25efcabfe8e..734c315e1da 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.List; import java.util.ListIterator; import java.util.Optional; +import java.util.stream.Collectors; /** * Performs preparation of node activation changes for an application. @@ -61,7 +62,6 @@ class Preparer { List usedIndices = clusterNodes.mapToList(node -> node.allocation().get().membership().index()); NodeIndices indices = new NodeIndices(usedIndices); - List acceptedNodes = new ArrayList<>(); GroupPreparer.PrepareResult result = groupPreparer.prepare(application, cluster, requestedNodes, @@ -74,11 +74,8 @@ class Preparer { .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent()) .toList(); } - - replace(acceptedNodes, accepted); moveToActiveGroup(surplusNodes, requestedNodes.groups(), cluster.group()); - acceptedNodes.removeAll(surplusNodes); - return acceptedNodes; + return accepted.stream().filter(node -> ! surplusNodes.contains(node)).collect(Collectors.toList()); } /** Prepare a load balancer for given application and cluster */ @@ -114,15 +111,4 @@ class Preparer { } } - /** - * Nodes are immutable so when changing attributes to the node we create a new instance. - * - * This method is used to both add new nodes and replaces old node references with the new references. - */ - private List replace(List list, List changed) { - list.removeAll(changed); - list.addAll(changed); - return list; - } - } -- cgit v1.2.3 From 2e06511ec5d6f977fa249f887f7eb2055ece7f90 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 08:45:02 +0200 Subject: Simplify --- .../hosted/provision/provisioning/GroupPreparer.java | 15 +++++---------- .../vespa/hosted/provision/provisioning/Preparer.java | 8 ++------ 2 files changed, 7 insertions(+), 16 deletions(-) (limited to 'node-repository/src') 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 e6b47d38779..f8cc3ffd530 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 @@ -35,9 +35,6 @@ public class GroupPreparer { private final NodeRepository nodeRepository; private final Optional hostProvisioner; - /** Contains list of prepared nodes and the {@link LockedNodeList} object to use for next prepare call */ - record PrepareResult(List prepared, LockedNodeList allNodes) {} - public GroupPreparer(NodeRepository nodeRepository, Optional hostProvisioner) { this.nodeRepository = nodeRepository; this.hostProvisioner = hostProvisioner; @@ -53,14 +50,13 @@ public class GroupPreparer { * This method will remove from this list if it finds it needs additional nodes * @param indices the next available node indices for this cluster. * This method will consume these when it allocates new nodes to the cluster. - * @param allNodes list of all nodes and hosts. Use {@link #createUnlockedNodeList()} to create param for - * first invocation. Then use previous {@link PrepareResult#allNodes()} for the following. - * @return the list of nodes this cluster group will have allocated if activated, and + * @param allNodes list of all nodes and hosts + * @return the list of nodes this cluster group will have allocated if activated */ // Note: This operation may make persisted changes to the set of reserved and inactive nodes, // but it may not change the set of active nodes, as the active nodes must stay in sync with the // active config model which is changed on activate - public PrepareResult prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, + public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, List surplusActiveNodes, NodeIndices indices, LockedNodeList allNodes) { log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + @@ -73,12 +69,11 @@ public class GroupPreparer { List acceptedNodes = probeAllocation.finalNodes(); surplusActiveNodes.removeAll(acceptedNodes); indices.commitProbe(); - return new PrepareResult(acceptedNodes, allNodes); + return acceptedNodes; } else { // There were some changes, so re-do the allocation with locks indices.resetProbe(); - List prepared = prepareWithLocks(application, cluster, requestedNodes, surplusActiveNodes, indices); - return new PrepareResult(prepared, createUnlockedNodeList()); + return prepareWithLocks(application, cluster, requestedNodes, surplusActiveNodes, indices); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 734c315e1da..71795fd7393 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -63,13 +63,9 @@ class Preparer { List usedIndices = clusterNodes.mapToList(node -> node.allocation().get().membership().index()); NodeIndices indices = new NodeIndices(usedIndices); - GroupPreparer.PrepareResult result = groupPreparer.prepare(application, cluster, - requestedNodes, - surplusNodes, indices, - allNodes); - List accepted = result.prepared(); + List accepted = groupPreparer.prepare(application, cluster, requestedNodes, surplusNodes, indices, allNodes); if (requestedNodes.rejectNonActiveParent()) { - NodeList activeHosts = result.allNodes().state(Node.State.active).parents().nodeType(requestedNodes.type().hostType()); + NodeList activeHosts = allNodes.state(Node.State.active).parents().nodeType(requestedNodes.type().hostType()); accepted = accepted.stream() .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent()) .toList(); -- cgit v1.2.3 From 7bf12d37859f3bcf32b8851b7cd8f824262cccdf Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 08:47:29 +0200 Subject: Simplify --- .../yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java | 8 ++++---- .../com/yahoo/vespa/hosted/provision/provisioning/Preparer.java | 6 +----- 2 files changed, 5 insertions(+), 9 deletions(-) (limited to 'node-repository/src') 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 f8cc3ffd530..6fbb0c6d268 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 @@ -48,8 +48,6 @@ public class GroupPreparer { * @param requestedNodes a specification of the requested nodes * @param surplusActiveNodes currently active nodes which are available to be assigned to this group. * This method will remove from this list if it finds it needs additional nodes - * @param indices the next available node indices for this cluster. - * This method will consume these when it allocates new nodes to the cluster. * @param allNodes list of all nodes and hosts * @return the list of nodes this cluster group will have allocated if activated */ @@ -57,12 +55,14 @@ public class GroupPreparer { // but it may not change the set of active nodes, as the active nodes must stay in sync with the // active config model which is changed on activate public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, - List surplusActiveNodes, NodeIndices indices, - LockedNodeList allNodes) { + List surplusActiveNodes, LockedNodeList allNodes) { log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + requestedNodes.resources().orElse(NodeResources.unspecified())); // Try preparing in memory without global unallocated lock. Most of the time there should be no changes, // and we can return nodes previously allocated. + List usedIndices = allNodes.cluster(cluster.id()).mapToList(node -> node.allocation().get().membership().index()); + NodeIndices indices = new NodeIndices(usedIndices); + NodeAllocation probeAllocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, indices::probeNext, allNodes); if (probeAllocation.fulfilledAndNoChanges()) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 71795fd7393..4900c0be5a6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -10,7 +10,6 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; -import java.time.Clock; import java.util.ArrayList; import java.util.List; import java.util.ListIterator; @@ -60,10 +59,7 @@ class Preparer { NodeList clusterNodes = allNodes.owner(application); List surplusNodes = findNodesInRemovableGroups(clusterNodes, requestedNodes.groups()); - List usedIndices = clusterNodes.mapToList(node -> node.allocation().get().membership().index()); - NodeIndices indices = new NodeIndices(usedIndices); - - List accepted = groupPreparer.prepare(application, cluster, requestedNodes, surplusNodes, indices, allNodes); + List accepted = groupPreparer.prepare(application, cluster, requestedNodes, surplusNodes, allNodes); if (requestedNodes.rejectNonActiveParent()) { NodeList activeHosts = allNodes.state(Node.State.active).parents().nodeType(requestedNodes.type().hostType()); accepted = accepted.stream() -- cgit v1.2.3 From 6163206dab8a5b0f921eca424f1504e49661372c Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 08:55:26 +0200 Subject: Simplify --- .../hosted/provision/provisioning/GroupPreparer.java | 3 +-- .../hosted/provision/provisioning/NodeIndices.java | 11 ++++++++--- .../vespa/hosted/provision/provisioning/Preparer.java | 18 ++++++++---------- 3 files changed, 17 insertions(+), 15 deletions(-) (limited to 'node-repository/src') 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 6fbb0c6d268..8600be62aa7 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 @@ -60,9 +60,8 @@ public class GroupPreparer { requestedNodes.resources().orElse(NodeResources.unspecified())); // Try preparing in memory without global unallocated lock. Most of the time there should be no changes, // and we can return nodes previously allocated. - List usedIndices = allNodes.cluster(cluster.id()).mapToList(node -> node.allocation().get().membership().index()); - NodeIndices indices = new NodeIndices(usedIndices); + NodeIndices indices = new NodeIndices(cluster.id(), allNodes); NodeAllocation probeAllocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, indices::probeNext, allNodes); if (probeAllocation.fulfilledAndNoChanges()) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeIndices.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeIndices.java index a5a098dbfd6..1ffd54872d3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeIndices.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeIndices.java @@ -1,9 +1,10 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.provisioning; -import java.util.List; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.vespa.hosted.provision.NodeList; -import static java.util.Comparator.naturalOrder; +import java.util.List; /** * Tracks indices of a node cluster, and proposes the index of the next allocation. @@ -18,8 +19,12 @@ class NodeIndices { private int probe; /** Pass the list of current indices in the cluster. */ + NodeIndices(ClusterSpec.Id cluster, NodeList allNodes) { + this(allNodes.cluster(cluster).mapToList(node -> node.allocation().get().membership().index())); + } + NodeIndices(List used) { - this.used = List.copyOf(used); + this.used = used; this.last = -1; this.probe = last; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 4900c0be5a6..a9e2333ea9f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -54,19 +54,17 @@ class Preparer { // Note: This operation may make persisted changes to the set of reserved and inactive nodes, // but it may not change the set of active nodes, as the active nodes must stay in sync with the // active config model which is changed on activate - private List prepareNodes(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes) { + private List prepareNodes(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { LockedNodeList allNodes = groupPreparer.createUnlockedNodeList(); - NodeList clusterNodes = allNodes.owner(application); - List surplusNodes = findNodesInRemovableGroups(clusterNodes, requestedNodes.groups()); - - List accepted = groupPreparer.prepare(application, cluster, requestedNodes, surplusNodes, allNodes); - if (requestedNodes.rejectNonActiveParent()) { - NodeList activeHosts = allNodes.state(Node.State.active).parents().nodeType(requestedNodes.type().hostType()); + List surplusNodes = findNodesInRemovableGroups(application, requested.groups(), allNodes); + List accepted = groupPreparer.prepare(application, cluster, requested, surplusNodes, allNodes); + if (requested.rejectNonActiveParent()) { // TODO: Move to NodeAllocation + NodeList activeHosts = allNodes.state(Node.State.active).parents().nodeType(requested.type().hostType()); accepted = accepted.stream() .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent()) .toList(); } - moveToActiveGroup(surplusNodes, requestedNodes.groups(), cluster.group()); + moveToActiveGroup(surplusNodes, requested.groups(), cluster.group()); return accepted.stream().filter(node -> ! surplusNodes.contains(node)).collect(Collectors.toList()); } @@ -79,9 +77,9 @@ class Preparer { * Returns a list of the nodes which are * in groups with index number above or equal the group count */ - private List findNodesInRemovableGroups(NodeList clusterNodes, int wantedGroups) { + private List findNodesInRemovableGroups(ApplicationId application, int wantedGroups, NodeList allNodes) { List surplusNodes = new ArrayList<>(); - for (Node node : clusterNodes.state(Node.State.active)) { + for (Node node : allNodes.owner(application).state(Node.State.active)) { ClusterSpec nodeCluster = node.allocation().get().membership().cluster(); if (nodeCluster.group().get().index() >= wantedGroups) surplusNodes.add(node); -- cgit v1.2.3 From 54c45551d4deeb423e254e0f721806fd6b7f89c5 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 08:58:02 +0200 Subject: Remove duplicate of processing done by GroupIndices --- .../vespa/hosted/provision/provisioning/Preparer.java | 17 ----------------- 1 file changed, 17 deletions(-) (limited to 'node-repository/src') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index a9e2333ea9f..5b2578d7a12 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.ClusterMembership; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeAllocationException; import com.yahoo.vespa.hosted.provision.LockedNodeList; @@ -12,7 +11,6 @@ import com.yahoo.vespa.hosted.provision.NodeRepository; import java.util.ArrayList; import java.util.List; -import java.util.ListIterator; import java.util.Optional; import java.util.stream.Collectors; @@ -64,7 +62,6 @@ class Preparer { .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent()) .toList(); } - moveToActiveGroup(surplusNodes, requested.groups(), cluster.group()); return accepted.stream().filter(node -> ! surplusNodes.contains(node)).collect(Collectors.toList()); } @@ -87,18 +84,4 @@ class Preparer { return surplusNodes; } - /** Move nodes from unwanted groups to wanted groups to avoid lingering groups consisting of retired nodes */ - private void moveToActiveGroup(List surplusNodes, int wantedGroups, Optional targetGroup) { - for (ListIterator i = surplusNodes.listIterator(); i.hasNext(); ) { - Node node = i.next(); - ClusterMembership membership = node.allocation().get().membership(); - ClusterSpec cluster = membership.cluster(); - if (cluster.group().get().index() >= wantedGroups) { - ClusterSpec.Group newGroup = targetGroup.orElse(ClusterSpec.Group.from(0)); - ClusterMembership newGroupMembership = membership.with(cluster.with(Optional.of(newGroup))); - i.set(node.with(node.allocation().get().with(newGroupMembership))); - } - } - } - } -- cgit v1.2.3 From 746370b7ee46d6c11d78e81435b475dc415c4552 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 09:06:04 +0200 Subject: Remove unnecessary tracking of surplus nodes --- .../maintenance/HostCapacityMaintainer.java | 2 +- .../provision/provisioning/GroupPreparer.java | 25 +++++++--------------- .../provision/provisioning/NodePrioritizer.java | 16 +------------- .../hosted/provision/provisioning/Preparer.java | 20 ++--------------- 4 files changed, 12 insertions(+), 51 deletions(-) (limited to 'node-repository/src') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java index 331759127e4..8213286639c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/HostCapacityMaintainer.java @@ -273,7 +273,7 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { NodePrioritizer prioritizer = new NodePrioritizer(allNodes, applicationId, clusterSpec, nodeSpec, true, nodeRepository().nameResolver(), nodeRepository().nodes(), nodeRepository().resourcesCalculator(), nodeRepository().spareCount(), nodeSpec.cloudAccount().isExclave(nodeRepository().zone())); - List nodeCandidates = prioritizer.collect(List.of()); + List nodeCandidates = prioritizer.collect(); MutableInteger index = new MutableInteger(0); return nodeCandidates .stream() 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 8600be62aa7..283ce5f88c2 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 @@ -46,33 +46,28 @@ public class GroupPreparer { * @param application the application we are allocating to * @param cluster the cluster and group we are allocating to * @param requestedNodes a specification of the requested nodes - * @param surplusActiveNodes currently active nodes which are available to be assigned to this group. - * This method will remove from this list if it finds it needs additional nodes * @param allNodes list of all nodes and hosts * @return the list of nodes this cluster group will have allocated if activated */ // Note: This operation may make persisted changes to the set of reserved and inactive nodes, // but it may not change the set of active nodes, as the active nodes must stay in sync with the // active config model which is changed on activate - public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, - List surplusActiveNodes, LockedNodeList allNodes) { + public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, LockedNodeList allNodes) { log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + requestedNodes.resources().orElse(NodeResources.unspecified())); // Try preparing in memory without global unallocated lock. Most of the time there should be no changes, // and we can return nodes previously allocated. NodeIndices indices = new NodeIndices(cluster.id(), allNodes); - NodeAllocation probeAllocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, - indices::probeNext, allNodes); + NodeAllocation probeAllocation = prepareAllocation(application, cluster, requestedNodes, indices::probeNext, allNodes); if (probeAllocation.fulfilledAndNoChanges()) { List acceptedNodes = probeAllocation.finalNodes(); - surplusActiveNodes.removeAll(acceptedNodes); indices.commitProbe(); return acceptedNodes; } else { // There were some changes, so re-do the allocation with locks indices.resetProbe(); - return prepareWithLocks(application, cluster, requestedNodes, surplusActiveNodes, indices); + return prepareWithLocks(application, cluster, requestedNodes, indices); } } @@ -80,13 +75,11 @@ public class GroupPreparer { LockedNodeList createUnlockedNodeList() { return nodeRepository.nodes().list(PROBE_LOCK); } /// Note that this will write to the node repo. - private List prepareWithLocks(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, - List surplusActiveNodes, NodeIndices indices) { + private List prepareWithLocks(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, NodeIndices indices) { try (Mutex lock = nodeRepository.applications().lock(application); Mutex allocationLock = nodeRepository.nodes().lockUnallocated()) { LockedNodeList allNodes = nodeRepository.nodes().list(allocationLock); - NodeAllocation allocation = prepareAllocation(application, cluster, requestedNodes, surplusActiveNodes, - indices::next, allNodes); + NodeAllocation allocation = prepareAllocation(application, cluster, requestedNodes, indices::next, allNodes); NodeType hostType = allocation.nodeType().hostType(); if (canProvisionDynamically(hostType) && allocation.hostDeficit().isPresent()) { HostSharing sharing = hostSharing(cluster, hostType); @@ -128,7 +121,7 @@ public class GroupPreparer { // Non-dynamically provisioned zone with a deficit because we just now retired some nodes. // Try again, but without retiring indices.resetProbe(); - List accepted = prepareWithLocks(application, cluster, cns.withoutRetiring(), surplusActiveNodes, indices); + List accepted = prepareWithLocks(application, cluster, cns.withoutRetiring(), indices); log.warning("Prepared " + application + " " + cluster.id() + " without retirement due to lack of capacity"); return accepted; } @@ -140,14 +133,12 @@ public class GroupPreparer { List acceptedNodes = allocation.finalNodes(); nodeRepository.nodes().reserve(allocation.reservableNodes()); nodeRepository.nodes().addReservedNodes(new LockedNodeList(allocation.newNodes(), allocationLock)); - surplusActiveNodes.removeAll(acceptedNodes); return acceptedNodes; } } private NodeAllocation prepareAllocation(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, - List surplusActiveNodes, Supplier nextIndex, - LockedNodeList allNodes) { + Supplier nextIndex, LockedNodeList allNodes) { NodeAllocation allocation = new NodeAllocation(allNodes, application, cluster, requestedNodes, nextIndex, nodeRepository); NodePrioritizer prioritizer = new NodePrioritizer(allNodes, @@ -160,7 +151,7 @@ public class GroupPreparer { nodeRepository.resourcesCalculator(), nodeRepository.spareCount(), requestedNodes.cloudAccount().isExclave(nodeRepository.zone())); - allocation.offer(prioritizer.collect(surplusActiveNodes)); + allocation.offer(prioritizer.collect()); return allocation; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java index 9f00e5fdbba..3c33897af57 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java @@ -81,9 +81,8 @@ public class NodePrioritizer { } /** Collects all node candidates for this application and returns them in the most-to-least preferred order */ - public List collect(List surplusActiveNodes) { + public List collect() { addApplicationNodes(); - addSurplusNodes(surplusActiveNodes); addReadyNodes(); addCandidatesOnExistingHosts(); return prioritize(); @@ -115,19 +114,6 @@ public class NodePrioritizer { return nodes; } - /** - * Add nodes that have been previously reserved to the same application from - * an earlier downsizing of a cluster - */ - private void addSurplusNodes(List surplusNodes) { - for (Node node : surplusNodes) { - NodeCandidate candidate = candidateFrom(node, true); - if (!candidate.violatesSpares || canAllocateToSpareHosts) { - candidates.add(candidate); - } - } - } - /** Add a node on each host with enough capacity for the requested flavor */ private void addCandidatesOnExistingHosts() { if (requestedNodes.resources().isEmpty()) return; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 5b2578d7a12..e4a7e0326e2 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.provision.NodeRepository; import java.util.ArrayList; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; /** * Performs preparation of node activation changes for an application. @@ -54,15 +53,14 @@ class Preparer { // active config model which is changed on activate private List prepareNodes(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { LockedNodeList allNodes = groupPreparer.createUnlockedNodeList(); - List surplusNodes = findNodesInRemovableGroups(application, requested.groups(), allNodes); - List accepted = groupPreparer.prepare(application, cluster, requested, surplusNodes, allNodes); + List accepted = groupPreparer.prepare(application, cluster, requested, allNodes); if (requested.rejectNonActiveParent()) { // TODO: Move to NodeAllocation NodeList activeHosts = allNodes.state(Node.State.active).parents().nodeType(requested.type().hostType()); accepted = accepted.stream() .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent()) .toList(); } - return accepted.stream().filter(node -> ! surplusNodes.contains(node)).collect(Collectors.toList()); + return new ArrayList<>(accepted); } /** Prepare a load balancer for given application and cluster */ @@ -70,18 +68,4 @@ class Preparer { loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requestedNodes)); } - /** - * Returns a list of the nodes which are - * in groups with index number above or equal the group count - */ - private List findNodesInRemovableGroups(ApplicationId application, int wantedGroups, NodeList allNodes) { - List surplusNodes = new ArrayList<>(); - for (Node node : allNodes.owner(application).state(Node.State.active)) { - ClusterSpec nodeCluster = node.allocation().get().membership().cluster(); - if (nodeCluster.group().get().index() >= wantedGroups) - surplusNodes.add(node); - } - return surplusNodes; - } - } -- cgit v1.2.3 From a73e7075523aaf2c9fd999da72fc42edc58466e5 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 10:41:44 +0200 Subject: Move filtering by inactive parent host down --- .../provision/provisioning/GroupPreparer.java | 34 ++++++++----- .../provision/provisioning/NodeAllocation.java | 55 ++++++++++++---------- .../provisioning/NodeRepositoryProvisioner.java | 1 - .../hosted/provision/provisioning/Preparer.java | 6 --- 4 files changed, 50 insertions(+), 46 deletions(-) (limited to 'node-repository/src') 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 283ce5f88c2..307ff937ddf 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 @@ -10,6 +10,7 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.provision.LockedNodeList; import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner.HostSharing; @@ -45,21 +46,21 @@ public class GroupPreparer { * * @param application the application we are allocating to * @param cluster the cluster and group we are allocating to - * @param requestedNodes a specification of the requested nodes + * @param requested a specification of the requested nodes * @param allNodes list of all nodes and hosts * @return the list of nodes this cluster group will have allocated if activated */ // Note: This operation may make persisted changes to the set of reserved and inactive nodes, // but it may not change the set of active nodes, as the active nodes must stay in sync with the // active config model which is changed on activate - public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, LockedNodeList allNodes) { + public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested, LockedNodeList allNodes) { log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + - requestedNodes.resources().orElse(NodeResources.unspecified())); + requested.resources().orElse(NodeResources.unspecified())); // Try preparing in memory without global unallocated lock. Most of the time there should be no changes, // and we can return nodes previously allocated. NodeIndices indices = new NodeIndices(cluster.id(), allNodes); - NodeAllocation probeAllocation = prepareAllocation(application, cluster, requestedNodes, indices::probeNext, allNodes); + NodeAllocation probeAllocation = prepareAllocation(application, cluster, requested, indices::probeNext, allNodes); if (probeAllocation.fulfilledAndNoChanges()) { List acceptedNodes = probeAllocation.finalNodes(); indices.commitProbe(); @@ -67,7 +68,7 @@ public class GroupPreparer { } else { // There were some changes, so re-do the allocation with locks indices.resetProbe(); - return prepareWithLocks(application, cluster, requestedNodes, indices); + return prepareWithLocks(application, cluster, requested, indices); } } @@ -75,11 +76,11 @@ public class GroupPreparer { LockedNodeList createUnlockedNodeList() { return nodeRepository.nodes().list(PROBE_LOCK); } /// Note that this will write to the node repo. - private List prepareWithLocks(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, NodeIndices indices) { + private List prepareWithLocks(ApplicationId application, ClusterSpec cluster, NodeSpec requested, NodeIndices indices) { try (Mutex lock = nodeRepository.applications().lock(application); Mutex allocationLock = nodeRepository.nodes().lockUnallocated()) { LockedNodeList allNodes = nodeRepository.nodes().list(allocationLock); - NodeAllocation allocation = prepareAllocation(application, cluster, requestedNodes, indices::next, allNodes); + NodeAllocation allocation = prepareAllocation(application, cluster, requested, indices::next, allNodes); NodeType hostType = allocation.nodeType().hostType(); if (canProvisionDynamically(hostType) && allocation.hostDeficit().isPresent()) { HostSharing sharing = hostSharing(cluster, hostType); @@ -87,13 +88,13 @@ public class GroupPreparer { NodeAllocation.HostDeficit deficit = allocation.hostDeficit().get(); List hosts = new ArrayList<>(); Consumer> whenProvisioned = provisionedHosts -> { - hosts.addAll(provisionedHosts.stream().map(host -> host.generateHost(requestedNodes.hostTTL())).toList()); + hosts.addAll(provisionedHosts.stream().map(host -> host.generateHost(requested.hostTTL())).toList()); nodeRepository.nodes().addNodes(hosts, Agent.application); // Offer the nodes on the newly provisioned hosts, this should be enough to cover the deficit List candidates = provisionedHosts.stream() .map(host -> NodeCandidate.createNewExclusiveChild(host.generateNode(), - host.generateHost(requestedNodes.hostTTL()))) + host.generateHost(requested.hostTTL()))) .toList(); allocation.offer(candidates); }; @@ -106,7 +107,7 @@ public class GroupPreparer { sharing, Optional.of(cluster.type()), Optional.of(cluster.id()), - requestedNodes.cloudAccount(), + requested.cloudAccount(), deficit.dueToFlavorUpgrade()); hostProvisioner.get().provisionHosts(request, whenProvisioned); } catch (NodeAllocationException e) { @@ -116,8 +117,8 @@ public class GroupPreparer { hosts.forEach(host -> nodeRepository.nodes().deprovision(host.hostname(), Agent.system, nodeRepository.clock().instant())); throw e; } - } else if (allocation.hostDeficit().isPresent() && requestedNodes.canFail() && - allocation.hasRetiredJustNow() && requestedNodes instanceof NodeSpec.CountNodeSpec cns) { + } else if (allocation.hostDeficit().isPresent() && requested.canFail() && + allocation.hasRetiredJustNow() && requested instanceof NodeSpec.CountNodeSpec cns) { // Non-dynamically provisioned zone with a deficit because we just now retired some nodes. // Try again, but without retiring indices.resetProbe(); @@ -126,13 +127,20 @@ public class GroupPreparer { return accepted; } - if (! allocation.fulfilled() && requestedNodes.canFail()) + if (! allocation.fulfilled() && requested.canFail()) throw new NodeAllocationException(allocation.allocationFailureDetails(), true); // Carry out and return allocation List acceptedNodes = allocation.finalNodes(); nodeRepository.nodes().reserve(allocation.reservableNodes()); nodeRepository.nodes().addReservedNodes(new LockedNodeList(allocation.newNodes(), allocationLock)); + + if (requested.rejectNonActiveParent()) { // TODO: Move into offer() - currently this must be done *after* reserving + NodeList activeHosts = allNodes.state(Node.State.active).parents().nodeType(requested.type().hostType()); + acceptedNodes = acceptedNodes.stream() + .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent()) + .toList(); + } return acceptedNodes; } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index 4110540ab95..45dd12c7bde 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -51,7 +51,7 @@ class NodeAllocation { private final ClusterSpec cluster; /** The requested nodes of this list */ - private final NodeSpec requestedNodes; + private final NodeSpec requested; /** The node candidates this has accepted so far, keyed on hostname */ private final Map nodes = new LinkedHashMap<>(); @@ -86,12 +86,12 @@ class NodeAllocation { private final NodeResourceLimits nodeResourceLimits; private final Optional requiredHostFlavor; - NodeAllocation(NodeList allNodes, ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, + NodeAllocation(NodeList allNodes, ApplicationId application, ClusterSpec cluster, NodeSpec requested, Supplier nextIndex, NodeRepository nodeRepository) { this.allNodes = allNodes; this.application = application; this.cluster = cluster; - this.requestedNodes = requestedNodes; + this.requested = requested; this.nextIndex = nextIndex; this.nodeRepository = nodeRepository; this.nodeResourceLimits = new NodeResourceLimits(nodeRepository); @@ -122,11 +122,11 @@ class NodeAllocation { if ( candidate.state() == Node.State.active && allocation.removable()) continue; // don't accept; causes removal if ( candidate.state() == Node.State.active && candidate.wantToFail()) continue; // don't accept; causes failing if ( indexes.contains(membership.index())) continue; // duplicate index (just to be sure) - if (nodeRepository.zone().cloud().allowEnclave() && candidate.parent.isPresent() && ! candidate.parent.get().cloudAccount().equals(requestedNodes.cloudAccount())) continue; // wrong account + if (nodeRepository.zone().cloud().allowEnclave() && candidate.parent.isPresent() && ! candidate.parent.get().cloudAccount().equals(requested.cloudAccount())) continue; // wrong account - boolean resizeable = requestedNodes.considerRetiring() && candidate.isResizable; + boolean resizeable = requested.considerRetiring() && candidate.isResizable; - if ((! saturated() && hasCompatibleResources(candidate) && requestedNodes.acceptable(candidate)) || acceptIncompatible(candidate)) { + if ((! saturated() && hasCompatibleResources(candidate) && requested.acceptable(candidate)) || acceptIncompatible(candidate)) { candidate = candidate.withNode(); if (candidate.isValid()) acceptNode(candidate, shouldRetire(candidate, candidates), resizeable); @@ -150,7 +150,7 @@ class NodeAllocation { } candidate = candidate.allocate(application, ClusterMembership.from(cluster, nextIndex.get()), - requestedNodes.resources().orElse(candidate.resources()), + requested.resources().orElse(candidate.resources()), nodeRepository.clock().instant()); if (candidate.isValid()) { acceptNode(candidate, Retirement.none, false); @@ -161,7 +161,7 @@ class NodeAllocation { /** Returns the cause of retirement for given candidate */ private Retirement shouldRetire(NodeCandidate candidate, List candidates) { - if ( ! requestedNodes.considerRetiring()) { + if ( ! requested.considerRetiring()) { boolean alreadyRetired = candidate.allocation().map(a -> a.membership().retired()).orElse(false); return alreadyRetired ? Retirement.alreadyRetired : Retirement.none; } @@ -199,7 +199,7 @@ class NodeAllocation { private boolean violatesExclusivity(NodeCandidate candidate) { if (candidate.parentHostname().isEmpty()) return false; - if (requestedNodes.type() != NodeType.tenant) return false; + if (requested.type() != NodeType.tenant) return false; // In zones which does not allow host sharing, exclusivity is violated if... if ( ! nodeRepository.zone().cloud().allowHostSharing()) { @@ -244,20 +244,20 @@ class NodeAllocation { if (candidate.state() != Node.State.active) return false; if (candidate.allocation().get().membership().retired()) return true; // don't second-guess if already retired - if ( ! requestedNodes.considerRetiring()) // the node is active and we are not allowed to remove gracefully, so keep + if ( ! requested.considerRetiring()) // the node is active and we are not allowed to remove gracefully, so keep return true; return cluster.isStateful() || (cluster.type() == ClusterSpec.Type.container && !hasCompatibleResources(candidate)); } private boolean hasCompatibleResources(NodeCandidate candidate) { - return requestedNodes.isCompatible(candidate.resources()) || candidate.isResizable; + return requested.isCompatible(candidate.resources()) || candidate.isResizable; } private Node acceptNode(NodeCandidate candidate, Retirement retirement, boolean resizeable) { Node node = candidate.toNode(); if (node.allocation().isPresent()) // Record the currently requested resources - node = node.with(node.allocation().get().withRequestedResources(requestedNodes.resources().orElse(node.resources()))); + node = node.with(node.allocation().get().withRequestedResources(requested.resources().orElse(node.resources()))); if (retirement == Retirement.none) { @@ -265,7 +265,7 @@ class NodeAllocation { // for the purpose of deciding when to stop accepting nodes (saturation) if (node.allocation().isEmpty() || (canBeUsedInGroupWithDeficiency(node) && - ! ( requestedNodes.needsResize(node) && (node.allocation().get().membership().retired() || ! requestedNodes.considerRetiring())))) { + ! (requested.needsResize(node) && (node.allocation().get().membership().retired() || ! requested.considerRetiring())))) { acceptedAndCompatible++; } @@ -296,12 +296,12 @@ class NodeAllocation { } private boolean canBeUsedInGroupWithDeficiency(Node node) { - if (requestedNodes.count().isEmpty()) return true; + if (requested.count().isEmpty()) return true; if (node.allocation().isEmpty()) return true; var group = node.allocation().get().membership().cluster().group(); if (group.isEmpty()) return true; long nodesInGroup = nodes.values().stream().filter(n -> groupOf(n).equals(group)).count(); - return nodesInGroup < requestedNodes.groupSize(); + return nodesInGroup < requested.groupSize(); } private Optional groupOf(NodeCandidate candidate) { @@ -310,10 +310,10 @@ class NodeAllocation { private Node resize(Node node) { NodeResources hostResources = allNodes.parentOf(node).get().flavor().resources(); - return node.with(new Flavor(requestedNodes.resources().get() - .with(hostResources.diskSpeed()) - .with(hostResources.storageType()) - .with(hostResources.architecture())), + return node.with(new Flavor(requested.resources().get() + .with(hostResources.diskSpeed()) + .with(hostResources.storageType()) + .with(hostResources.architecture())), Agent.application, nodeRepository.clock().instant()); } @@ -324,12 +324,12 @@ class NodeAllocation { /** Returns true if no more nodes are needed in this list */ public boolean saturated() { - return requestedNodes.saturatedBy(acceptedAndCompatible); + return requested.saturatedBy(acceptedAndCompatible); } /** Returns true if the content of this list is sufficient to meet the request */ boolean fulfilled() { - return requestedNodes.fulfilledBy(acceptedAndCompatibleOrResizable()); + return requested.fulfilledBy(acceptedAndCompatibleOrResizable()); } /** Returns true if this allocation was already fulfilled and resulted in no new changes */ @@ -352,10 +352,10 @@ class NodeAllocation { if (nodeType().isHost()) { return Optional.empty(); // Hosts are provisioned as required by the child application } - int deficit = requestedNodes.fulfilledDeficitCount(acceptedAndCompatibleOrResizable()); + int deficit = requested.fulfilledDeficitCount(acceptedAndCompatibleOrResizable()); // We can only require flavor upgrade if the entire deficit is caused by upgrades boolean dueToFlavorUpgrade = deficit == wasRetiredDueToFlavorUpgrade; - return Optional.of(new HostDeficit(requestedNodes.resources().orElseGet(NodeResources::unspecified), + return Optional.of(new HostDeficit(requested.resources().orElseGet(NodeResources::unspecified), deficit, dueToFlavorUpgrade)) .filter(hostDeficit -> hostDeficit.count() > 0); @@ -364,7 +364,7 @@ class NodeAllocation { /** Returns the indices to use when provisioning hosts for this */ List provisionIndices(int count) { if (count < 1) throw new IllegalArgumentException("Count must be positive"); - NodeType hostType = requestedNodes.type().hostType(); + NodeType hostType = requested.type().hostType(); // Tenant hosts have a continuously increasing index if (hostType == NodeType.host) return nodeRepository.database().readProvisionIndices(count); @@ -398,7 +398,7 @@ class NodeAllocation { /** The node type this is allocating */ NodeType nodeType() { - return requestedNodes.type(); + return requested.type(); } List finalNodes() { @@ -411,10 +411,13 @@ class NodeAllocation { nodes.put(candidate.toNode().hostname(), candidate); } - GroupIndices groupIndices = new GroupIndices(requestedNodes, allNodes, nodeRepository.clock()); + // Place in groups + GroupIndices groupIndices = new GroupIndices(requested, allNodes, nodeRepository.clock()); Collection finalNodes = groupIndices.assignTo(nodes.values()); nodes.clear(); finalNodes.forEach(candidate -> nodes.put(candidate.toNode().hostname(), candidate)); + + // Set cluster ID and index return finalNodes.stream().map(NodeCandidate::toNode).toList(); } 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 c29c51ccbd5..ebaa8a1b9c4 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 @@ -86,7 +86,6 @@ public class NodeRepositoryProvisioner implements Provisioner { " for application " + application + ", cluster " + cluster); validate(application, cluster, requested, logger); - int groups; NodeResources resources; NodeSpec nodeSpec; if (requested.type() == NodeType.tenant) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index e4a7e0326e2..d30323063cb 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -54,12 +54,6 @@ class Preparer { private List prepareNodes(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { LockedNodeList allNodes = groupPreparer.createUnlockedNodeList(); List accepted = groupPreparer.prepare(application, cluster, requested, allNodes); - if (requested.rejectNonActiveParent()) { // TODO: Move to NodeAllocation - NodeList activeHosts = allNodes.state(Node.State.active).parents().nodeType(requested.type().hostType()); - accepted = accepted.stream() - .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent()) - .toList(); - } return new ArrayList<>(accepted); } -- cgit v1.2.3 From 20a4805e4289db5914a7d846467e5b8c5f55f122 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 10:44:18 +0200 Subject: Simplify --- .../provisioning/NodeRepositoryProvisioner.java | 1 + .../vespa/hosted/provision/provisioning/Preparer.java | 16 +++++++--------- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'node-repository/src') 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 ebaa8a1b9c4..43b8cd08989 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 @@ -238,6 +238,7 @@ public class NodeRepositoryProvisioner implements Provisioner { } private List asSortedHosts(List nodes, NodeResources requestedResources) { + nodes = new ArrayList<>(nodes); nodes.sort(Comparator.comparingInt(node -> node.allocation().get().membership().index())); List hosts = new ArrayList<>(nodes.size()); for (Node node : nodes) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index d30323063cb..0617eaff9d9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -30,14 +30,14 @@ class Preparer { } /** Prepare all required resources for the given application and cluster */ - public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes) { + public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { try { - var nodes = prepareNodes(application, cluster, requestedNodes); - prepareLoadBalancer(application, cluster, requestedNodes); + var nodes = prepareNodes(application, cluster, requested); + prepareLoadBalancer(application, cluster, requested); return nodes; } catch (NodeAllocationException e) { - throw new NodeAllocationException("Could not satisfy " + requestedNodes + + throw new NodeAllocationException("Could not satisfy " + requested + " in " + application + " " + cluster, e, e.retryable()); } @@ -52,14 +52,12 @@ class Preparer { // but it may not change the set of active nodes, as the active nodes must stay in sync with the // active config model which is changed on activate private List prepareNodes(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { - LockedNodeList allNodes = groupPreparer.createUnlockedNodeList(); - List accepted = groupPreparer.prepare(application, cluster, requested, allNodes); - return new ArrayList<>(accepted); + return groupPreparer.prepare(application, cluster, requested, groupPreparer.createUnlockedNodeList()); } /** Prepare a load balancer for given application and cluster */ - public void prepareLoadBalancer(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes) { - loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requestedNodes)); + public void prepareLoadBalancer(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { + loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requested)); } } -- cgit v1.2.3 From 393487787afc13a88067d0e0e07711b7587b7bfe Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 10:46:32 +0200 Subject: Simplify --- .../hosted/provision/provisioning/Preparer.java | 33 ++++++++-------------- 1 file changed, 11 insertions(+), 22 deletions(-) (limited to 'node-repository/src') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 0617eaff9d9..6dab216e911 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -29,35 +29,24 @@ class Preparer { this.groupPreparer = new GroupPreparer(nodeRepository, hostProvisioner); } - /** Prepare all required resources for the given application and cluster */ + /** + * Prepare all required resources for the given application and cluster. + * + * @return the list of nodes this cluster will have allocated if activated + */ + // Note: This may make persisted changes to the set of reserved and inactive nodes, + // but it may not change the set of active nodes, as the active nodes must stay in sync with the + // active config model which is changed on activate public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { try { - var nodes = prepareNodes(application, cluster, requested); - prepareLoadBalancer(application, cluster, requested); + var nodes = groupPreparer.prepare(application, cluster, requested, groupPreparer.createUnlockedNodeList()); + loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requested)); return nodes; } catch (NodeAllocationException e) { - throw new NodeAllocationException("Could not satisfy " + requested + - " in " + application + " " + cluster, e, + throw new NodeAllocationException("Could not satisfy " + requested + " in " + application + " " + cluster, e, e.retryable()); } } - /** - * Ensure sufficient nodes are reserved or active for the given application and cluster - * - * @return the list of nodes this cluster will have allocated if activated - */ - // Note: This operation may make persisted changes to the set of reserved and inactive nodes, - // but it may not change the set of active nodes, as the active nodes must stay in sync with the - // active config model which is changed on activate - private List prepareNodes(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { - return groupPreparer.prepare(application, cluster, requested, groupPreparer.createUnlockedNodeList()); - } - - /** Prepare a load balancer for given application and cluster */ - public void prepareLoadBalancer(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { - loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requested)); - } - } -- cgit v1.2.3 From db3d8de32915fa6a5ab75ca3f29bf579ede09bcc Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 10:48:03 +0200 Subject: Simplify --- .../java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'node-repository/src') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 6dab216e911..6ea59edc37d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -39,9 +39,8 @@ class Preparer { // active config model which is changed on activate public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { try { - var nodes = groupPreparer.prepare(application, cluster, requested, groupPreparer.createUnlockedNodeList()); loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requested)); - return nodes; + return groupPreparer.prepare(application, cluster, requested, groupPreparer.createUnlockedNodeList()); } catch (NodeAllocationException e) { throw new NodeAllocationException("Could not satisfy " + requested + " in " + application + " " + cluster, e, -- cgit v1.2.3 From 63dc331187f9d0f2a3570552edaff2c6e3536bcf Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 11:22:24 +0200 Subject: Simplify --- .../vespa/hosted/provision/provisioning/GroupPreparer.java | 11 ++++++++--- .../vespa/hosted/provision/provisioning/NodeAllocation.java | 2 +- .../yahoo/vespa/hosted/provision/provisioning/Preparer.java | 13 ++----------- 3 files changed, 11 insertions(+), 15 deletions(-) (limited to 'node-repository/src') 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 307ff937ddf..ff24175f2a0 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 @@ -35,10 +35,12 @@ public class GroupPreparer { private final NodeRepository nodeRepository; private final Optional hostProvisioner; + private final Optional loadBalancerProvisioner; - public GroupPreparer(NodeRepository nodeRepository, Optional hostProvisioner) { + public GroupPreparer(NodeRepository nodeRepository, Optional hostProvisioner, Optional loadBalancerProvisioner) { this.nodeRepository = nodeRepository; this.hostProvisioner = hostProvisioner; + this.loadBalancerProvisioner = loadBalancerProvisioner; } /** @@ -56,9 +58,11 @@ public class GroupPreparer { public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested, LockedNodeList allNodes) { log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + requested.resources().orElse(NodeResources.unspecified())); + + loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requested)); + // Try preparing in memory without global unallocated lock. Most of the time there should be no changes, // and we can return nodes previously allocated. - NodeIndices indices = new NodeIndices(cluster.id(), allNodes); NodeAllocation probeAllocation = prepareAllocation(application, cluster, requested, indices::probeNext, allNodes); if (probeAllocation.fulfilledAndNoChanges()) { @@ -128,7 +132,8 @@ public class GroupPreparer { } if (! allocation.fulfilled() && requested.canFail()) - throw new NodeAllocationException(allocation.allocationFailureDetails(), true); + throw new NodeAllocationException("Could not satisfy " + requested + " in " + application + " " + cluster + + allocation.allocationFailureDetails(), true); // Carry out and return allocation List acceptedNodes = allocation.finalNodes(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index 45dd12c7bde..5de0911b2f9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -455,7 +455,7 @@ class NodeAllocation { reasons.add("insufficient real resources on hosts"); if (reasons.isEmpty()) return ""; - return "Not enough suitable nodes available due to " + String.join(", ", reasons); + return ": Not enough suitable nodes available due to " + String.join(", ", reasons); } private static Integer parseIndex(String hostname) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 6ea59edc37d..5a33f2f2bb9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -21,12 +21,10 @@ import java.util.Optional; class Preparer { private final GroupPreparer groupPreparer; - private final Optional loadBalancerProvisioner; public Preparer(NodeRepository nodeRepository, Optional hostProvisioner, Optional loadBalancerProvisioner) { - this.loadBalancerProvisioner = loadBalancerProvisioner; - this.groupPreparer = new GroupPreparer(nodeRepository, hostProvisioner); + this.groupPreparer = new GroupPreparer(nodeRepository, hostProvisioner, loadBalancerProvisioner); } /** @@ -38,14 +36,7 @@ class Preparer { // but it may not change the set of active nodes, as the active nodes must stay in sync with the // active config model which is changed on activate public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { - try { - loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requested)); - return groupPreparer.prepare(application, cluster, requested, groupPreparer.createUnlockedNodeList()); - } - catch (NodeAllocationException e) { - throw new NodeAllocationException("Could not satisfy " + requested + " in " + application + " " + cluster, e, - e.retryable()); - } + return groupPreparer.prepare(application, cluster, requested, groupPreparer.createUnlockedNodeList()); } } -- cgit v1.2.3 From 4cbdf56d3cfa8e8c828d1a0b658d959870515a1d Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 11:26:22 +0200 Subject: Simplify --- .../vespa/hosted/provision/provisioning/GroupPreparer.java | 7 ++----- .../yahoo/vespa/hosted/provision/provisioning/Preparer.java | 10 +--------- 2 files changed, 3 insertions(+), 14 deletions(-) (limited to 'node-repository/src') 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 ff24175f2a0..e3e1f265f4c 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 @@ -49,13 +49,12 @@ public class GroupPreparer { * @param application the application we are allocating to * @param cluster the cluster and group we are allocating to * @param requested a specification of the requested nodes - * @param allNodes list of all nodes and hosts * @return the list of nodes this cluster group will have allocated if activated */ // Note: This operation may make persisted changes to the set of reserved and inactive nodes, // but it may not change the set of active nodes, as the active nodes must stay in sync with the // active config model which is changed on activate - public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested, LockedNodeList allNodes) { + public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + requested.resources().orElse(NodeResources.unspecified())); @@ -63,6 +62,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. + LockedNodeList allNodes = nodeRepository.nodes().list(PROBE_LOCK); NodeIndices indices = new NodeIndices(cluster.id(), allNodes); NodeAllocation probeAllocation = prepareAllocation(application, cluster, requested, indices::probeNext, allNodes); if (probeAllocation.fulfilledAndNoChanges()) { @@ -76,9 +76,6 @@ public class GroupPreparer { } } - // Use this to create allNodes param to prepare method for first invocation of prepare - LockedNodeList createUnlockedNodeList() { return nodeRepository.nodes().list(PROBE_LOCK); } - /// Note that this will write to the node repo. private List prepareWithLocks(ApplicationId application, ClusterSpec cluster, NodeSpec requested, NodeIndices indices) { try (Mutex lock = nodeRepository.applications().lock(application); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 5a33f2f2bb9..1002f428b06 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -27,16 +27,8 @@ class Preparer { this.groupPreparer = new GroupPreparer(nodeRepository, hostProvisioner, loadBalancerProvisioner); } - /** - * Prepare all required resources for the given application and cluster. - * - * @return the list of nodes this cluster will have allocated if activated - */ - // Note: This may make persisted changes to the set of reserved and inactive nodes, - // but it may not change the set of active nodes, as the active nodes must stay in sync with the - // active config model which is changed on activate public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { - return groupPreparer.prepare(application, cluster, requested, groupPreparer.createUnlockedNodeList()); + return groupPreparer.prepare(application, cluster, requested); } } -- cgit v1.2.3 From 2e24a80d598fac1ff769423ff41703876149a53f Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 11:28:22 +0200 Subject: Remove Provisioner --- .../provisioning/NodeRepositoryProvisioner.java | 4 +-- .../hosted/provision/provisioning/Preparer.java | 34 ---------------------- 2 files changed, 2 insertions(+), 36 deletions(-) delete mode 100644 node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java (limited to 'node-repository/src') 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 43b8cd08989..36a38e7f771 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 @@ -53,7 +53,7 @@ public class NodeRepositoryProvisioner implements Provisioner { private final AllocationOptimizer allocationOptimizer; private final CapacityPolicies capacityPolicies; private final Zone zone; - private final Preparer preparer; + private final GroupPreparer preparer; private final Activator activator; private final Optional loadBalancerProvisioner; private final NodeResourceLimits nodeResourceLimits; @@ -69,7 +69,7 @@ public class NodeRepositoryProvisioner implements Provisioner { this.loadBalancerProvisioner = provisionServiceProvider.getLoadBalancerService() .map(lbService -> new LoadBalancerProvisioner(nodeRepository, lbService)); this.nodeResourceLimits = new NodeResourceLimits(nodeRepository); - this.preparer = new Preparer(nodeRepository, + this.preparer = new GroupPreparer(nodeRepository, provisionServiceProvider.getHostProvisioner(), loadBalancerProvisioner); this.activator = new Activator(nodeRepository, loadBalancerProvisioner); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java deleted file mode 100644 index 1002f428b06..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright Yahoo. 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.ClusterSpec; -import com.yahoo.config.provision.NodeAllocationException; -import com.yahoo.vespa.hosted.provision.LockedNodeList; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.NodeRepository; - -import java.util.ArrayList; -import java.util.List; -import java.util.Optional; - -/** - * Performs preparation of node activation changes for an application. - * - * @author bratseth - */ -class Preparer { - - private final GroupPreparer groupPreparer; - - public Preparer(NodeRepository nodeRepository, Optional hostProvisioner, - Optional loadBalancerProvisioner) { - this.groupPreparer = new GroupPreparer(nodeRepository, hostProvisioner, loadBalancerProvisioner); - } - - public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { - return groupPreparer.prepare(application, cluster, requested); - } - -} -- cgit v1.2.3 From a11c2e0d6c5b25b1933b392fb4b15601a1b4c048 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 11:30:19 +0200 Subject: GroupPreparer -> Preparer --- .../provision/provisioning/GroupPreparer.java | 180 --------------------- .../provisioning/NodeRepositoryProvisioner.java | 4 +- .../hosted/provision/provisioning/Preparer.java | 180 +++++++++++++++++++++ 3 files changed, 182 insertions(+), 182 deletions(-) delete mode 100644 node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java create mode 100644 node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java (limited to 'node-repository/src') 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 deleted file mode 100644 index e3e1f265f4c..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ /dev/null @@ -1,180 +0,0 @@ -// Copyright Yahoo. 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.component.Version; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.NodeAllocationException; -import com.yahoo.config.provision.NodeResources; -import com.yahoo.config.provision.NodeType; -import com.yahoo.transaction.Mutex; -import com.yahoo.vespa.hosted.provision.LockedNodeList; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.NodeRepository; -import com.yahoo.vespa.hosted.provision.node.Agent; -import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner.HostSharing; - -import java.util.ArrayList; -import java.util.List; -import java.util.Optional; -import java.util.function.Consumer; -import java.util.function.Supplier; -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * Performs preparation of node activation changes for a single host group in an application. - * - * @author bratseth - */ -public class GroupPreparer { - - private static final Mutex PROBE_LOCK = () -> {}; - private static final Logger log = Logger.getLogger(GroupPreparer.class.getName()); - - private final NodeRepository nodeRepository; - private final Optional hostProvisioner; - private final Optional loadBalancerProvisioner; - - public GroupPreparer(NodeRepository nodeRepository, Optional hostProvisioner, Optional loadBalancerProvisioner) { - this.nodeRepository = nodeRepository; - this.hostProvisioner = hostProvisioner; - this.loadBalancerProvisioner = loadBalancerProvisioner; - } - - /** - * Ensure sufficient nodes are reserved or active for the given application, group and cluster - * - * @param application the application we are allocating to - * @param cluster the cluster and group we are allocating to - * @param requested a specification of the requested nodes - * @return the list of nodes this cluster group will have allocated if activated - */ - // Note: This operation may make persisted changes to the set of reserved and inactive nodes, - // but it may not change the set of active nodes, as the active nodes must stay in sync with the - // active config model which is changed on activate - public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { - log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + - requested.resources().orElse(NodeResources.unspecified())); - - loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requested)); - - // Try preparing in memory without global unallocated lock. Most of the time there should be no changes, - // and we can return nodes previously allocated. - LockedNodeList allNodes = nodeRepository.nodes().list(PROBE_LOCK); - NodeIndices indices = new NodeIndices(cluster.id(), allNodes); - NodeAllocation probeAllocation = prepareAllocation(application, cluster, requested, indices::probeNext, allNodes); - if (probeAllocation.fulfilledAndNoChanges()) { - List acceptedNodes = probeAllocation.finalNodes(); - indices.commitProbe(); - return acceptedNodes; - } else { - // There were some changes, so re-do the allocation with locks - indices.resetProbe(); - return prepareWithLocks(application, cluster, requested, indices); - } - } - - /// Note that this will write to the node repo. - private List prepareWithLocks(ApplicationId application, ClusterSpec cluster, NodeSpec requested, NodeIndices indices) { - try (Mutex lock = nodeRepository.applications().lock(application); - Mutex allocationLock = nodeRepository.nodes().lockUnallocated()) { - LockedNodeList allNodes = nodeRepository.nodes().list(allocationLock); - NodeAllocation allocation = prepareAllocation(application, cluster, requested, indices::next, allNodes); - NodeType hostType = allocation.nodeType().hostType(); - if (canProvisionDynamically(hostType) && allocation.hostDeficit().isPresent()) { - HostSharing sharing = hostSharing(cluster, hostType); - Version osVersion = nodeRepository.osVersions().targetFor(hostType).orElse(Version.emptyVersion); - NodeAllocation.HostDeficit deficit = allocation.hostDeficit().get(); - List hosts = new ArrayList<>(); - Consumer> whenProvisioned = provisionedHosts -> { - hosts.addAll(provisionedHosts.stream().map(host -> host.generateHost(requested.hostTTL())).toList()); - nodeRepository.nodes().addNodes(hosts, Agent.application); - - // Offer the nodes on the newly provisioned hosts, this should be enough to cover the deficit - List candidates = provisionedHosts.stream() - .map(host -> NodeCandidate.createNewExclusiveChild(host.generateNode(), - host.generateHost(requested.hostTTL()))) - .toList(); - allocation.offer(candidates); - }; - try { - HostProvisionRequest request = new HostProvisionRequest(allocation.provisionIndices(deficit.count()), - hostType, - deficit.resources(), - application, - osVersion, - sharing, - Optional.of(cluster.type()), - Optional.of(cluster.id()), - requested.cloudAccount(), - deficit.dueToFlavorUpgrade()); - hostProvisioner.get().provisionHosts(request, whenProvisioned); - } catch (NodeAllocationException e) { - // Mark the nodes that were written to ZK in the consumer for deprovisioning. While these hosts do - // not exist, we cannot remove them from ZK here because other nodes may already have been - // allocated on them, so let HostDeprovisioner deal with it - hosts.forEach(host -> nodeRepository.nodes().deprovision(host.hostname(), Agent.system, nodeRepository.clock().instant())); - throw e; - } - } else if (allocation.hostDeficit().isPresent() && requested.canFail() && - allocation.hasRetiredJustNow() && requested instanceof NodeSpec.CountNodeSpec cns) { - // Non-dynamically provisioned zone with a deficit because we just now retired some nodes. - // Try again, but without retiring - indices.resetProbe(); - List accepted = prepareWithLocks(application, cluster, cns.withoutRetiring(), indices); - log.warning("Prepared " + application + " " + cluster.id() + " without retirement due to lack of capacity"); - return accepted; - } - - if (! allocation.fulfilled() && requested.canFail()) - throw new NodeAllocationException("Could not satisfy " + requested + " in " + application + " " + cluster + - allocation.allocationFailureDetails(), true); - - // Carry out and return allocation - List acceptedNodes = allocation.finalNodes(); - nodeRepository.nodes().reserve(allocation.reservableNodes()); - nodeRepository.nodes().addReservedNodes(new LockedNodeList(allocation.newNodes(), allocationLock)); - - if (requested.rejectNonActiveParent()) { // TODO: Move into offer() - currently this must be done *after* reserving - NodeList activeHosts = allNodes.state(Node.State.active).parents().nodeType(requested.type().hostType()); - acceptedNodes = acceptedNodes.stream() - .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent()) - .toList(); - } - return acceptedNodes; - } - } - - private NodeAllocation prepareAllocation(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, - Supplier nextIndex, LockedNodeList allNodes) { - - NodeAllocation allocation = new NodeAllocation(allNodes, application, cluster, requestedNodes, nextIndex, nodeRepository); - NodePrioritizer prioritizer = new NodePrioritizer(allNodes, - application, - cluster, - requestedNodes, - nodeRepository.zone().cloud().dynamicProvisioning(), - nodeRepository.nameResolver(), - nodeRepository.nodes(), - nodeRepository.resourcesCalculator(), - nodeRepository.spareCount(), - requestedNodes.cloudAccount().isExclave(nodeRepository.zone())); - allocation.offer(prioritizer.collect()); - return allocation; - } - - private boolean canProvisionDynamically(NodeType hostType) { - return nodeRepository.zone().cloud().dynamicProvisioning() && - (hostType == NodeType.host || hostType.isConfigServerHostLike()); - } - - private HostSharing hostSharing(ClusterSpec cluster, NodeType hostType) { - if ( hostType.isSharable()) - return nodeRepository.exclusiveAllocation(cluster) ? HostSharing.exclusive : HostSharing.any; - else - return HostSharing.any; - } - -} 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 36a38e7f771..43b8cd08989 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 @@ -53,7 +53,7 @@ public class NodeRepositoryProvisioner implements Provisioner { private final AllocationOptimizer allocationOptimizer; private final CapacityPolicies capacityPolicies; private final Zone zone; - private final GroupPreparer preparer; + private final Preparer preparer; private final Activator activator; private final Optional loadBalancerProvisioner; private final NodeResourceLimits nodeResourceLimits; @@ -69,7 +69,7 @@ public class NodeRepositoryProvisioner implements Provisioner { this.loadBalancerProvisioner = provisionServiceProvider.getLoadBalancerService() .map(lbService -> new LoadBalancerProvisioner(nodeRepository, lbService)); this.nodeResourceLimits = new NodeResourceLimits(nodeRepository); - this.preparer = new GroupPreparer(nodeRepository, + this.preparer = new Preparer(nodeRepository, provisionServiceProvider.getHostProvisioner(), loadBalancerProvisioner); this.activator = new Activator(nodeRepository, loadBalancerProvisioner); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java new file mode 100644 index 00000000000..55e9a9349db --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -0,0 +1,180 @@ +// Copyright Yahoo. 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.component.Version; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.NodeAllocationException; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; +import com.yahoo.transaction.Mutex; +import com.yahoo.vespa.hosted.provision.LockedNodeList; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeList; +import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.node.Agent; +import com.yahoo.vespa.hosted.provision.provisioning.HostProvisioner.HostSharing; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.function.Consumer; +import java.util.function.Supplier; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Performs preparation of node activation changes for a cluster of an application. + * + * @author bratseth + */ +public class Preparer { + + private static final Mutex PROBE_LOCK = () -> {}; + private static final Logger log = Logger.getLogger(Preparer.class.getName()); + + private final NodeRepository nodeRepository; + private final Optional hostProvisioner; + private final Optional loadBalancerProvisioner; + + public Preparer(NodeRepository nodeRepository, Optional hostProvisioner, Optional loadBalancerProvisioner) { + this.nodeRepository = nodeRepository; + this.hostProvisioner = hostProvisioner; + this.loadBalancerProvisioner = loadBalancerProvisioner; + } + + /** + * Ensure sufficient nodes are reserved or active for the given application, group and cluster + * + * @param application the application we are allocating to + * @param cluster the cluster and group we are allocating to + * @param requested a specification of the requested nodes + * @return the list of nodes this cluster group will have allocated if activated + */ + // Note: This operation may make persisted changes to the set of reserved and inactive nodes, + // but it may not change the set of active nodes, as the active nodes must stay in sync with the + // active config model which is changed on activate + public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { + log.log(Level.FINE, () -> "Preparing " + cluster.type().name() + " " + cluster.id() + " with requested resources " + + requested.resources().orElse(NodeResources.unspecified())); + + loadBalancerProvisioner.ifPresent(provisioner -> provisioner.prepare(application, cluster, requested)); + + // Try preparing in memory without global unallocated lock. Most of the time there should be no changes, + // and we can return nodes previously allocated. + LockedNodeList allNodes = nodeRepository.nodes().list(PROBE_LOCK); + NodeIndices indices = new NodeIndices(cluster.id(), allNodes); + NodeAllocation probeAllocation = prepareAllocation(application, cluster, requested, indices::probeNext, allNodes); + if (probeAllocation.fulfilledAndNoChanges()) { + List acceptedNodes = probeAllocation.finalNodes(); + indices.commitProbe(); + return acceptedNodes; + } else { + // There were some changes, so re-do the allocation with locks + indices.resetProbe(); + return prepareWithLocks(application, cluster, requested, indices); + } + } + + /// Note that this will write to the node repo. + private List prepareWithLocks(ApplicationId application, ClusterSpec cluster, NodeSpec requested, NodeIndices indices) { + try (Mutex lock = nodeRepository.applications().lock(application); + Mutex allocationLock = nodeRepository.nodes().lockUnallocated()) { + LockedNodeList allNodes = nodeRepository.nodes().list(allocationLock); + NodeAllocation allocation = prepareAllocation(application, cluster, requested, indices::next, allNodes); + NodeType hostType = allocation.nodeType().hostType(); + if (canProvisionDynamically(hostType) && allocation.hostDeficit().isPresent()) { + HostSharing sharing = hostSharing(cluster, hostType); + Version osVersion = nodeRepository.osVersions().targetFor(hostType).orElse(Version.emptyVersion); + NodeAllocation.HostDeficit deficit = allocation.hostDeficit().get(); + List hosts = new ArrayList<>(); + Consumer> whenProvisioned = provisionedHosts -> { + hosts.addAll(provisionedHosts.stream().map(host -> host.generateHost(requested.hostTTL())).toList()); + nodeRepository.nodes().addNodes(hosts, Agent.application); + + // Offer the nodes on the newly provisioned hosts, this should be enough to cover the deficit + List candidates = provisionedHosts.stream() + .map(host -> NodeCandidate.createNewExclusiveChild(host.generateNode(), + host.generateHost(requested.hostTTL()))) + .toList(); + allocation.offer(candidates); + }; + try { + HostProvisionRequest request = new HostProvisionRequest(allocation.provisionIndices(deficit.count()), + hostType, + deficit.resources(), + application, + osVersion, + sharing, + Optional.of(cluster.type()), + Optional.of(cluster.id()), + requested.cloudAccount(), + deficit.dueToFlavorUpgrade()); + hostProvisioner.get().provisionHosts(request, whenProvisioned); + } catch (NodeAllocationException e) { + // Mark the nodes that were written to ZK in the consumer for deprovisioning. While these hosts do + // not exist, we cannot remove them from ZK here because other nodes may already have been + // allocated on them, so let HostDeprovisioner deal with it + hosts.forEach(host -> nodeRepository.nodes().deprovision(host.hostname(), Agent.system, nodeRepository.clock().instant())); + throw e; + } + } else if (allocation.hostDeficit().isPresent() && requested.canFail() && + allocation.hasRetiredJustNow() && requested instanceof NodeSpec.CountNodeSpec cns) { + // Non-dynamically provisioned zone with a deficit because we just now retired some nodes. + // Try again, but without retiring + indices.resetProbe(); + List accepted = prepareWithLocks(application, cluster, cns.withoutRetiring(), indices); + log.warning("Prepared " + application + " " + cluster.id() + " without retirement due to lack of capacity"); + return accepted; + } + + if (! allocation.fulfilled() && requested.canFail()) + throw new NodeAllocationException("Could not satisfy " + requested + " in " + application + " " + cluster + + allocation.allocationFailureDetails(), true); + + // Carry out and return allocation + List acceptedNodes = allocation.finalNodes(); + nodeRepository.nodes().reserve(allocation.reservableNodes()); + nodeRepository.nodes().addReservedNodes(new LockedNodeList(allocation.newNodes(), allocationLock)); + + if (requested.rejectNonActiveParent()) { // TODO: Move into offer() - currently this must be done *after* reserving + NodeList activeHosts = allNodes.state(Node.State.active).parents().nodeType(requested.type().hostType()); + acceptedNodes = acceptedNodes.stream() + .filter(node -> node.parentHostname().isEmpty() || activeHosts.parentOf(node).isPresent()) + .toList(); + } + return acceptedNodes; + } + } + + private NodeAllocation prepareAllocation(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, + Supplier nextIndex, LockedNodeList allNodes) { + + NodeAllocation allocation = new NodeAllocation(allNodes, application, cluster, requestedNodes, nextIndex, nodeRepository); + NodePrioritizer prioritizer = new NodePrioritizer(allNodes, + application, + cluster, + requestedNodes, + nodeRepository.zone().cloud().dynamicProvisioning(), + nodeRepository.nameResolver(), + nodeRepository.nodes(), + nodeRepository.resourcesCalculator(), + nodeRepository.spareCount(), + requestedNodes.cloudAccount().isExclave(nodeRepository.zone())); + allocation.offer(prioritizer.collect()); + return allocation; + } + + private boolean canProvisionDynamically(NodeType hostType) { + return nodeRepository.zone().cloud().dynamicProvisioning() && + (hostType == NodeType.host || hostType.isConfigServerHostLike()); + } + + private HostSharing hostSharing(ClusterSpec cluster, NodeType hostType) { + if ( hostType.isSharable()) + return nodeRepository.exclusiveAllocation(cluster) ? HostSharing.exclusive : HostSharing.any; + else + return HostSharing.any; + } + +} -- cgit v1.2.3 From 963d0f7efa2949ab35c1a2e552114a16fdc24ab2 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 11:32:20 +0200 Subject: GroupIndices -> GroupAssigner --- .../provision/provisioning/GroupAssigner.java | 163 +++++++++++++++++++++ .../provision/provisioning/GroupIndices.java | 163 --------------------- .../provision/provisioning/NodeAllocation.java | 4 +- 3 files changed, 165 insertions(+), 165 deletions(-) create mode 100644 node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupAssigner.java delete mode 100644 node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupIndices.java (limited to 'node-repository/src') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupAssigner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupAssigner.java new file mode 100644 index 00000000000..4d7114520b3 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupAssigner.java @@ -0,0 +1,163 @@ +// Copyright Yahoo. 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.ClusterSpec; +import com.yahoo.config.provision.Flavor; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeList; +import com.yahoo.vespa.hosted.provision.node.Agent; + +import java.time.Clock; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; +import java.util.Optional; + +/** + * Knows how to assign a group index to a number of nodes (some of which have an index already), + * such that the nodes are placed in the desired groups with minimal group movement. + * + * @author bratseth + */ +class GroupAssigner { + + private final NodeSpec requested; + private final NodeList allNodes; + private final Clock clock; + + GroupAssigner(NodeSpec requested, NodeList allNodes, Clock clock) { + if (requested.groups() > 1 && requested.count().isEmpty()) + throw new IllegalArgumentException("Unlimited nodes cannot be grouped"); + this.requested = requested; + this.allNodes = allNodes; + this.clock = clock; + } + + Collection assignTo(Collection nodes) { + int[] countInGroup = countInEachGroup(nodes); + nodes = byUnretiringPriority(nodes).stream().map(node -> unretireNodeInExpandedGroup(node, countInGroup)).toList(); + nodes = nodes.stream().map(node -> assignGroupToNewNode(node, countInGroup)).toList(); + nodes = byUnretiringPriority(nodes).stream().map(node -> moveNodeInSurplusGroup(node, countInGroup)).toList(); + nodes = byRetiringPriority(nodes).stream().map(node -> retireSurplusNodeInGroup(node, countInGroup)).toList(); + nodes = nodes.stream().filter(node -> ! shouldRemove(node)).toList(); + return nodes; + } + + /** Prefer to retire nodes we want the least */ + private List byRetiringPriority(Collection candidates) { + return candidates.stream().sorted(Comparator.reverseOrder()).toList(); + } + + /** Prefer to unretire nodes we don't want to retire, and otherwise those with lower index */ + private List byUnretiringPriority(Collection candidates) { + return candidates.stream() + .sorted(Comparator.comparing(NodeCandidate::wantToRetire) + .thenComparing(n -> n.allocation().get().membership().index())) + .toList(); + } + + private int[] countInEachGroup(Collection nodes) { + int[] countInGroup = new int[requested.groups()]; + for (var node : nodes) { + if (node.allocation().get().membership().retired()) continue; + var currentGroup = node.allocation().get().membership().cluster().group(); + if (currentGroup.isEmpty()) continue; + if (currentGroup.get().index() >= requested.groups()) continue; + countInGroup[currentGroup.get().index()]++; + } + return countInGroup; + } + + /** Assign a group to new or to be reactivated nodes. */ + private NodeCandidate assignGroupToNewNode(NodeCandidate node, int[] countInGroup) { + if (node.state() == Node.State.active && node.allocation().get().membership().retired()) return node; + if (node.state() == Node.State.active && node.allocation().get().membership().cluster().group().isPresent()) return node; + return inFirstGroupWithDeficiency(node, countInGroup); + } + + private NodeCandidate moveNodeInSurplusGroup(NodeCandidate node, int[] countInGroup) { + var currentGroup = node.allocation().get().membership().cluster().group(); + if (currentGroup.isEmpty()) return node; + if (currentGroup.get().index() < requested.groups()) return node; + return inFirstGroupWithDeficiency(node, countInGroup); + } + + private NodeCandidate retireSurplusNodeInGroup(NodeCandidate node, int[] countInGroup) { + if (node.allocation().get().membership().retired()) return node; + var currentGroup = node.allocation().get().membership().cluster().group(); + if (currentGroup.isEmpty()) return node; + if (currentGroup.get().index() >= requested.groups()) return node; + if (requested.count().isEmpty()) return node; // Can't retire + if (countInGroup[currentGroup.get().index()] <= requested.groupSize()) return node; + countInGroup[currentGroup.get().index()]--; + return node.withNode(node.toNode().retire(Agent.application, clock.instant())); + } + + /** Unretire nodes that are already in the correct group when the group is deficient. */ + private NodeCandidate unretireNodeInExpandedGroup(NodeCandidate node, int[] countInGroup) { + if ( ! node.allocation().get().membership().retired()) return node; + var currentGroup = node.allocation().get().membership().cluster().group(); + if (currentGroup.isEmpty()) return node; + if (currentGroup.get().index() >= requested.groups()) return node; + if (node.preferToRetire() || node.wantToRetire()) return node; + if (requested.count().isPresent() && countInGroup[currentGroup.get().index()] >= requested.groupSize()) return node; + node = unretire(node); + if (node.allocation().get().membership().retired()) return node; + countInGroup[currentGroup.get().index()]++; + return node; + } + + private NodeCandidate inFirstGroupWithDeficiency(NodeCandidate node, int[] countInGroup) { + for (int group = 0; group < requested.groups(); group++) { + if (requested.count().isEmpty() || countInGroup[group] < requested.groupSize()) { + return inGroup(group, node, countInGroup); + } + } + return node; + } + + private boolean shouldRemove(NodeCandidate node) { + var currentGroup = node.allocation().get().membership().cluster().group(); + if (currentGroup.isEmpty()) return true; // new and not assigned an index: Not needed + return currentGroup.get().index() >= requested.groups(); + } + + private NodeCandidate inGroup(int group, NodeCandidate node, int[] countInGroup) { + node = unretire(node); + if (node.allocation().get().membership().retired()) return node; + var membership = node.allocation().get().membership(); + var currentGroup = membership.cluster().group(); + countInGroup[group]++; + if ( ! currentGroup.isEmpty() && currentGroup.get().index() < requested.groups()) + countInGroup[membership.cluster().group().get().index()]--; + return node.withNode(node.toNode().with(node.allocation().get().with(membership.with(membership.cluster().with(Optional.of(ClusterSpec.Group.from(group))))))); + } + + /** Attempt to unretire the given node if it is retired. */ + private NodeCandidate unretire(NodeCandidate node) { + if (node.retiredNow()) return node; + if ( ! node.allocation().get().membership().retired()) return node; + if ( ! hasCompatibleResources(node) ) return node; + var parent = node.parentHostname().flatMap(hostname -> allNodes.node(hostname)); + if (parent.isPresent() && (parent.get().status().wantToRetire() || parent.get().status().preferToRetire())) return node; + node = node.withNode(); + if ( ! requested.isCompatible(node.resources())) + node = node.withNode(resize(node.toNode())); + return node.withNode(node.toNode().unretire()); + } + + private Node resize(Node node) { + NodeResources hostResources = allNodes.parentOf(node).get().flavor().resources(); + return node.with(new Flavor(requested.resources().get() + .with(hostResources.diskSpeed()) + .with(hostResources.storageType()) + .with(hostResources.architecture())), + Agent.application, clock.instant()); + } + + private boolean hasCompatibleResources(NodeCandidate candidate) { + return requested.isCompatible(candidate.resources()) || candidate.isResizable; + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupIndices.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupIndices.java deleted file mode 100644 index 27b31d3bea3..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupIndices.java +++ /dev/null @@ -1,163 +0,0 @@ -// Copyright Yahoo. 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.ClusterSpec; -import com.yahoo.config.provision.Flavor; -import com.yahoo.config.provision.NodeResources; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeList; -import com.yahoo.vespa.hosted.provision.node.Agent; - -import java.time.Clock; -import java.util.Collection; -import java.util.Comparator; -import java.util.List; -import java.util.Optional; - -/** - * Knows how to assign a group index to a number of nodes (some of which have an index already), - * such that the nodes are placed in the desired groups with minimal group movement. - * - * @author bratseth - */ -class GroupIndices { - - private final NodeSpec requested; - private final NodeList allNodes; - private final Clock clock; - - GroupIndices(NodeSpec requested, NodeList allNodes, Clock clock) { - if (requested.groups() > 1 && requested.count().isEmpty()) - throw new IllegalArgumentException("Unlimited nodes cannot be grouped"); - this.requested = requested; - this.allNodes = allNodes; - this.clock = clock; - } - - Collection assignTo(Collection nodes) { - int[] countInGroup = countInEachGroup(nodes); - nodes = byUnretiringPriority(nodes).stream().map(node -> unretireNodeInExpandedGroup(node, countInGroup)).toList(); - nodes = nodes.stream().map(node -> assignGroupToNewNode(node, countInGroup)).toList(); - nodes = byUnretiringPriority(nodes).stream().map(node -> moveNodeInSurplusGroup(node, countInGroup)).toList(); - nodes = byRetiringPriority(nodes).stream().map(node -> retireSurplusNodeInGroup(node, countInGroup)).toList(); - nodes = nodes.stream().filter(node -> ! shouldRemove(node)).toList(); - return nodes; - } - - /** Prefer to retire nodes we want the least */ - private List byRetiringPriority(Collection candidates) { - return candidates.stream().sorted(Comparator.reverseOrder()).toList(); - } - - /** Prefer to unretire nodes we don't want to retire, and otherwise those with lower index */ - private List byUnretiringPriority(Collection candidates) { - return candidates.stream() - .sorted(Comparator.comparing(NodeCandidate::wantToRetire) - .thenComparing(n -> n.allocation().get().membership().index())) - .toList(); - } - - private int[] countInEachGroup(Collection nodes) { - int[] countInGroup = new int[requested.groups()]; - for (var node : nodes) { - if (node.allocation().get().membership().retired()) continue; - var currentGroup = node.allocation().get().membership().cluster().group(); - if (currentGroup.isEmpty()) continue; - if (currentGroup.get().index() >= requested.groups()) continue; - countInGroup[currentGroup.get().index()]++; - } - return countInGroup; - } - - /** Assign a group to new or to be reactivated nodes. */ - private NodeCandidate assignGroupToNewNode(NodeCandidate node, int[] countInGroup) { - if (node.state() == Node.State.active && node.allocation().get().membership().retired()) return node; - if (node.state() == Node.State.active && node.allocation().get().membership().cluster().group().isPresent()) return node; - return inFirstGroupWithDeficiency(node, countInGroup); - } - - private NodeCandidate moveNodeInSurplusGroup(NodeCandidate node, int[] countInGroup) { - var currentGroup = node.allocation().get().membership().cluster().group(); - if (currentGroup.isEmpty()) return node; - if (currentGroup.get().index() < requested.groups()) return node; - return inFirstGroupWithDeficiency(node, countInGroup); - } - - private NodeCandidate retireSurplusNodeInGroup(NodeCandidate node, int[] countInGroup) { - if (node.allocation().get().membership().retired()) return node; - var currentGroup = node.allocation().get().membership().cluster().group(); - if (currentGroup.isEmpty()) return node; - if (currentGroup.get().index() >= requested.groups()) return node; - if (requested.count().isEmpty()) return node; // Can't retire - if (countInGroup[currentGroup.get().index()] <= requested.groupSize()) return node; - countInGroup[currentGroup.get().index()]--; - return node.withNode(node.toNode().retire(Agent.application, clock.instant())); - } - - /** Unretire nodes that are already in the correct group when the group is deficient. */ - private NodeCandidate unretireNodeInExpandedGroup(NodeCandidate node, int[] countInGroup) { - if ( ! node.allocation().get().membership().retired()) return node; - var currentGroup = node.allocation().get().membership().cluster().group(); - if (currentGroup.isEmpty()) return node; - if (currentGroup.get().index() >= requested.groups()) return node; - if (node.preferToRetire() || node.wantToRetire()) return node; - if (requested.count().isPresent() && countInGroup[currentGroup.get().index()] >= requested.groupSize()) return node; - node = unretire(node); - if (node.allocation().get().membership().retired()) return node; - countInGroup[currentGroup.get().index()]++; - return node; - } - - private NodeCandidate inFirstGroupWithDeficiency(NodeCandidate node, int[] countInGroup) { - for (int group = 0; group < requested.groups(); group++) { - if (requested.count().isEmpty() || countInGroup[group] < requested.groupSize()) { - return inGroup(group, node, countInGroup); - } - } - return node; - } - - private boolean shouldRemove(NodeCandidate node) { - var currentGroup = node.allocation().get().membership().cluster().group(); - if (currentGroup.isEmpty()) return true; // new and not assigned an index: Not needed - return currentGroup.get().index() >= requested.groups(); - } - - private NodeCandidate inGroup(int group, NodeCandidate node, int[] countInGroup) { - node = unretire(node); - if (node.allocation().get().membership().retired()) return node; - var membership = node.allocation().get().membership(); - var currentGroup = membership.cluster().group(); - countInGroup[group]++; - if ( ! currentGroup.isEmpty() && currentGroup.get().index() < requested.groups()) - countInGroup[membership.cluster().group().get().index()]--; - return node.withNode(node.toNode().with(node.allocation().get().with(membership.with(membership.cluster().with(Optional.of(ClusterSpec.Group.from(group))))))); - } - - /** Attempt to unretire the given node if it is retired. */ - private NodeCandidate unretire(NodeCandidate node) { - if (node.retiredNow()) return node; - if ( ! node.allocation().get().membership().retired()) return node; - if ( ! hasCompatibleResources(node) ) return node; - var parent = node.parentHostname().flatMap(hostname -> allNodes.node(hostname)); - if (parent.isPresent() && (parent.get().status().wantToRetire() || parent.get().status().preferToRetire())) return node; - node = node.withNode(); - if ( ! requested.isCompatible(node.resources())) - node = node.withNode(resize(node.toNode())); - return node.withNode(node.toNode().unretire()); - } - - private Node resize(Node node) { - NodeResources hostResources = allNodes.parentOf(node).get().flavor().resources(); - return node.with(new Flavor(requested.resources().get() - .with(hostResources.diskSpeed()) - .with(hostResources.storageType()) - .with(hostResources.architecture())), - Agent.application, clock.instant()); - } - - private boolean hasCompatibleResources(NodeCandidate candidate) { - return requested.isCompatible(candidate.resources()) || candidate.isResizable; - } - -} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index 5de0911b2f9..1d05548e571 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -412,8 +412,8 @@ class NodeAllocation { } // Place in groups - GroupIndices groupIndices = new GroupIndices(requested, allNodes, nodeRepository.clock()); - Collection finalNodes = groupIndices.assignTo(nodes.values()); + GroupAssigner groupAssigner = new GroupAssigner(requested, allNodes, nodeRepository.clock()); + Collection finalNodes = groupAssigner.assignTo(nodes.values()); nodes.clear(); finalNodes.forEach(candidate -> nodes.put(candidate.toNode().hostname(), candidate)); -- cgit v1.2.3 From 8daf09b502135f120e2f1fab23b86de597cac982 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 12 Jul 2023 11:33:38 +0200 Subject: Simplify --- .../provisioning/LoadBalancerProvisioner.java | 6 ++--- .../provision/provisioning/NodePrioritizer.java | 30 +++++++++++----------- .../hosted/provision/provisioning/Preparer.java | 8 +++--- 3 files changed, 22 insertions(+), 22 deletions(-) (limited to 'node-repository/src') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index 10aae94a986..5a35ed1cc42 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -88,12 +88,12 @@ public class LoadBalancerProvisioner { *

* Calling this for irrelevant node or cluster types is a no-op. */ - public void prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes) { - if (!shouldProvision(application, requestedNodes.type(), cluster.type())) return; + public void prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requested) { + if (!shouldProvision(application, requested.type(), cluster.type())) return; try (var lock = db.lock(application)) { ClusterSpec.Id clusterId = effectiveId(cluster); LoadBalancerId loadBalancerId = requireNonClashing(new LoadBalancerId(application, clusterId)); - prepare(loadBalancerId, cluster.zoneEndpoint(), requestedNodes.cloudAccount()); + prepare(loadBalancerId, cluster.zoneEndpoint(), requested.cloudAccount()); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java index 3c33897af57..4ac90753ed1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodePrioritizer.java @@ -33,7 +33,7 @@ public class NodePrioritizer { private final LockedNodeList allNodes; private final HostCapacity capacity; private final HostResourcesCalculator calculator; - private final NodeSpec requestedNodes; + private final NodeSpec requested; private final ApplicationId application; private final ClusterSpec clusterSpec; private final NameResolver nameResolver; @@ -51,7 +51,7 @@ public class NodePrioritizer { this.allNodes = allNodes; this.calculator = hostResourcesCalculator; this.capacity = new HostCapacity(this.allNodes, hostResourcesCalculator); - this.requestedNodes = nodeSpec; + this.requested = nodeSpec; this.clusterSpec = clusterSpec; this.application = application; this.dynamicProvisioning = dynamicProvisioning; @@ -70,7 +70,7 @@ public class NodePrioritizer { .stream()) .distinct() .count(); - this.topologyChange = currentGroups != requestedNodes.groups(); + this.topologyChange = currentGroups != requested.groups(); this.currentClusterSize = (int) nonRetiredNodesInCluster.state(Node.State.active).stream().count(); @@ -116,7 +116,7 @@ public class NodePrioritizer { /** Add a node on each host with enough capacity for the requested flavor */ private void addCandidatesOnExistingHosts() { - if (requestedNodes.resources().isEmpty()) return; + if (requested.resources().isEmpty()) return; for (Node host : allNodes) { if ( ! nodes.canAllocateTenantNodeTo(host, dynamicProvisioning)) continue; @@ -126,10 +126,10 @@ public class NodePrioritizer { if (host.exclusiveToApplicationId().isPresent() && ! fitsPerfectly(host)) continue; if ( ! host.exclusiveToClusterType().map(clusterSpec.type()::equals).orElse(true)) continue; if (spareHosts.contains(host) && !canAllocateToSpareHosts) continue; - if ( ! capacity.hasCapacity(host, requestedNodes.resources().get())) continue; + if ( ! capacity.hasCapacity(host, requested.resources().get())) continue; if ( ! allNodes.childrenOf(host).owner(application).cluster(clusterSpec.id()).isEmpty()) continue; - candidates.add(NodeCandidate.createNewChild(requestedNodes.resources().get(), + candidates.add(NodeCandidate.createNewChild(requested.resources().get(), capacity.availableCapacityOf(host), host, spareHosts.contains(host), @@ -140,14 +140,14 @@ public class NodePrioritizer { } private boolean fitsPerfectly(Node host) { - return calculator.advertisedResourcesOf(host.flavor()).compatibleWith(requestedNodes.resources().get()); + return calculator.advertisedResourcesOf(host.flavor()).compatibleWith(requested.resources().get()); } /** Add existing nodes allocated to the application */ private void addApplicationNodes() { EnumSet legalStates = EnumSet.of(Node.State.active, Node.State.inactive, Node.State.reserved); allNodes.stream() - .filter(node -> node.type() == requestedNodes.type()) + .filter(node -> node.type() == requested.type()) .filter(node -> legalStates.contains(node.state())) .filter(node -> node.allocation().isPresent()) .filter(node -> node.allocation().get().owner().equals(application)) @@ -160,7 +160,7 @@ public class NodePrioritizer { /** Add nodes already provisioned, but not allocated to any application */ private void addReadyNodes() { allNodes.stream() - .filter(node -> node.type() == requestedNodes.type()) + .filter(node -> node.type() == requested.type()) .filter(node -> node.state() == Node.State.ready) .map(node -> candidateFrom(node, false)) .filter(n -> !n.violatesSpares || canAllocateToSpareHosts) @@ -179,11 +179,11 @@ public class NodePrioritizer { isSurplus, false, parent.exclusiveToApplicationId().isEmpty() - && requestedNodes.canResize(node.resources(), - capacity.unusedCapacityOf(parent), - clusterSpec.type(), - topologyChange, - currentClusterSize)); + && requested.canResize(node.resources(), + capacity.unusedCapacityOf(parent), + clusterSpec.type(), + topologyChange, + currentClusterSize)); } else { return NodeCandidate.createStandalone(node, isSurplus, false); } @@ -196,7 +196,7 @@ public class NodePrioritizer { .orElse(nodesInCluster); int failedNodesInGroup = nodesInGroup.failing().size() + nodesInGroup.state(Node.State.failed).size(); if (failedNodesInGroup == 0) return false; - return ! requestedNodes.fulfilledBy(nodesInGroup.size() - failedNodesInGroup); + return ! requested.fulfilledBy(nodesInGroup.size() - failedNodesInGroup); } /** diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java index 55e9a9349db..8975dda8e60 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Preparer.java @@ -147,20 +147,20 @@ public class Preparer { } } - private NodeAllocation prepareAllocation(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, + private NodeAllocation prepareAllocation(ApplicationId application, ClusterSpec cluster, NodeSpec requested, Supplier nextIndex, LockedNodeList allNodes) { - NodeAllocation allocation = new NodeAllocation(allNodes, application, cluster, requestedNodes, nextIndex, nodeRepository); + NodeAllocation allocation = new NodeAllocation(allNodes, application, cluster, requested, nextIndex, nodeRepository); NodePrioritizer prioritizer = new NodePrioritizer(allNodes, application, cluster, - requestedNodes, + requested, nodeRepository.zone().cloud().dynamicProvisioning(), nodeRepository.nameResolver(), nodeRepository.nodes(), nodeRepository.resourcesCalculator(), nodeRepository.spareCount(), - requestedNodes.cloudAccount().isExclave(nodeRepository.zone())); + requested.cloudAccount().isExclave(nodeRepository.zone())); allocation.offer(prioritizer.collect()); return allocation; } -- cgit v1.2.3