From 9b1e9cb063fc5d4dbc8c157c746589ec26394bbf Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Sun, 5 Nov 2023 23:11:52 +0100 Subject: Remove exclusive methods from NodeRepository --- .../vespa/hosted/provision/NodeRepository.java | 21 ------------- .../maintenance/HostCapacityMaintainer.java | 17 +++-------- .../provision/provisioning/CapacityPolicies.java | 2 +- .../provision/provisioning/NodeAllocation.java | 6 +--- .../provision/provisioning/NodeCandidate.java | 21 +++++++------ .../provision/provisioning/NodePrioritizer.java | 34 +++++++++------------- .../provisioning/NodeRepositoryProvisioner.java | 2 +- .../provision/provisioning/NodeResourceLimits.java | 6 ++-- .../hosted/provision/provisioning/Preparer.java | 25 +++++++--------- 9 files changed, 45 insertions(+), 89 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java index d31621cf2d5..25919d6a81d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeRepository.java @@ -197,27 +197,6 @@ public class NodeRepository extends AbstractComponent { /** The number of nodes we should ensure has free capacity for node failures whenever possible */ public int spareCount() { return spareCount; } - /** Returns whether nodes must be allocated to hosts that are exclusive to the cluster type. */ - public boolean exclusiveClusterType(AllocationParams params, ClusterSpec cluster) { - return params.sharedHost().hasClusterType(cluster.type().name()); - } - - /** - * Returns whether nodes are allocated exclusively in this instance given this cluster spec. - * Exclusive allocation requires that the wanted node resources matches the advertised resources of the node - * perfectly. - */ - public boolean exclusiveAllocation(AllocationParams params, ClusterSpec clusterSpec) { - return clusterSpec.isExclusive() || - ( clusterSpec.type().isContainer() && zone.system().isPublic() && !zone.environment().isTest() ) || - ( !zone().cloud().allowHostSharing() && !params.sharedHost().supportsClusterType(clusterSpec.type().name())); - } - - /** Whether the nodes of this cluster must be running on hosts that are specifically provisioned for the application. */ - public boolean exclusiveProvisioning(ClusterSpec clusterSpec) { - return !zone.cloud().allowHostSharing() && clusterSpec.isExclusive(); - } - /** * Returns ACLs for the children of the given host. * 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 e8cd2e7b6fe..c51f05496cf 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 @@ -45,6 +45,7 @@ import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import static com.yahoo.vespa.hosted.provision.provisioning.NodeCandidate.ExclusivityViolation.YES; import static java.util.Comparator.comparing; import static java.util.Comparator.naturalOrder; import static java.util.stream.Collectors.groupingBy; @@ -294,21 +295,11 @@ public class HostCapacityMaintainer extends NodeRepositoryMaintainer { var allocationContext = IP.Allocation.Context.from(nodeRepository().zone().cloud().name(), nodeSpec.cloudAccount().isExclave(nodeRepository().zone()), nodeRepository().nameResolver()); - NodePrioritizer prioritizer = new NodePrioritizer(allNodes, params.application(), params.cluster(), nodeSpec, - true, false, allocationContext, nodeRepository().nodes(), - nodeRepository().resourcesCalculator(), nodeRepository().spareCount(), - params.exclusiveAllocation(), params); + NodePrioritizer prioritizer = new NodePrioritizer(params, allNodes, nodeSpec, true, false, allocationContext, nodeRepository().nodes(), + nodeRepository().resourcesCalculator(), nodeRepository().spareCount()); List nodeCandidates = prioritizer.collect() .stream() - .filter(node -> node.violatesExclusivity(params.cluster(), - params.application(), - params.exclusiveClusterType(), - params.exclusiveAllocation(), - params.exclusiveProvisioning(), - nodeRepository().zone().cloud().allowHostSharing(), - allNodes, - params) - != NodeCandidate.ExclusivityViolation.YES) + .filter(node -> node.violatesExclusivity(params, allNodes) != YES) .toList(); MutableInteger index = new MutableInteger(0); return nodeCandidates diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java index 73010e3c47c..1d1be35aee8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/CapacityPolicies.java @@ -102,7 +102,7 @@ public class CapacityPolicies { if (params.cluster().type() == ClusterSpec.Type.admin) { Architecture architecture = adminClusterArchitecture(params.application()); - if (nodeRepository.exclusiveAllocation(params, params.cluster())) { + if (params.exclusiveAllocation()) { return smallestExclusiveResources().with(architecture); } 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 720d4c000b0..e22c039196c 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 @@ -197,11 +197,7 @@ class NodeAllocation { } private NodeCandidate.ExclusivityViolation violatesExclusivity(NodeCandidate candidate) { - return candidate.violatesExclusivity(cluster, application, - nodeRepository.exclusiveClusterType(params, cluster), - nodeRepository.exclusiveAllocation(params, cluster), - nodeRepository.exclusiveProvisioning(cluster), - nodeRepository.zone().cloud().allowHostSharing(), allNodes, params); + return candidate.violatesExclusivity(params, allNodes); } /** 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 f5f83cdfef0..76c8445eb8c 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 @@ -594,9 +594,8 @@ public abstract class NodeCandidate implements Nodelike, Comparable spareHosts; - public NodePrioritizer(LockedNodeList allNodes, ApplicationId application, ClusterSpec clusterSpec, NodeSpec nodeSpec, + public NodePrioritizer(AllocationParams params, LockedNodeList allNodes, NodeSpec nodeSpec, boolean dynamicProvisioning, boolean allowHostSharing, IP.Allocation.Context ipAllocationContext, Nodes nodes, - HostResourcesCalculator hostResourcesCalculator, int spareCount, boolean exclusiveAllocation, AllocationParams params) { + HostResourcesCalculator hostResourcesCalculator, int spareCount) { this.allNodes = allNodes; this.calculator = hostResourcesCalculator; this.capacity = new HostCapacity(this.allNodes, hostResourcesCalculator); this.requested = nodeSpec; - this.clusterSpec = clusterSpec; - this.application = application; this.dynamicProvisioning = dynamicProvisioning; this.allowHostSharing = allowHostSharing; - this.exclusiveAllocation = exclusiveAllocation; this.params = params; this.spareHosts = dynamicProvisioning ? capacity.findSpareHostsInDynamicallyProvisionedZones(this.allNodes.asList()) : @@ -66,7 +60,7 @@ public class NodePrioritizer { this.ipAllocationContext = ipAllocationContext; this.nodes = nodes; - NodeList nodesInCluster = this.allNodes.owner(application).type(clusterSpec.type()).cluster(clusterSpec.id()); + NodeList nodesInCluster = this.allNodes.owner(params.application()).type(params.cluster().type()).cluster(params.cluster().id()); NodeList nonRetiredNodesInCluster = nodesInCluster.not().retired(); long currentGroups = nonRetiredNodesInCluster.state(Node.State.active).stream() .flatMap(node -> node.allocation() @@ -81,7 +75,7 @@ public class NodePrioritizer { // In dynamically provisioned zones, we can always take spare hosts since we can provision new on-demand, // NodeCandidate::compareTo will ensure that they will not be used until there is no room elsewhere. // In non-dynamically provisioned zones, we only allow allocating to spare hosts to replace failed nodes. - this.canAllocateToSpareHosts = dynamicProvisioning || isReplacement(nodesInCluster, clusterSpec.group()); + this.canAllocateToSpareHosts = dynamicProvisioning || isReplacement(nodesInCluster, params.cluster().group()); } /** Collects all node candidates for this application and returns them in the most-to-least preferred order */ @@ -125,19 +119,19 @@ public class NodePrioritizer { for (Node host : allNodes) { if ( ! nodes.canAllocateTenantNodeTo(host, dynamicProvisioning)) continue; if (nodes.suspended(host)) continue; // Hosts that are suspended may be down for some time, e.g. for OS upgrade - if (host.reservedTo().isPresent() && !host.reservedTo().get().equals(application.tenant())) continue; - if (host.reservedTo().isPresent() && application.instance().isTester()) continue; + if (host.reservedTo().isPresent() && !host.reservedTo().get().equals(params.application().tenant())) continue; + if (host.reservedTo().isPresent() && params.application().instance().isTester()) continue; if (params.makeExclusive()) { - if ( ! allowHostSharing && exclusiveAllocation && ! fitsPerfectly(host)) continue; + if ( ! allowHostSharing && params.exclusiveAllocation() && ! fitsPerfectly(host)) continue; } else { if (host.exclusiveToApplicationId().isPresent() && ! fitsPerfectly(host)) continue; } - if ( ! host.provisionedForApplicationId().map(application::equals).orElse(true)) continue; - if ( ! host.exclusiveToApplicationId().map(application::equals).orElse(true)) continue; - if ( ! host.exclusiveToClusterType().map(clusterSpec.type()::equals).orElse(true)) continue; + if ( ! host.provisionedForApplicationId().map(params.application()::equals).orElse(true)) continue; + if ( ! host.exclusiveToApplicationId().map(params.application()::equals).orElse(true)) continue; + if ( ! host.exclusiveToClusterType().map(params.cluster().type()::equals).orElse(true)) continue; if (spareHosts.contains(host) && !canAllocateToSpareHosts) continue; if ( ! capacity.hasCapacity(host, requested.resources().get())) continue; - if ( ! allNodes.childrenOf(host).owner(application).cluster(clusterSpec.id()).isEmpty()) continue; + if ( ! allNodes.childrenOf(host).owner(params.application()).cluster(params.cluster().id()).isEmpty()) continue; if ( ! requested.cloudAccount().isUnspecified() && ! requested.cloudAccount().equals(host.cloudAccount())) continue; candidates.add(NodeCandidate.createNewChild(requested.resources().get(), @@ -160,8 +154,8 @@ public class NodePrioritizer { .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)) - .filter(node -> node.allocation().get().membership().cluster().id().equals(clusterSpec.id())) + .filter(node -> node.allocation().get().owner().equals(params.application())) + .filter(node -> node.allocation().get().membership().cluster().id().equals(params.cluster().id())) .filter(node -> node.state() == Node.State.active || canStillAllocate(node)) .map(node -> candidateFrom(node, false)) .forEach(candidates::add); @@ -191,7 +185,7 @@ public class NodePrioritizer { parent.exclusiveToApplicationId().isEmpty() && requested.canResize(node.resources(), capacity.unusedCapacityOf(parent), - clusterSpec.type(), + params.cluster().type(), topologyChange, currentClusterSize)); } else { 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 4b636baf878..23065055228 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 @@ -280,7 +280,7 @@ public class NodeRepositoryProvisioner implements Provisioner { private IllegalArgumentException newNoAllocationPossible(AllocationParams params, ClusterSpec spec, Limits limits) { StringBuilder message = new StringBuilder("No allocation possible within ").append(limits); - if (nodeRepository.exclusiveAllocation(params, spec) && findNearestNodeResources(limits).isPresent()) + if (params.exclusiveAllocation() && findNearestNodeResources(limits).isPresent()) message.append(". Nearest allowed node resources: ").append(findNearestNodeResources(limits).get()); return new IllegalArgumentException(message.toString()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java index b9f1f89e9c3..f592851f45e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeResourceLimits.java @@ -58,7 +58,7 @@ public class NodeResourceLimits { public boolean isWithinRealLimits(AllocationParams params, NodeResources realResources, ApplicationId applicationId, ClusterSpec cluster) { if (realResources.isUnspecified()) return true; - if (realResources.vcpu() < minRealVcpu(params, applicationId, cluster)) return false; + if (realResources.vcpu() < minRealVcpu(params)) return false; if (realResources.memoryGb() < minRealMemoryGb(cluster)) return false; if (realResources.diskGb() < minRealDiskGb()) return false; return true; @@ -115,8 +115,8 @@ public class NodeResourceLimits { return 4; } - private double minRealVcpu(AllocationParams params, ApplicationId applicationId, ClusterSpec cluster) { - return minAdvertisedVcpu(applicationId, cluster, nodeRepository.exclusiveAllocation(params, cluster)); + private double minRealVcpu(AllocationParams params) { + return minAdvertisedVcpu(params.application(), params.cluster(), params.exclusiveAllocation()); } private static double minRealMemoryGb(ClusterSpec cluster) { 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 58c27bc9802..2f1f39c3301 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 @@ -106,7 +106,7 @@ public class Preparer { NodeAllocation allocation = prepareAllocation(application, cluster, requested, indices::next, allNodes, params); NodeType hostType = allocation.nodeType().hostType(); if (canProvisionDynamically(hostType) && allocation.hostDeficit().isPresent()) { - HostSharing sharing = hostSharing(params, cluster, hostType); + HostSharing sharing = hostSharing(params, hostType); Version osVersion = nodeRepository.osVersions().targetFor(hostType).orElse(Version.emptyVersion); NodeAllocation.HostDeficit deficit = allocation.hostDeficit().get(); Set hosts = new LinkedHashSet<>(); @@ -193,18 +193,16 @@ public class Preparer { var allocationContext = IP.Allocation.Context.from(nodeRepository.zone().cloud().name(), requested.cloudAccount().isExclave(nodeRepository.zone()), nodeRepository.nameResolver()); - NodePrioritizer prioritizer = new NodePrioritizer(allNodes, - application, - cluster, + NodePrioritizer prioritizer = new NodePrioritizer(params, + allNodes, requested, nodeRepository.zone().cloud().dynamicProvisioning(), nodeRepository.zone().cloud().allowHostSharing(), allocationContext, nodeRepository.nodes(), nodeRepository.resourcesCalculator(), - nodeRepository.spareCount(), - nodeRepository.exclusiveAllocation(params, cluster), - params); + nodeRepository.spareCount() + ); allocation.offer(prioritizer.collect()); return allocation; } @@ -231,13 +229,12 @@ public class Preparer { (hostType == NodeType.host || hostType.isConfigServerHostLike()); } - private HostSharing hostSharing(AllocationParams params, ClusterSpec cluster, NodeType hostType) { - if ( hostType.isSharable()) - return nodeRepository.exclusiveProvisioning(cluster) ? HostSharing.provision : - nodeRepository.exclusiveAllocation(params, cluster) ? HostSharing.exclusive : - HostSharing.any; - else - return HostSharing.any; + private HostSharing hostSharing(AllocationParams params, NodeType hostType) { + if (hostType.isSharable()) { + if (params.exclusiveProvisioning()) return HostSharing.provision; + if (params.exclusiveAllocation()) return HostSharing.exclusive; + } + return HostSharing.any; } } -- cgit v1.2.3