diff options
author | Jon Bratseth <bratseth@oath.com> | 2021-08-20 10:03:38 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-08-20 10:03:38 +0200 |
commit | 4c97161ae4ac76bcb7a2753762c9e17c6447b636 (patch) | |
tree | 2ffe834e452971cab75ac7a987aefd17f2e14834 /node-repository | |
parent | aa9367720f0d55af373a1e64acc88167deb3c82c (diff) | |
parent | 34b0b9d01729e83f2d3ba8b24379571f709d6785 (diff) |
Merge pull request #18802 from vespa-engine/mpolden/fix-capacity-check
Always account for available IP addresses when checking capacity
Diffstat (limited to 'node-repository')
6 files changed, 65 insertions, 36 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 29dcfddd821..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 @@ -56,7 +56,7 @@ public abstract class NodeMover<MOVE> extends NodeRepositoryMaintainer { 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 ( ! capacity.freeCapacityOf(toHost).satisfies(node.resources())) continue; + if ( ! capacity.hasCapacity(toHost, node.resources())) continue; 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/maintenance/Rebalancer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java index 7bb748c92c9..50bb213ad73 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/Rebalancer.java @@ -69,20 +69,20 @@ public class Rebalancer extends NodeMover<Rebalancer.Move> { int hostCount = 0; for (Node host : allNodes.nodeType(NodeType.host).state(Node.State.active)) { hostCount++; - totalSkew += Node.skew(host.flavor().resources(), capacity.freeCapacityOf(host)); + totalSkew += Node.skew(host.flavor().resources(), capacity.unusedCapacityOf(host)); } metric.set("hostedVespa.docker.skew", totalSkew/hostCount, null); } private double skewReductionByRemoving(Node node, Node fromHost, HostCapacity capacity) { - NodeResources freeHostCapacity = capacity.freeCapacityOf(fromHost); + NodeResources freeHostCapacity = capacity.unusedCapacityOf(fromHost); double skewBefore = Node.skew(fromHost.flavor().resources(), freeHostCapacity); double skewAfter = Node.skew(fromHost.flavor().resources(), freeHostCapacity.add(node.flavor().resources().justNumbers())); return skewBefore - skewAfter; } private double skewReductionByAdding(Node node, Node toHost, HostCapacity capacity) { - NodeResources freeHostCapacity = capacity.freeCapacityOf(toHost); + NodeResources freeHostCapacity = capacity.unusedCapacityOf(toHost); double skewBefore = Node.skew(toHost.flavor().resources(), freeHostCapacity); double skewAfter = Node.skew(toHost.flavor().resources(), freeHostCapacity.subtract(node.resources().justNumbers())); return skewBefore - skewAfter; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java index 0589571e9d8..2447503515a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java @@ -220,7 +220,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { return null; if ( ! host.resources().satisfies(node.resources())) return null; - NodeResources freeCapacity = freeCapacityWith(movesMade, host); + NodeResources freeCapacity = availableCapacityWith(movesMade, host); if (freeCapacity.satisfies(node.resources())) return List.of(); List<Move> shortest = null; @@ -291,8 +291,8 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { return list; } - private NodeResources freeCapacityWith(List<Move> moves, Node host) { - NodeResources resources = hostCapacity.freeCapacityOf(host); + private NodeResources availableCapacityWith(List<Move> moves, Node host) { + NodeResources resources = hostCapacity.availableCapacityOf(host); for (Move move : moves) { if ( ! move.toHost().equals(host)) continue; resources = resources.subtract(move.node().resources()); 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 58a7aa2b189..9109f9fcf72 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 @@ -62,7 +62,7 @@ public class HostCapacity { } private int compareWithoutInactive(Node a, Node b) { - int result = compare(freeCapacityOf(b, true), freeCapacityOf(a, true)); + 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 @@ -73,33 +73,41 @@ public class HostCapacity { return NodeResourceComparator.defaultOrder().compare(a, b); } - /** Returns whether host has room for requested capacity */ - boolean hasCapacity(Node host, NodeResources requestedCapacity) { - return freeCapacityOf(host, false).satisfies(requestedCapacity) && freeIps(host) > 0; + /** Returns whether host can allocate a node with requested capacity */ + public boolean hasCapacity(Node host, NodeResources requestedCapacity) { + return availableCapacityOf(host).satisfies(requestedCapacity); } /** Returns the number of available IP addresses on given host */ int freeIps(Node host) { if (host.type() == NodeType.host) { return host.ipConfig().pool().eventuallyUnusedAddressCount(allNodes); - } else { - return host.ipConfig().pool().findUnusedIpAddresses(allNodes).size(); } + return host.ipConfig().pool().findUnusedIpAddresses(allNodes).size(); + } + + /** Returns the capacity of given host that is both free and usable */ + public NodeResources availableCapacityOf(Node host) { + return availableCapacityOf(host, false, true); } /** - * Calculate the remaining capacity of a host. + * Calculate the unused capacity of given host. + * + * Note that unlike {@link this#availableCapacityOf(Node)}, this only considers resources and returns any unused + * capacity even if the host does not have available IP addresses. * - * @param host the host to find free capacity of. - * @return a default (empty) capacity if not host, otherwise the free/unallocated/rest capacity + * @param host the host to find free capacity of + * @return a default (empty) capacity if not host, otherwise the free capacity */ - public NodeResources freeCapacityOf(Node host) { - return freeCapacityOf(host, false); + public NodeResources unusedCapacityOf(Node host) { + return availableCapacityOf(host, false, false); } - NodeResources freeCapacityOf(Node host, boolean excludeInactive) { + private NodeResources availableCapacityOf(Node host, boolean excludeInactive, boolean requireIps) { // Only hosts have free capacity - if ( ! host.type().canRun(NodeType.tenant)) return new NodeResources(0, 0, 0, 0); + if ( ! host.type().canRun(NodeType.tenant)) return NodeResources.zero(); + if ( requireIps && freeIps(host) == 0) return NodeResources.zero(); NodeResources hostResources = hostResourcesCalculator.advertisedResourcesOf(host.flavor()); return allNodes.childrenOf(host).asList().stream() 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 6568046991b..bfc848d4031 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 @@ -94,8 +94,10 @@ public class NodePrioritizer { /** Returns the list of nodes sorted by {@link NodeCandidate#compareTo(NodeCandidate)} */ private List<NodeCandidate> prioritize() { // Group candidates by their switch hostname - Map<Optional<String>, List<NodeCandidate>> candidatesBySwitch = this.nodes.stream() - .collect(Collectors.groupingBy(candidate -> candidate.parent.orElseGet(candidate::toNode).switchHostname())); + Map<String, List<NodeCandidate>> candidatesBySwitch = this.nodes.stream() + .collect(Collectors.groupingBy(candidate -> candidate.parent.orElseGet(candidate::toNode) + .switchHostname() + .orElse(""))); // Mark lower priority nodes on shared switch as non-exclusive List<NodeCandidate> nodes = new ArrayList<>(this.nodes.size()); for (var clusterSwitch : candidatesBySwitch.keySet()) { @@ -142,7 +144,7 @@ public class NodePrioritizer { if ( ! capacity.hasCapacity(host, requestedNodes.resources().get())) continue; if ( ! allNodes.childrenOf(host).owner(application).cluster(clusterSpec.id()).isEmpty()) continue; nodes.add(NodeCandidate.createNewChild(requestedNodes.resources().get(), - capacity.freeCapacityOf(host, false), + capacity.availableCapacityOf(host), host, spareHosts.contains(host), allNodes, @@ -179,14 +181,14 @@ public class NodePrioritizer { Optional<Node> parent = allNodes.parentOf(node); if (parent.isPresent()) { return NodeCandidate.createChild(node, - capacity.freeCapacityOf(parent.get(), false), + capacity.availableCapacityOf(parent.get()), parent.get(), spareHosts.contains(parent.get()), isSurplus, false, parent.get().exclusiveToApplicationId().isEmpty() && requestedNodes.canResize(node.resources(), - capacity.freeCapacityOf(parent.get(), false), + capacity.availableCapacityOf(parent.get()), topologyChange, currentClusterSize)); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java index 995d25043f3..d03a238f64f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java @@ -34,7 +34,10 @@ public class HostCapacityTest { private final HostResourcesCalculator hostResourcesCalculator = mock(HostResourcesCalculator.class); private HostCapacity capacity; private List<Node> nodes; - private Node host1, host2, host3; + private Node host1; + private Node host2; + private Node host3; + private Node host4; private final NodeResources resources0 = new NodeResources(1, 30, 20, 1.5); private final NodeResources resources1 = new NodeResources(2, 40, 40, 0.5); @@ -45,10 +48,11 @@ public class HostCapacityTest { // Create flavors NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("host", "docker", "docker2"); - // Create three hosts + // Create hosts host1 = Node.create("host1", IP.Config.of(Set.of("::1"), createIps(2, 4), List.of()), "host1", nodeFlavors.getFlavorOrThrow("host"), NodeType.host).build(); host2 = Node.create("host2", IP.Config.of(Set.of("::11"), createIps(12, 3), List.of()), "host2", nodeFlavors.getFlavorOrThrow("host"), NodeType.host).build(); - host3 = Node.create("host3", IP.Config.of(Set.of("::21"), createIps(22, 1), List.of()), "host3", nodeFlavors.getFlavorOrThrow("host"), NodeType.host).build(); + host3 = Node.create("host3", IP.Config.of(Set.of("::21"), createIps(22, 2), List.of()), "host3", nodeFlavors.getFlavorOrThrow("host"), NodeType.host).build(); + host4 = Node.create("host3", IP.Config.of(Set.of("::21"), createIps(50, 0), List.of()), "host4", nodeFlavors.getFlavorOrThrow("host"), NodeType.host).build(); // Add two containers to host1 var nodeA = Node.reserve(Set.of("::2"), "nodeA", "host1", resources0, NodeType.tenant).build(); @@ -72,8 +76,10 @@ public class HostCapacityTest { assertTrue(capacity.hasCapacity(host1, resources1)); assertTrue(capacity.hasCapacity(host2, resources0)); assertTrue(capacity.hasCapacity(host2, resources1)); - assertFalse(capacity.hasCapacity(host3, resources0)); // No ip available - assertFalse(capacity.hasCapacity(host3, resources1)); // No ip available + assertTrue(capacity.hasCapacity(host3, resources0)); + assertTrue(capacity.hasCapacity(host3, resources1)); + assertFalse(capacity.hasCapacity(host4, resources0)); // No IPs available + assertFalse(capacity.hasCapacity(host4, resources1)); // No IPs available // Add a new node to host1 to deplete the memory resource Node nodeF = Node.reserve(Set.of("::6"), "nodeF", "host1", resources0, NodeType.tenant).build(); @@ -87,15 +93,18 @@ public class HostCapacityTest { public void freeIPs() { assertEquals(2, capacity.freeIps(host1)); assertEquals(1, capacity.freeIps(host2)); - assertEquals(0, capacity.freeIps(host3)); + assertEquals(1, capacity.freeIps(host3)); + assertEquals(0, capacity.freeIps(host4)); } @Test - public void freeCapacityOf() { + public void unusedCapacityOf() { assertEquals(new NodeResources(5, 40, 80, 2, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), - capacity.freeCapacityOf(host1, false)); + capacity.unusedCapacityOf(host1)); assertEquals(new NodeResources(5, 60, 80, 4.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), - capacity.freeCapacityOf(host3, false)); + capacity.unusedCapacityOf(host3)); + assertEquals(new NodeResources(7, 100, 120, 5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + capacity.unusedCapacityOf(host4)); doAnswer(invocation -> { NodeResources totalHostResources = ((Flavor) invocation.getArguments()[0]).resources(); @@ -103,9 +112,19 @@ public class HostCapacityTest { }).when(hostResourcesCalculator).advertisedResourcesOf(any()); assertEquals(new NodeResources(4, 38, 77, 1.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), - capacity.freeCapacityOf(host1, false)); + capacity.unusedCapacityOf(host1)); assertEquals(new NodeResources(4, 58, 77, 4, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), - capacity.freeCapacityOf(host3, false)); + capacity.unusedCapacityOf(host3)); + } + + @Test + public void availableCapacityOf() { + assertEquals(new NodeResources(5, 40, 80, 2, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + capacity.availableCapacityOf(host1)); + assertEquals(new NodeResources(5, 60, 80, 4.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + capacity.availableCapacityOf(host3)); + assertEquals(NodeResources.zero(), + capacity.availableCapacityOf(host4)); } @Test |