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);
     }