diff options
author | Jon Bratseth <bratseth@vespa.ai> | 2024-04-24 22:06:09 +0200 |
---|---|---|
committer | Jon Bratseth <bratseth@vespa.ai> | 2024-04-24 22:06:09 +0200 |
commit | d3a158d2b4033bca6302231b849222ac2e68053e (patch) | |
tree | 2bebb832d84a5673dbfecdbc8875fd7cdece4b6b /node-repository | |
parent | e43836f2d47c65382a59c9866b8682bb55751796 (diff) |
Simplify - surplus tracking not needed
Diffstat (limited to 'node-repository')
3 files changed, 30 insertions, 41 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java index d8565b81e41..fa0421555d4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java @@ -54,10 +54,6 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat /** True if this node is allocated on an exclusive network switch in its cluster */ final boolean exclusiveSwitch; - /** True if this node belongs to a group which will not be needed after this deployment */ - // TODO: Always false - final boolean isSurplus; - /** This node does not exist in the node repository yet */ final boolean isNew; @@ -68,7 +64,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat final boolean exclusiveParent; private NodeCandidate(NodeResources freeParentCapacity, Optional<Node> parent, boolean violatesSpares, boolean exclusiveSwitch, - boolean exclusiveParent, boolean isSurplus, boolean isNew, boolean isResizeable) { + boolean exclusiveParent, boolean isNew, boolean isResizeable) { if (isResizeable && isNew) throw new IllegalArgumentException("A new node cannot be resizable"); @@ -77,7 +73,6 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat this.violatesSpares = violatesSpares; this.exclusiveSwitch = exclusiveSwitch; this.exclusiveParent = exclusiveParent; - this.isSurplus = isSurplus; this.isNew = isNew; this.isResizable = isResizeable; } @@ -147,10 +142,6 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat if (this.state() == Node.State.active && other.state() != Node.State.active) return -1; if (other.state() == Node.State.active && this.state() != Node.State.active) return 1; - // Choose active node that is not retired first (surplus is active but retired) - if (!this.isSurplus && other.isSurplus) return -1; - if (!other.isSurplus && this.isSurplus) return 1; - // Choose reserved nodes from a previous allocation attempt (which exist in node repo) if (this.isInNodeRepoAndReserved() && ! other.isInNodeRepoAndReserved()) return -1; if (other.isInNodeRepoAndReserved() && ! this.isInNodeRepoAndReserved()) return 1; @@ -236,7 +227,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat /** Returns a copy of this with node set to given value */ NodeCandidate withNode(Node node, boolean retiredNow) { - return new ConcreteNodeCandidate(node, retiredNow, freeParentCapacity, parent, violatesSpares, exclusiveSwitch, exclusiveParent, isSurplus, isNew, isResizable); + return new ConcreteNodeCandidate(node, retiredNow, freeParentCapacity, parent, violatesSpares, exclusiveSwitch, exclusiveParent, isNew, isResizable); } /** Returns the switch priority, based on switch exclusivity, of this compared to other */ @@ -276,10 +267,9 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat NodeResources freeParentCapacity, Node parent, boolean violatesSpares, - boolean isSurplus, boolean isNew, boolean isResizeable) { - return new ConcreteNodeCandidate(node, false, freeParentCapacity, Optional.of(parent), violatesSpares, true, false, isSurplus, isNew, isResizeable); + return new ConcreteNodeCandidate(node, false, freeParentCapacity, Optional.of(parent), violatesSpares, true, false, isNew, isResizeable); } public static NodeCandidate createNewChild(NodeResources resources, @@ -292,11 +282,11 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat } public static NodeCandidate createNewExclusiveChild(Node node, Node parent) { - return new ConcreteNodeCandidate(node, false, node.resources(), Optional.of(parent), false, true, false, false, true, false); + return new ConcreteNodeCandidate(node, false, node.resources(), Optional.of(parent), false, true, false, true, false); } - public static NodeCandidate createStandalone(Node node, boolean isSurplus, boolean isNew) { - return new ConcreteNodeCandidate(node, false, node.resources(), Optional.empty(), false, true, false, isSurplus, isNew, false); + public static NodeCandidate createStandalone(Node node, boolean isNew) { + return new ConcreteNodeCandidate(node, false, node.resources(), Optional.empty(), false, true, false, isNew, false); } /** A candidate backed by a node */ @@ -309,8 +299,8 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat boolean retiredNow, NodeResources freeParentCapacity, Optional<Node> parent, boolean violatesSpares, boolean exclusiveSwitch, boolean exclusiveParent, - boolean isSurplus, boolean isNew, boolean isResizeable) { - super(freeParentCapacity, parent, violatesSpares, exclusiveSwitch, exclusiveParent, isSurplus, isNew, isResizeable); + boolean isNew, boolean isResizeable) { + super(freeParentCapacity, parent, violatesSpares, exclusiveSwitch, exclusiveParent, isNew, isResizeable); this.retiredNow = retiredNow; this.node = Objects.requireNonNull(node, "Node cannot be null"); } @@ -348,7 +338,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat @Override public NodeCandidate allocate(ApplicationId owner, ClusterMembership membership, NodeResources requestedResources, Instant at) { return new ConcreteNodeCandidate(node.allocate(owner, membership, requestedResources, at), retiredNow, - freeParentCapacity, parent, violatesSpares, exclusiveSwitch, exclusiveParent, isSurplus, isNew, isResizable); + freeParentCapacity, parent, violatesSpares, exclusiveSwitch, exclusiveParent, isNew, isResizable); } /** Called when the node described by this candidate must be created */ @@ -358,13 +348,13 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat @Override public NodeCandidate withExclusiveSwitch(boolean exclusiveSwitch) { return new ConcreteNodeCandidate(node, retiredNow, freeParentCapacity, parent, violatesSpares, exclusiveSwitch, - exclusiveParent, isSurplus, isNew, isResizable); + exclusiveParent, isNew, isResizable); } @Override public NodeCandidate withExclusiveParent(boolean exclusiveParent) { return new ConcreteNodeCandidate(node, retiredNow, freeParentCapacity, parent, violatesSpares, exclusiveSwitch, - exclusiveParent, isSurplus, isNew, isResizable); + exclusiveParent, isNew, isResizable); } @Override @@ -408,7 +398,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat boolean exclusiveParent, LockedNodeList allNodes, IP.Allocation.Context ipAllocationContext) { - super(freeParentCapacity, Optional.of(parent), violatesSpares, exclusiveSwitch, exclusiveParent, false, true, false); + super(freeParentCapacity, Optional.of(parent), violatesSpares, exclusiveSwitch, exclusiveParent, true, false); this.resources = resources; this.allNodes = allNodes; this.ipAllocationContext = ipAllocationContext; @@ -468,7 +458,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat NodeType.tenant) .cloudAccount(parent.get().cloudAccount()) .build(); - return new ConcreteNodeCandidate(node, false, freeParentCapacity, parent, violatesSpares, exclusiveSwitch, exclusiveParent, isSurplus, isNew, isResizable); + return new ConcreteNodeCandidate(node, false, freeParentCapacity, parent, violatesSpares, exclusiveSwitch, exclusiveParent, isNew, isResizable); } @@ -516,7 +506,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat private InvalidNodeCandidate(NodeResources resources, NodeResources freeParentCapacity, Node parent, String invalidReason) { - super(freeParentCapacity, Optional.of(parent), false, false, false, false, true, false); + super(freeParentCapacity, Optional.of(parent), false, false, false, true, false); this.resources = resources; this.invalidReason = invalidReason; } 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 3ff970e5645..65dc6a0c7d0 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 @@ -157,7 +157,7 @@ public class NodePrioritizer { .filter(node -> node.allocation().get().owner().equals(application)) .filter(node -> node.allocation().get().membership().cluster().id().equals(clusterSpec.id())) .filter(node -> node.state() == Node.State.active || canStillAllocate(node)) - .map(node -> candidateFrom(node, false)) + .map(node -> candidateFrom(node)) .forEach(candidates::add); } @@ -166,13 +166,13 @@ public class NodePrioritizer { allNodes.stream() .filter(node -> node.type() == requested.type()) .filter(node -> node.state() == Node.State.ready) - .map(node -> candidateFrom(node, false)) + .map(node -> candidateFrom(node)) .filter(n -> !n.violatesSpares || canAllocateToSpareHosts) .forEach(candidates::add); } /** Create a candidate from given pre-existing node */ - private NodeCandidate candidateFrom(Node node, boolean isSurplus) { + private NodeCandidate candidateFrom(Node node) { Optional<Node> optionalParent = allNodes.parentOf(node); if (optionalParent.isPresent()) { Node parent = optionalParent.get(); @@ -180,7 +180,6 @@ public class NodePrioritizer { capacity.availableCapacityOf(parent), parent, spareHosts.contains(parent), - isSurplus, false, parent.exclusiveToApplicationId().isEmpty() && requested.canResize(node.resources(), @@ -189,7 +188,7 @@ public class NodePrioritizer { topologyChange, currentClusterSize)); } else { - return NodeCandidate.createStandalone(node, isSurplus, false); + return NodeCandidate.createStandalone(node, false); } } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidateTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidateTest.java index 3f5992b2a64..98e8c60094c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidateTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidateTest.java @@ -23,17 +23,17 @@ public class NodeCandidateTest { @Test public void testOrdering() { List<NodeCandidate> expected = List.of( - new NodeCandidate.ConcreteNodeCandidate(node("01", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.empty(), false, true, false, true, false, false), - new NodeCandidate.ConcreteNodeCandidate(node("02", Node.State.active), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, false, false, false), - new NodeCandidate.ConcreteNodeCandidate(node("04", Node.State.reserved), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, false, false, false), - new NodeCandidate.ConcreteNodeCandidate(node("03", Node.State.inactive), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, false, false, false), - new NodeCandidate.ConcreteNodeCandidate(node("05", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.of(node("host1", Node.State.active)), true, true, false, false, true, false), - new NodeCandidate.ConcreteNodeCandidate(node("06", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.of(node("host1", Node.State.ready)), true, true, false, false, true, false), - new NodeCandidate.ConcreteNodeCandidate(node("07", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.of(node("host1", Node.State.provisioned)), true, true, false, false, true, false), - new NodeCandidate.ConcreteNodeCandidate(node("08", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.of(node("host1", Node.State.failed)), true, true, false, false, true, false), - new NodeCandidate.ConcreteNodeCandidate(node("09", Node.State.ready), false, new NodeResources(1, 1, 1, 1), Optional.empty(), true, true, false, false, true, false), - new NodeCandidate.ConcreteNodeCandidate(node("10", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, false, true, false), - new NodeCandidate.ConcreteNodeCandidate(node("11", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, false, true, false) + new NodeCandidate.ConcreteNodeCandidate(node("01", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.empty(), false, true, false, false, false), + new NodeCandidate.ConcreteNodeCandidate(node("02", Node.State.active), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, false, false), + new NodeCandidate.ConcreteNodeCandidate(node("04", Node.State.reserved), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, false, false), + new NodeCandidate.ConcreteNodeCandidate(node("03", Node.State.inactive), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, false, false), + new NodeCandidate.ConcreteNodeCandidate(node("05", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.of(node("host1", Node.State.active)), true, true, false, true, false), + new NodeCandidate.ConcreteNodeCandidate(node("06", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.of(node("host1", Node.State.ready)), true, true, false, true, false), + new NodeCandidate.ConcreteNodeCandidate(node("07", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.of(node("host1", Node.State.provisioned)), true, true, false, true, false), + new NodeCandidate.ConcreteNodeCandidate(node("08", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.of(node("host1", Node.State.failed)), true, true, false, true, false), + new NodeCandidate.ConcreteNodeCandidate(node("09", Node.State.ready), false, new NodeResources(1, 1, 1, 1), Optional.empty(), true, true, false, true, false), + new NodeCandidate.ConcreteNodeCandidate(node("10", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, true, false), + new NodeCandidate.ConcreteNodeCandidate(node("11", Node.State.ready), false, new NodeResources(2, 2, 2, 2), Optional.empty(), true, true, false, true, false) ); assertOrder(expected); } @@ -148,7 +148,7 @@ public class NodeCandidateTest { .ipConfig(IP.Config.of(List.of("::1"), List.of("::2"))) .build(); return new NodeCandidate.ConcreteNodeCandidate(node, false, totalHostResources.subtract(allocatedHostResources), Optional.of(parent), - false, exclusiveSwitch, false, false, true, false); + false, exclusiveSwitch, false, true, false); } private static NodeCandidate node(String hostname, NodeResources nodeResources, |