diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-09-28 14:05:29 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-09-28 14:05:29 +0200 |
commit | daff9cf69012cb5ebaa85b900b3353b2060c1fcd (patch) | |
tree | f22de06bc41ac4e335f6e8d185360482b1d87892 | |
parent | b7b6eb10a64787b6a4be8d43d52af7d7aeaa5267 (diff) | |
parent | 371e1bfb512247e8af10540ccc871632fcd0ad7e (diff) |
Merge pull request #19325 from vespa-engine/revert-19313-balder/precompute-hostresources-to-avoid-redoing-it-on-every-compare
Revert "Precompute host resources to avoid redoing it on every compare during…"
2 files changed, 21 insertions, 54 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMover.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMover.java index b0d69c40a86..54e9886d2b7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMover.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMover.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; -import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.provision.Node; @@ -13,8 +12,6 @@ import com.yahoo.vespa.hosted.provision.provisioning.HostCapacity; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; -import java.util.List; import java.util.Random; import java.util.Set; @@ -41,19 +38,6 @@ public abstract class NodeMover<MOVE> extends NodeRepositoryMaintainer { /** Returns a suggested move for given node */ protected abstract MOVE suggestedMove(Node node, Node fromHost, Node toHost, NodeList allNodes); - private static class HostWithResources { - private final Node node; - private final NodeResources hostResources; - HostWithResources(Node node, NodeResources hostResources) { - this.node = node; - this.hostResources = hostResources; - } - boolean hasCapacity(NodeResources requested) { - return hostResources.satisfies(requested); - } - - } - /** Find the best possible move */ protected final MOVE findBestMove(NodeList allNodes) { HostCapacity capacity = new HostCapacity(allNodes, nodeRepository().resourcesCalculator()); @@ -64,19 +48,17 @@ public abstract class NodeMover<MOVE> extends NodeRepositoryMaintainer { .state(Node.State.active) .shuffle(random); Set<Node> spares = capacity.findSpareHosts(allNodes.asList(), nodeRepository().spareCount()); - List<HostWithResources> hostResources = new ArrayList<>(); - allNodes.matching(nodeRepository().nodes()::canAllocateTenantNodeTo).forEach(host -> hostResources.add(new HostWithResources(host, capacity.availableCapacityOf(host)))); for (Node node : activeNodes) { if (node.parentHostname().isEmpty()) continue; ApplicationId applicationId = node.allocation().get().owner(); if (applicationId.instance().isTester()) continue; if (deployedRecently(applicationId)) continue; - for (HostWithResources toHost : hostResources) { - if (toHost.node.hostname().equals(node.parentHostname().get())) continue; + for (Node toHost : allNodes.matching(nodeRepository().nodes()::canAllocateTenantNodeTo)) { + if (toHost.hostname().equals(node.parentHostname().get())) continue; if (spares.contains(toHost)) continue; // Do not offer spares as a valid move as they are reserved for replacement of failed nodes - if ( ! toHost.hasCapacity(node.resources())) continue; + if ( ! capacity.hasCapacity(toHost, node.resources())) continue; - MOVE suggestedMove = suggestedMove(node, allNodes.parentOf(node).get(), toHost.node, allNodes); + MOVE suggestedMove = suggestedMove(node, allNodes.parentOf(node).get(), toHost, allNodes); bestMove = bestMoveOf(bestMove, suggestedMove); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java index d20abdb5b41..59c942f3298 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java @@ -6,8 +6,6 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; -import java.util.ArrayList; -import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Set; @@ -44,35 +42,14 @@ public class HostCapacity { * @param count the max number of spare hosts to return */ public Set<Node> findSpareHosts(List<Node> candidates, int count) { - List<NodeWithHostResources> nodesWithIp = new ArrayList<>(candidates.size()); - for (Node node : candidates) { - if (node.type().canRun(NodeType.tenant) && (node.state() == Node.State.active) && node.reservedTo().isEmpty()) { - int numFreeIps = freeIps(node); - if (numFreeIps > 0) { - nodesWithIp.add(new NodeWithHostResources(node, availableCapacityOf(node, true, false), numFreeIps)); - } - } - } - Set<Node> nodes = new HashSet<>(); - nodesWithIp.stream().sorted(NodeWithHostResources::compareTo).limit(count).forEach(n -> nodes.add(n.node)); - return nodes; - } - private static class NodeWithHostResources implements Comparable<NodeWithHostResources> { - private final Node node; - private final NodeResources hostResources; - private final int numFreeIps; - NodeWithHostResources(Node node, NodeResources hostResources, int freeIps) { - this.node = node; - this.hostResources = hostResources; - this.numFreeIps = freeIps; - } - - @Override - public int compareTo(HostCapacity.NodeWithHostResources b) { - int result = compare(b.hostResources, hostResources); - if (result != 0) return result; - return b.numFreeIps - numFreeIps; - } + return candidates.stream() + .filter(node -> node.type().canRun(NodeType.tenant)) + .filter(host -> host.state() == Node.State.active) + .filter(host -> host.reservedTo().isEmpty()) + .filter(host -> freeIps(host) > 0) + .sorted(this::compareWithoutInactive) + .limit(count) + .collect(Collectors.toSet()); } public Set<Node> findSpareHostsInDynamicallyProvisionedZones(List<Node> candidates) { @@ -84,7 +61,15 @@ public class HostCapacity { .collect(Collectors.toSet()); } - private static int compare(NodeResources a, NodeResources b) { + private int compareWithoutInactive(Node a, Node b) { + int result = compare(availableCapacityOf(b, true, false), availableCapacityOf(a, true, false)); + if (result != 0) return result; + + // If resources are equal we want to assign to the one with the most IP addresses free + return freeIps(b) - freeIps(a); + } + + private int compare(NodeResources a, NodeResources b) { return NodeResourceComparator.defaultOrder().compare(a, b); } |