From 7f55a0f05ea9ab8d684e278f832a175ab543b9df Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Tue, 9 Jun 2020 17:51:55 +0200 Subject: Spare capacity maintainenance skeleton --- .../vespa/hosted/provision/NodeRepository.java | 14 ++-- .../provision/maintenance/CapacityChecker.java | 41 ++++++----- .../maintenance/CapacityReportMaintainer.java | 58 ---------------- .../maintenance/NodeRepositoryMaintenance.java | 10 +-- .../hosted/provision/maintenance/Rebalancer.java | 2 +- .../maintenance/SpareCapacityMaintainer.java | 81 ++++++++++++++++++++++ .../provision/provisioning/GroupPreparer.java | 2 +- .../provision/restapi/HostCapacityResponse.java | 1 + .../provision/restapi/responses/maintenance.json | 6 +- 9 files changed, 122 insertions(+), 93 deletions(-) delete mode 100644 node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java create mode 100644 node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java (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 b41820a461b..bec35e7ee4f 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 @@ -102,7 +102,7 @@ public class NodeRepository extends AbstractComponent { private final DockerImages dockerImages; private final JobControl jobControl; private final Applications applications; - private final boolean canProvisionHostsWhenRequired; + private final boolean canProvisionHosts; /** * Creates a node repository from a zookeeper provider. @@ -136,7 +136,7 @@ public class NodeRepository extends AbstractComponent { NameResolver nameResolver, DockerImage dockerImage, boolean useCuratorClientCache, - boolean canProvisionHostsWhenRequired) { + boolean canProvisionHosts) { this.db = new CuratorDatabaseClient(flavors, curator, clock, zone, useCuratorClientCache); this.zone = zone; this.clock = clock; @@ -149,7 +149,7 @@ public class NodeRepository extends AbstractComponent { this.dockerImages = new DockerImages(db, dockerImage); this.jobControl = new JobControl(db); this.applications = new Applications(db); - this.canProvisionHostsWhenRequired = canProvisionHostsWhenRequired; + this.canProvisionHosts = canProvisionHosts; // read and write all nodes to make sure they are stored in the latest version of the serialized format for (State state : State.values()) @@ -800,16 +800,14 @@ public class NodeRepository extends AbstractComponent { if (host.status().wantToRetire()) return false; if (host.allocation().map(alloc -> alloc.membership().retired()).orElse(false)) return false; - if ( canProvisionHostsWhenRequired()) + if ( canProvisionHosts()) return EnumSet.of(State.active, State.ready, State.provisioned).contains(host.state()); else return host.state() == State.active; } - /** Returns whether this has the ability to conjure hosts when required */ - public boolean canProvisionHostsWhenRequired() { - return canProvisionHostsWhenRequired; - } + /** Returns whether this repository can provision hosts on demand */ + public boolean canProvisionHosts() { return canProvisionHosts; } /** Returns the time keeper of this system */ public Clock clock() { return clock; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java index ca8399da629..0ab343f0795 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java @@ -11,6 +11,9 @@ import java.util.*; import java.util.function.Function; import java.util.stream.Collectors; +/** + * @author mgimle + */ public class CapacityChecker { private List hosts; @@ -42,15 +45,15 @@ public class CapacityChecker { } public List nodesFromHostnames(List hostnames) { - List nodes = hostnames.stream() - .filter(h -> nodeMap.containsKey(h)) - .map(h -> nodeMap.get(h)) - .collect(Collectors.toList()); + List nodes = hostnames.stream().filter(h -> nodeMap.containsKey(h)) + .map(h -> nodeMap.get(h)) + .collect(Collectors.toList()); + if (nodes.size() != hostnames.size()) { Set notFoundNodes = new HashSet<>(hostnames); notFoundNodes.removeAll(nodes.stream().map(Node::hostname).collect(Collectors.toList())); throw new IllegalArgumentException(String.format("Host(s) not found: [ %s ]", - String.join(", ", notFoundNodes))); + String.join(", ", notFoundNodes))); } return nodes; @@ -92,9 +95,9 @@ public class CapacityChecker { if (hosts.size() == 0) return Optional.empty(); List parentRemovalPriorityList = heuristic.entrySet().stream() - .sorted(Comparator.comparingInt(Map.Entry::getValue)) - .map(Map.Entry::getKey) - .collect(Collectors.toList()); + .sorted(Comparator.comparingInt(Map.Entry::getValue)) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); for (int i = 1; i <= parentRemovalPriorityList.size(); i++) { List hostsToRemove = parentRemovalPriorityList.subList(0, i); @@ -116,12 +119,12 @@ public class CapacityChecker { private Map> constructNodeChildrenMap(List tenants, List hosts, Map hostnameToNode) { Map> nodeChildren = tenants.stream() - .filter(n -> n.parentHostname().isPresent()) - .filter(n -> hostnameToNode.containsKey(n.parentHostname().get())) - .collect(Collectors.groupingBy( - n -> hostnameToNode.get(n.parentHostname().orElseThrow()))); + .filter(n -> n.parentHostname().isPresent()) + .filter(n -> hostnameToNode.containsKey(n.parentHostname().get())) + .collect(Collectors.groupingBy(n -> hostnameToNode.get(n.parentHostname().orElseThrow()))); - for (var host : hosts) nodeChildren.putIfAbsent(host, List.of()); + for (var host : hosts) + nodeChildren.putIfAbsent(host, List.of()); return nodeChildren; } @@ -149,10 +152,8 @@ public class CapacityChecker { private Map computeMaximalRepeatedRemovals(List hosts, Map> nodeChildren, Map availableResources) { - Map timesNodeCanBeRemoved = hosts.stream().collect(Collectors.toMap( - Function.identity(), - __ -> Integer.MAX_VALUE - )); + Map timesNodeCanBeRemoved = hosts.stream().collect(Collectors.toMap(Function.identity(), + __ -> Integer.MAX_VALUE)); for (Node host : hosts) { List children = nodeChildren.get(host); if (children.size() == 0) continue; @@ -326,8 +327,10 @@ public class CapacityChecker { * as well as the specific host and tenant which caused it. */ public static class HostFailurePath { + public List hostsCausingFailure; public HostRemovalFailure failureReason; + } /** @@ -336,6 +339,7 @@ public class CapacityChecker { * will be empty. */ public static class HostRemovalFailure { + public Optional host; public Optional tenant; public AllocationFailureReasonList allocationFailures; @@ -406,6 +410,7 @@ public class CapacityChecker { public AllocationResources subtract(AllocationResources other) { return new AllocationResources(this.nodeResources.subtract(other.nodeResources), this.availableIPs - other.availableIPs); } + } /** @@ -449,6 +454,7 @@ public class CapacityChecker { return String.format("[%s]", String.join(", ", reasons)); } + } /** @@ -487,6 +493,7 @@ public class CapacityChecker { insufficientVcpu(), insufficientMemoryGb(), insufficientDiskGb(), incompatibleDiskSpeed(), incompatibleStorageType(), insufficientAvailableIps(), violatesParentHostPolicy()); } + } public static class AllocationHistory { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java deleted file mode 100644 index f6cadabec54..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityReportMaintainer.java +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.maintenance; - -import com.yahoo.jdisc.Metric; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeRepository; - -import java.time.Duration; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.logging.Level; -import java.util.logging.Logger; -import java.util.stream.Collectors; - -/** - * Performs analysis on the node repository to produce metrics that pertain to the capacity of the node repository. - * These metrics include: - * Spare host capacity, or how many hosts the repository can stand to lose without ending up in a situation where it's - * unable to find a new home for orphaned tenants. - * Overcommitted hosts, which tracks if there are any hosts whose capacity is less than the sum of its children's. - * - * @author mgimle - */ -public class CapacityReportMaintainer extends NodeRepositoryMaintainer { - - private final Metric metric; - private final NodeRepository nodeRepository; - private static final Logger log = Logger.getLogger(CapacityReportMaintainer.class.getName()); - - CapacityReportMaintainer(NodeRepository nodeRepository, - Metric metric, - Duration interval) { - super(nodeRepository, interval); - this.nodeRepository = nodeRepository; - this.metric = Objects.requireNonNull(metric); - } - - @Override - protected void maintain() { - if (nodeRepository.zone().getCloud().dynamicProvisioning()) return; // Hosts and nodes are 1-1 - - CapacityChecker capacityChecker = new CapacityChecker(this.nodeRepository); - List overcommittedHosts = capacityChecker.findOvercommittedHosts(); - if (overcommittedHosts.size() != 0) { - log.log(Level.WARNING, String.format("%d nodes are overcommitted! [ %s ]", overcommittedHosts.size(), - overcommittedHosts.stream().map(Node::hostname).collect(Collectors.joining(", ")))); - } - metric.set("overcommittedHosts", overcommittedHosts.size(), null); - - Optional failurePath = capacityChecker.worstCaseHostLossLeadingToFailure(); - if (failurePath.isPresent()) { - int worstCaseHostLoss = failurePath.get().hostsCausingFailure.size(); - metric.set("spareHostCapacity", worstCaseHostLoss - 1, null); - } - } - -} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index 8a82c74dd17..caf845d36cb 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -47,7 +47,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final InfrastructureProvisioner infrastructureProvisioner; private final Optional loadBalancerExpirer; private final Optional dynamicProvisioningMaintainer; - private final CapacityReportMaintainer capacityReportMaintainer; + private final SpareCapacityMaintainer spareCapacityMaintainer; private final OsUpgradeActivator osUpgradeActivator; private final Rebalancer rebalancer; private final NodeMetricsDbMaintainer nodeMetricsDbMaintainer; @@ -88,7 +88,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { new LoadBalancerExpirer(nodeRepository, defaults.loadBalancerExpirerInterval, lbService)); dynamicProvisioningMaintainer = provisionServiceProvider.getHostProvisioner().map(hostProvisioner -> new DynamicProvisioningMaintainer(nodeRepository, defaults.dynamicProvisionerInterval, hostProvisioner, flagSource)); - capacityReportMaintainer = new CapacityReportMaintainer(nodeRepository, metric, defaults.capacityReportInterval); + spareCapacityMaintainer = new SpareCapacityMaintainer(deployer, nodeRepository, metric, clock, defaults.spareCapacityMaintenanceInterval); osUpgradeActivator = new OsUpgradeActivator(nodeRepository, defaults.osUpgradeActivatorInterval); rebalancer = new Rebalancer(deployer, nodeRepository, metric, clock, defaults.rebalancerInterval); nodeMetricsDbMaintainer = new NodeMetricsDbMaintainer(nodeRepository, nodeMetrics, nodeMetricsDb, defaults.nodeMetricsCollectionInterval); @@ -110,7 +110,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { failedExpirer.close(); dirtyExpirer.close(); nodeRebooter.close(); - capacityReportMaintainer.close(); + spareCapacityMaintainer.close(); provisionedExpirer.close(); metricsReporter.close(); infrastructureProvisioner.close(); @@ -153,7 +153,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final Duration failedExpirerInterval; private final Duration dirtyExpiry; private final Duration provisionedExpiry; - private final Duration capacityReportInterval; + private final Duration spareCapacityMaintenanceInterval; private final Duration metricsInterval; private final Duration retiredInterval; private final Duration infrastructureProvisionInterval; @@ -175,7 +175,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { operatorChangeRedeployInterval = Duration.ofMinutes(1); failedExpirerInterval = Duration.ofMinutes(10); provisionedExpiry = Duration.ofHours(4); - capacityReportInterval = Duration.ofMinutes(10); + spareCapacityMaintenanceInterval = Duration.ofMinutes(10); metricsInterval = Duration.ofMinutes(1); infrastructureProvisionInterval = Duration.ofMinutes(1); throttlePolicy = NodeFailer.ThrottlePolicy.hosted; 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 12990447eee..2cb46a6a78e 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 @@ -41,7 +41,7 @@ public class Rebalancer extends NodeRepositoryMaintainer { @Override protected void maintain() { - if (nodeRepository().canProvisionHostsWhenRequired()) return; // All nodes will be allocated on new hosts, so rebalancing makes no sense + if ( ! nodeRepository().zone().getCloud().allowHostSharing()) return; // Rebalancing not necessary if (nodeRepository().zone().environment().isTest()) return; // Test zones have short lived deployments, no need to rebalance // Work with an unlocked snapshot as this can take a long time and full consistency is not needed 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 new file mode 100644 index 00000000000..05bfac47fb9 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java @@ -0,0 +1,81 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.maintenance; + +import com.yahoo.config.provision.Deployer; +import com.yahoo.jdisc.Metric; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeList; +import com.yahoo.vespa.hosted.provision.NodeRepository; + +import java.time.Clock; +import java.time.Duration; +import java.util.List; +import java.util.Optional; +import java.util.logging.Level; +import java.util.stream.Collectors; + +/** + * A maintainer which attempts to ensure there is spare capacity available in chunks which can fit + * all node resource configuration in use, such that the system is able to quickly replace a failed node + * if necessary. + * + * This also emits the following metrics: + * - Overcommitted hosts: Hosts whose capacity is less than the sum of its children's + * - Spare host capacity, or how many hosts the repository can stand to lose without ending up in a situation where it's + * unable to find a new home for orphaned tenants. + * + * @author mgimle + * @author bratseth + */ +public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { + + private final Deployer deployer; + private final Metric metric; + private final Clock clock; + + public SpareCapacityMaintainer(Deployer deployer, + NodeRepository nodeRepository, + Metric metric, + Clock clock, + Duration interval) { + super(nodeRepository, interval); + this.deployer = deployer; + this.metric = metric; + this.clock = clock; + } + + @Override + protected void maintain() { + if ( ! nodeRepository().zone().getCloud().allowHostSharing()) return; + + CapacityChecker capacityChecker = new CapacityChecker(nodeRepository()); + + List overcommittedHosts = capacityChecker.findOvercommittedHosts(); + if (overcommittedHosts.size() != 0) { + log.log(Level.WARNING, String.format("%d nodes are overcommitted! [ %s ]", + overcommittedHosts.size(), + overcommittedHosts.stream().map(Node::hostname).collect(Collectors.joining(", ")))); + } + metric.set("overcommittedHosts", overcommittedHosts.size(), null); + + Optional failurePath = capacityChecker.worstCaseHostLossLeadingToFailure(); + if (failurePath.isPresent()) { + int worstCaseHostLoss = failurePath.get().hostsCausingFailure.size(); + metric.set("spareHostCapacity", worstCaseHostLoss - 1, null); + if (worstCaseHostLoss <= 1) { + Optional moveCandidate = identifyMoveCandidate(failurePath.get()); + if (moveCandidate.isPresent()) + move(moveCandidate.get()); + } + } + } + + private Optional identifyMoveCandidate(CapacityChecker.HostFailurePath failurePath) { + return Optional.empty(); + } + + private void move(Node node) { + + } + +} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index caecf8edf2f..d3e5f60599f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -58,7 +58,7 @@ public class GroupPreparer { // active config model which is changed on activate public List prepare(ApplicationId application, ClusterSpec cluster, NodeSpec requestedNodes, List surplusActiveNodes, MutableInteger highestIndex, int spareCount, int wantedGroups) { - boolean dynamicProvisioningEnabled = nodeRepository.canProvisionHostsWhenRequired() && nodeRepository.zone().getCloud().dynamicProvisioning(); + boolean dynamicProvisioningEnabled = nodeRepository.canProvisionHosts() && nodeRepository.zone().getCloud().dynamicProvisioning(); boolean allocateFully = dynamicProvisioningEnabled && preprovisionCapacityFlag.value().isEmpty(); try (Mutex lock = nodeRepository.lock(application)) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/HostCapacityResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/HostCapacityResponse.java index 12a29707303..7e81a9cc002 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/HostCapacityResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/HostCapacityResponse.java @@ -20,6 +20,7 @@ import java.util.Optional; * @author mgimle */ public class HostCapacityResponse extends HttpResponse { + private final StringBuilder text; private final Slime slime; private final CapacityChecker capacityChecker; diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json index e041a7b8b54..6bb30d90218 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/maintenance.json @@ -3,9 +3,6 @@ { "name": "AutoscalingMaintainer" }, - { - "name": "CapacityReportMaintainer" - }, { "name": "DirtyExpirer" }, @@ -56,6 +53,9 @@ }, { "name":"ScalingSuggestionsMaintainer" + }, + { + "name": "SpareCapacityMaintainer" } ], "inactive": [ -- cgit v1.2.3 From 49ad958565e4db19872bfb1a069a0209a652c011 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 10 Jun 2020 12:42:51 +0200 Subject: Nonfunctional changes only --- .../com/yahoo/config/provision/zone/ZoneId.java | 2 -- .../jdisc/messagebus/MbusRequestContext.java | 33 +++++++----------- .../docproc/jdisc/messagebus/MessageFactory.java | 14 ++++---- .../messagebus/protocol/DocumentMessage.java | 1 + .../src/main/java/com/yahoo/jdisc/Request.java | 39 ++++++++++------------ .../main/java/com/yahoo/messagebus/Message.java | 23 +++---------- .../maintenance/SpareCapacityMaintainer.java | 5 +-- .../provision/provisioning/NodePrioritizer.java | 2 +- 8 files changed, 45 insertions(+), 74 deletions(-) (limited to 'node-repository') diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java index b0ac59718aa..5795770c294 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/ZoneId.java @@ -1,10 +1,8 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision.zone; -import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; -import com.yahoo.config.provision.SystemName; import java.util.Objects; diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MbusRequestContext.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MbusRequestContext.java index 2da494e19cf..a9bd63d96c3 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MbusRequestContext.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MbusRequestContext.java @@ -75,9 +75,8 @@ public class MbusRequestContext implements RequestContext, ResponseHandler { @Override public void skip() { - if (deserialized.get()) { - throw new IllegalStateException("Can not skip processing after deserialization."); - } + if (deserialized.get()) + throw new IllegalStateException("Can not skip processing after deserialization"); dispatchRequest(requestMsg, request.getUri().getPath(), responseHandler); } @@ -91,10 +90,7 @@ public class MbusRequestContext implements RequestContext, ResponseHandler { } } } - if (log.isLoggable(Level.FINE)) { - log.log(Level.FINE, "Forwarding " + messages.size() + " messages from " + processings.size() + - " processings."); - } + log.log(Level.FINE, () ->"Forwarding " + messages.size() + " messages from " + processings.size() + " processings."); if (messages.isEmpty()) { dispatchResponse(Response.Status.OK); return; @@ -102,10 +98,10 @@ public class MbusRequestContext implements RequestContext, ResponseHandler { long inputSequenceId = requestMsg.getSequenceId(); ResponseMerger responseHandler = new ResponseMerger(requestMsg, messages.size(), this); for (Message message : messages) { - // See comment for internalNoThrottledSource. - dispatchRequest(message, (inputSequenceId == message.getSequenceId()) - ? getUri().getPath() - : "/" + internalNoThrottledSource, + // See comment for internalNoThrottledSource + dispatchRequest(message, + message.getSequenceId() == inputSequenceId ? getUri().getPath() + : "/" + internalNoThrottledSource, responseHandler); } } @@ -177,7 +173,7 @@ public class MbusRequestContext implements RequestContext, ResponseHandler { ResponseDispatch.newInstance(new MbusResponse(status, requestMsg.createReply())).dispatch(this); } - private void dispatchRequest(final Message msg, final String uriPath, final ResponseHandler handler) { + private void dispatchRequest(Message msg, String uriPath, ResponseHandler handler) { try { new RequestDispatch() { @@ -197,15 +193,10 @@ public class MbusRequestContext implements RequestContext, ResponseHandler { } } - private static MessageFactory newMessageFactory(final DocumentMessage msg) { - if (msg == null) { - return null; - } - final Route route = msg.getRoute(); - if (route == null || !route.hasHops()) { - return null; - } - return new MessageFactory(msg); + private static MessageFactory newMessageFactory(DocumentMessage message) { + if (message == null) return null; + if (message.getRoute() == null || ! message.getRoute().hasHops()) return null; + return new MessageFactory(message); } private static URI resolveUri(String path) { diff --git a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MessageFactory.java b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MessageFactory.java index 92f4952b5b5..1365ed955fc 100644 --- a/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MessageFactory.java +++ b/docproc/src/main/java/com/yahoo/docproc/jdisc/messagebus/MessageFactory.java @@ -42,14 +42,12 @@ class MessageFactory { message.setTimeReceivedNow(); message.setTimeRemaining(requestMsg.getTimeRemainingNow()); message.getTrace().setLevel(requestMsg.getTrace().getLevel()); - if (log.isLoggable(Level.FINE)) { - log.log(Level.FINE, "Created '" + message.getClass().getName() + - "', route = '" + message.getRoute() + - "', priority = '" + message.getPriority().name() + - "', load type = '" + message.getLoadType() + - "', trace level = '" + message.getTrace().getLevel() + - "', time remaining = '" + message.getTimeRemaining() + "'."); - } + log.log(Level.FINE, () -> "Created '" + message.getClass().getName() + + "', route = '" + message.getRoute() + + "', priority = '" + message.getPriority().name() + + "', load type = '" + message.getLoadType() + + "', trace level = '" + message.getTrace().getLevel() + + "', time remaining = '" + message.getTimeRemaining() + "'."); return message; } diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentMessage.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentMessage.java index 0db239b33bf..dbe6bb2ff45 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentMessage.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentMessage.java @@ -82,4 +82,5 @@ public abstract class DocumentMessage extends Message { public Utf8String getProtocol() { return DocumentProtocol.NAME; } + } diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java b/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java index 466e74202c1..25194a5502a 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/Request.java @@ -116,20 +116,15 @@ public class Request extends AbstractResource { setUri(uri); } - /** - *

Returns the {@link Container} for which this Request was created.

- * - * @return The container instance. - */ + /** Returns the {@link Container} for which this Request was created */ public Container container() { return parent != null ? parent.container() : container; } /** - *

Returns the Uniform Resource Identifier used by the {@link Container} to resolve the appropriate {@link - * RequestHandler} for this Request.

+ * Returns the Uniform Resource Identifier used by the {@link Container} to resolve the appropriate {@link + * RequestHandler} for this Request * - * @return The resource identifier. * @see #setUri(URI) */ public URI getUri() { @@ -137,12 +132,12 @@ public class Request extends AbstractResource { } /** - *

Sets the Uniform Resource Identifier used by the {@link Container} to resolve the appropriate {@link + * Sets the Uniform Resource Identifier used by the {@link Container} to resolve the appropriate {@link * RequestHandler} for this Request. Because access to the URI is not guarded by any lock, any changes made after - * calling {@link #connect(ResponseHandler)} might never become visible to other threads.

+ * calling {@link #connect(ResponseHandler)} might never become visible to other threads. * - * @param uri The URI to set. - * @return This, to allow chaining. + * @param uri the URI to set + * @return this, to allow chaining * @see #getUri() */ public Request setUri(URI uri) { @@ -151,22 +146,22 @@ public class Request extends AbstractResource { } /** - *

Returns whether or not this Request was created by a {@link ServerProvider}. The value of this is used by - * {@link Container#resolveHandler(Request)} to decide whether to match against server- or client-bindings.

+ * Returns whether or not this Request was created by a {@link ServerProvider}. The value of this is used by + * {@link Container#resolveHandler(Request)} to decide whether to match against server- or client-bindings. * - * @return True, if this is a server request. + * @return true, if this is a server request */ public boolean isServerRequest() { return serverRequest; } /** - *

Sets whether or not this Request was created by a {@link ServerProvider}. The constructor that accepts a + * Sets whether or not this Request was created by a {@link ServerProvider}. The constructor that accepts a * {@link CurrentContainer} sets this to true, whereas the constructor that accepts a parent Request sets - * this to false.

+ * this to false. * - * @param serverRequest Whether or not this is a server request. - * @return This, to allow chaining. + * @param serverRequest whether or not this is a server request + * @return this, to allow chaining * @see #isServerRequest() */ public Request setServerRequest(boolean serverRequest) { @@ -175,13 +170,13 @@ public class Request extends AbstractResource { } /** - *

Returns the last resolved {@link BindingMatch}, or null if none has been resolved yet. This is set + * Returns the last resolved {@link BindingMatch}, or null if none has been resolved yet. This is set * automatically when calling the {@link Container#resolveHandler(Request)} method. The BindingMatch object holds * information about the match of this Request's {@link #getUri() URI} to the {@link UriPattern} of the resolved * {@link RequestHandler}. It allows you to reflect on the parts of the URI that were matched by wildcards in the - * UriPattern.

+ * UriPattern. * - * @return The last resolved BindingMatch, or null. + * @return the last resolved BindingMatch, or null * @see #setBindingMatch(BindingMatch) * @see Container#resolveHandler(Request) */ diff --git a/messagebus/src/main/java/com/yahoo/messagebus/Message.java b/messagebus/src/main/java/com/yahoo/messagebus/Message.java index 43f5c8d2dfd..6848ffc55d8 100644 --- a/messagebus/src/main/java/com/yahoo/messagebus/Message.java +++ b/messagebus/src/main/java/com/yahoo/messagebus/Message.java @@ -47,21 +47,10 @@ public abstract class Message extends Routable { } } - /** - *

Return the route of this routable.

- * - * @return The route. - */ - public Route getRoute() { - return route; - } + /** Returns the route of this routable */ + public Route getRoute() { return route; } - /** - *

Set a new route for this routable.

- * - * @param route The new route. - * @return This, to allow chaining. - */ + /** Sets a new route for this routable */ public Message setRoute(Route route) { this.route = new Route(route); return this; @@ -157,11 +146,9 @@ public abstract class Message extends Routable { } /** - *

Returns the identifier used to order messages. Any two messages that have the same sequence id are ensured to + * Returns the identifier used to order messages. Any two messages that have the same sequence id are ensured to * arrive at the recipient in the order they were sent by the client. This value is only respected if the {@link - * #hasSequenceId()} method returns true.

- * - * @return The sequence identifier. + * #hasSequenceId()} method returns true. */ public long getSequenceId() { return 0; 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 05bfac47fb9..4f7902d4dbc 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 @@ -62,7 +62,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { if (failurePath.isPresent()) { int worstCaseHostLoss = failurePath.get().hostsCausingFailure.size(); metric.set("spareHostCapacity", worstCaseHostLoss - 1, null); - if (worstCaseHostLoss <= 1) { + if (worstCaseHostLoss == 1) { // Try to get back to needing 2 hosts to fail in the worst case Optional moveCandidate = identifyMoveCandidate(failurePath.get()); if (moveCandidate.isPresent()) move(moveCandidate.get()); @@ -71,7 +71,8 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { } private Optional identifyMoveCandidate(CapacityChecker.HostFailurePath failurePath) { - return Optional.empty(); + Node host = failurePath.hostsCausingFailure.get(0); + } private void move(Node node) { 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 8a15c058ff4..5e297900767 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 @@ -206,7 +206,7 @@ public class NodePrioritizer { builder.parent(parent).freeParentCapacity(parentCapacity); if (!isNewNode) - builder.resizable(!allocateFully + builder.resizable(! allocateFully && requestedNodes.canResize(node.flavor().resources(), parentCapacity, isTopologyChange, currentClusterSize)); if (spareHosts.contains(parent)) -- cgit v1.2.3 From 9fc05281d6a79c26efe04edeb7604300f0c05845 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 10 Jun 2020 14:04:29 +0200 Subject: Refactor - no funcntional changes --- .../provision/maintenance/CapacityChecker.java | 3 +- .../maintenance/MaintenanceDeployment.java | 76 +++++++++++++++++++++ .../hosted/provision/maintenance/Rebalancer.java | 77 +++------------------- .../maintenance/SpareCapacityMaintainer.java | 33 ++++++++-- 4 files changed, 116 insertions(+), 73 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java index 0ab343f0795..97253df900b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java @@ -197,7 +197,8 @@ public class CapacityChecker { /** * Tests whether it's possible to remove the provided hosts. * Does not mutate any input variable. - * @return Empty optional if removal is possible, information on what caused the failure otherwise + * + * @return empty optional if removal is possible, information on what caused the failure otherwise */ private Optional findHostRemovalFailure(List hostsToRemove, List allHosts, Map> nodechildren, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java index 18471637da7..b006b2f964b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java @@ -11,6 +11,7 @@ import java.util.logging.Level; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.yolean.Exceptions; import java.io.Closeable; @@ -128,4 +129,79 @@ class MaintenanceDeployment implements Closeable { return "deployment of " + application; } + public static class Move { + + final Node node; + final Node toHost; + + Move(Node node, Node toHost) { + this.node = node; + this.toHost = toHost; + } + + /** + * Try to deploy to make this move. + * + * @return true if the move was done, false if it couldn't be + */ + public boolean execute(Agent agent, Deployer deployer, Metric metric, NodeRepository nodeRepository) { + if (isEmpty()) return false; + ApplicationId application = node.allocation().get().owner(); + try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository)) { + if ( ! deployment.isValid()) return false; + + boolean couldMarkRetiredNow = markWantToRetire(node, true, agent, nodeRepository); + if ( ! couldMarkRetiredNow) return false; + + Optional expectedNewNode = Optional.empty(); + try { + if ( ! deployment.prepare()) return false; + expectedNewNode = + nodeRepository.getNodes(application, Node.State.reserved).stream() + .filter(n -> !n.hostname().equals(node.hostname())) + .filter(n -> n.allocation().get().membership().cluster().id().equals(node.allocation().get().membership().cluster().id())) + .findAny(); + if (expectedNewNode.isEmpty()) return false; + if ( ! expectedNewNode.get().hasParent(toHost.hostname())) return false; + if ( ! deployment.activate()) return false; + + log.info(agent + " redeployed " + application + " to " + this); + return true; + } + finally { + markWantToRetire(node, false, agent, nodeRepository); // Necessary if this failed, no-op otherwise + + // Immediately clean up if we reserved the node but could not activate or reserved a node on the wrong host + expectedNewNode.flatMap(node -> nodeRepository.getNode(node.hostname(), Node.State.reserved)) + .ifPresent(node -> nodeRepository.setDirty(node, Agent.Rebalancer, "Expired by " + agent)); + } + } + } + + /** Returns true only if this operation changes the state of the wantToRetire flag */ + private boolean markWantToRetire(Node node, boolean wantToRetire, Agent agent, NodeRepository nodeRepository) { + try (Mutex lock = nodeRepository.lock(node)) { + Optional nodeToMove = nodeRepository.getNode(node.hostname()); + if (nodeToMove.isEmpty()) return false; + if (nodeToMove.get().state() != Node.State.active) return false; + + if (nodeToMove.get().status().wantToRetire() == wantToRetire) return false; + + nodeRepository.write(nodeToMove.get().withWantToRetire(wantToRetire, agent, nodeRepository.clock().instant()), lock); + return true; + } + } + + public boolean isEmpty() { return node == null; } + + @Override + public String toString() { + return "move " + + ( isEmpty() ? "none" : (node.hostname() + " to " + toHost)); + } + + public static Move empty() { return new Move(null, null); } + + } + } 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 2cb46a6a78e..da3b9bd0e65 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 @@ -42,18 +42,13 @@ public class Rebalancer extends NodeRepositoryMaintainer { @Override protected void maintain() { if ( ! nodeRepository().zone().getCloud().allowHostSharing()) return; // Rebalancing not necessary - if (nodeRepository().zone().environment().isTest()) return; // Test zones have short lived deployments, no need to rebalance + if (nodeRepository().zone().environment().isTest()) return; // Short lived deployments; no need to rebalance // Work with an unlocked snapshot as this can take a long time and full consistency is not needed NodeList allNodes = nodeRepository().list(); - updateSkewMetric(allNodes); - if ( ! zoneIsStable(allNodes)) return; - - Move bestMove = findBestMove(allNodes); - if (bestMove == Move.none) return; - deployTo(bestMove); + findBestMove(allNodes).execute(Agent.Rebalancer, deployer, metric, nodeRepository()); } /** We do this here rather than in MetricsReporter because it is expensive and frequent updates are unnecessary */ @@ -81,7 +76,7 @@ public class Rebalancer extends NodeRepositoryMaintainer { */ private Move findBestMove(NodeList allNodes) { DockerHostCapacity capacity = new DockerHostCapacity(allNodes, nodeRepository().resourcesCalculator()); - Move bestMove = Move.none; + Move bestMove = Move.empty(); for (Node node : allNodes.nodeType(NodeType.tenant).state(Node.State.active)) { if (node.parentHostname().isEmpty()) continue; ApplicationId applicationId = node.allocation().get().owner(); @@ -101,59 +96,6 @@ public class Rebalancer extends NodeRepositoryMaintainer { return bestMove; } - /** Returns true only if this operation changes the state of the wantToRetire flag */ - private boolean markWantToRetire(Node node, boolean wantToRetire) { - try (Mutex lock = nodeRepository().lock(node)) { - Optional nodeToMove = nodeRepository().getNode(node.hostname()); - if (nodeToMove.isEmpty()) return false; - if (nodeToMove.get().state() != Node.State.active) return false; - - if (nodeToMove.get().status().wantToRetire() == wantToRetire) return false; - - nodeRepository().write(nodeToMove.get().withWantToRetire(wantToRetire, Agent.Rebalancer, clock.instant()), lock); - return true; - } - } - - /** - * Try a redeployment to effect the chosen move. - * If it can be done, that's ok; we'll try this or another move later. - * - * @return true if the move was done, false if it couldn't be - */ - private boolean deployTo(Move move) { - ApplicationId application = move.node.allocation().get().owner(); - try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository())) { - if ( ! deployment.isValid()) return false; - - boolean couldMarkRetiredNow = markWantToRetire(move.node, true); - if ( ! couldMarkRetiredNow) return false; - - Optional expectedNewNode = Optional.empty(); - try { - if ( ! deployment.prepare()) return false; - expectedNewNode = - nodeRepository().getNodes(application, Node.State.reserved).stream() - .filter(node -> !node.hostname().equals(move.node.hostname())) - .filter(node -> node.allocation().get().membership().cluster().id().equals(move.node.allocation().get().membership().cluster().id())) - .findAny(); - if (expectedNewNode.isEmpty()) return false; - if ( ! expectedNewNode.get().hasParent(move.toHost.hostname())) return false; - if ( ! deployment.activate()) return false; - - log.info("Rebalancer redeployed " + application + " to " + move); - return true; - } - finally { - markWantToRetire(move.node, false); // Necessary if this failed, no-op otherwise - - // Immediately clean up if we reserved the node but could not activate or reserved a node on the wrong host - expectedNewNode.flatMap(node -> nodeRepository().getNode(node.hostname(), Node.State.reserved)) - .ifPresent(node -> nodeRepository().setDirty(node, Agent.Rebalancer, "Expired by Rebalancer")); - } - } - } - private double skewReductionByRemoving(Node node, Node fromHost, DockerHostCapacity capacity) { NodeResources freeHostCapacity = capacity.freeCapacityOf(fromHost); double skewBefore = Node.skew(fromHost.flavor().resources(), freeHostCapacity); @@ -176,17 +118,12 @@ public class Rebalancer extends NodeRepositoryMaintainer { .orElse(true); } - private static class Move { - - static final Move none = new Move(null, null, 0); + private static class Move extends MaintenanceDeployment.Move { - final Node node; - final Node toHost; final double netSkewReduction; Move(Node node, Node toHost, double netSkewReduction) { - this.node = node; - this.toHost = toHost; + super(node, toHost); this.netSkewReduction = netSkewReduction; } @@ -197,6 +134,10 @@ public class Rebalancer extends NodeRepositoryMaintainer { (node.hostname() + " to " + toHost + " [skew reduction " + netSkewReduction + "]")); } + public static Move empty() { + return new Move(null, null, 0); + } + } } 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 4f7902d4dbc..c0e39b39e94 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 @@ -63,19 +63,44 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { int worstCaseHostLoss = failurePath.get().hostsCausingFailure.size(); metric.set("spareHostCapacity", worstCaseHostLoss - 1, null); if (worstCaseHostLoss == 1) { // Try to get back to needing 2 hosts to fail in the worst case - Optional moveCandidate = identifyMoveCandidate(failurePath.get()); + Optional moveCandidate = identifyMoveCandidate(failurePath.get()); if (moveCandidate.isPresent()) move(moveCandidate.get()); } } } - private Optional identifyMoveCandidate(CapacityChecker.HostFailurePath failurePath) { - Node host = failurePath.hostsCausingFailure.get(0); + private Optional identifyMoveCandidate(CapacityChecker.HostFailurePath failurePath) { + Optional nodeWhichCantMove = failurePath.failureReason.tenant; + if (nodeWhichCantMove.isEmpty()) return Optional.empty(); + return findMoveWhichMakesRoomFor(nodeWhichCantMove.get()); + } + private Optional findMoveWhichMakesRoomFor(Node node) { + return Optional.empty(); } - private void move(Node node) { + private void move(Move move) { + + } + + private static class Move { + + static final Move none = new Move(null, null); + + final Node node; + final Node toHost; + + Move(Node node, Node toHost) { + this.node = node; + this.toHost = toHost; + } + + @Override + public String toString() { + return "move " + + ( node == null ? "none" : (node.hostname() + " to " + toHost)); + } } -- cgit v1.2.3 From 7f7b6777514bf05916e2edcbc3e27b1bfd28906c Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Fri, 12 Jun 2020 12:51:22 +0200 Subject: SpareCapacityMaintainer sketch --- .../com/yahoo/config/provision/package-info.java | 1 - .../document/select/DocumentSelectorTestCase.java | 2 + .../messagebus/MessageBusVisitorSession.java | 1 - .../yahoo/language/opennlp/OpenNlpTokenizer.java | 1 + .../yahoo/language/process/CharacterClasses.java | 3 +- .../com/yahoo/language/process/GramSplitter.java | 83 +++------ .../com/yahoo/language/process/Normalizer.java | 8 +- .../language/process/ProcessingException.java | 2 +- .../com/yahoo/language/process/Transformer.java | 4 +- .../com/yahoo/vespa/hosted/provision/Node.java | 2 + .../com/yahoo/vespa/hosted/provision/NodeList.java | 15 +- .../autoscale/AllocatableClusterResources.java | 12 +- .../maintenance/AutoscalingMaintainer.java | 2 +- .../provision/maintenance/CapacityChecker.java | 10 +- .../maintenance/MaintenanceDeployment.java | 15 +- .../maintenance/NodeRepositoryMaintenance.java | 2 +- .../hosted/provision/maintenance/Rebalancer.java | 29 ++- .../maintenance/SpareCapacityMaintainer.java | 205 ++++++++++++++++++--- .../yahoo/vespa/hosted/provision/node/Agent.java | 3 +- .../hosted/provision/provisioning/Activator.java | 4 +- .../provision/provisioning/DockerHostCapacity.java | 84 --------- .../EmptyProvisionServiceProvider.java | 2 +- .../provision/provisioning/HostCapacity.java | 109 +++++++++++ .../provision/provisioning/NodeAllocation.java | 4 +- .../provision/provisioning/NodePrioritizer.java | 26 +-- .../hosted/provision/provisioning/NodeSpec.java | 2 +- .../provision/provisioning/PrioritizableNode.java | 2 +- .../provision/restapi/ApplicationSerializer.java | 2 +- .../provision/restapi/HostCapacityResponse.java | 2 +- .../provision/autoscale/AutoscalingTest.java | 2 +- .../provision/autoscale/AutoscalingTester.java | 4 +- .../maintenance/CapacityCheckerTester.java | 4 +- .../provision/maintenance/MetricsReporterTest.java | 2 +- .../provision/maintenance/NodeFailerTest.java | 4 +- .../provisioning/AllocationVisualizer.java | 4 +- .../provisioning/DockerHostCapacityTest.java | 138 -------------- .../provisioning/DockerProvisioningTest.java | 4 +- .../provisioning/DynamicDockerAllocationTest.java | 2 +- .../provision/provisioning/HostCapacityTest.java | 138 ++++++++++++++ .../provisioning/InPlaceResizeProvisionTest.java | 6 +- .../provisioning/InfraDeployerImplTest.java | 2 +- .../provision/provisioning/ProvisioningTester.java | 6 +- .../java/com/yahoo/vespavisit/VdsVisitHandler.java | 33 ++-- 43 files changed, 564 insertions(+), 422 deletions(-) delete mode 100644 node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java create mode 100644 node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java delete mode 100644 node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java create mode 100644 node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java (limited to 'node-repository') diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/package-info.java b/config-provisioning/src/main/java/com/yahoo/config/provision/package-info.java index c85c00d8ebe..1eebf6388c7 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/package-info.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/package-info.java @@ -2,5 +2,4 @@ @ExportPackage package com.yahoo.config.provision; -import com.yahoo.api.annotations.PublicApi; import com.yahoo.osgi.annotation.ExportPackage; diff --git a/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java b/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java index 2e08814bf43..5e61f8c7ba0 100644 --- a/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java +++ b/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java @@ -42,6 +42,7 @@ public class DocumentSelectorTestCase { type.addField("hfloat", DataType.FLOAT); type.addField("hstring", DataType.STRING); type.addField("content", DataType.STRING); + type.addField("truth", DataType.BOOL); StructDataType mystruct = new StructDataType("mystruct"); mystruct.addField(new Field("key", DataType.INT)); @@ -87,6 +88,7 @@ public class DocumentSelectorTestCase { assertParse("music_.artist = \"*\""); assertParse("music_foo.artist = \"*\""); assertParse("music_foo_.artist = \"*\""); + assertParse("truth"); assertParse("(4 + 3) > 0", "(4+3) > 0"); assertParse("1 + 1 > 0", "1 +1 > 0"); assertParse("1 + -1 > 0", "1 + -1 > 0"); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusVisitorSession.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusVisitorSession.java index 00a68c99500..69db27686f0 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusVisitorSession.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/MessageBusVisitorSession.java @@ -690,7 +690,6 @@ public class MessageBusVisitorSession implements VisitorSession { private void markSessionCompleted() { // 'done' is only ever written when token mutex is held, so safe to check // outside of completionMonitor lock. - assert(!done) : "Session was marked as completed more than once"; log.log(Level.FINE, "Visitor session '" + sessionName + "' has completed"); if (params.getLocalDataHandler() != null) { params.getLocalDataHandler().onDone(); diff --git a/linguistics/src/main/java/com/yahoo/language/opennlp/OpenNlpTokenizer.java b/linguistics/src/main/java/com/yahoo/language/opennlp/OpenNlpTokenizer.java index d3f6fcf2ee3..93599fa7dbe 100644 --- a/linguistics/src/main/java/com/yahoo/language/opennlp/OpenNlpTokenizer.java +++ b/linguistics/src/main/java/com/yahoo/language/opennlp/OpenNlpTokenizer.java @@ -15,6 +15,7 @@ import java.util.logging.Logger; import java.util.logging.Level; public class OpenNlpTokenizer implements Tokenizer { + private final static int SPACE_CODE = 32; private static final Logger log = Logger.getLogger(OpenNlpTokenizer.class.getName()); private final Normalizer normalizer; diff --git a/linguistics/src/main/java/com/yahoo/language/process/CharacterClasses.java b/linguistics/src/main/java/com/yahoo/language/process/CharacterClasses.java index ce0291c85e5..59ae664e79e 100644 --- a/linguistics/src/main/java/com/yahoo/language/process/CharacterClasses.java +++ b/linguistics/src/main/java/com/yahoo/language/process/CharacterClasses.java @@ -17,7 +17,7 @@ public class CharacterClasses { if (Character.isDigit(c) && ! isLatin(c)) return true; // Not considering these digits, so treat them as letters // if (c == '_') return true; - // Ticket 3864695, some CJK punctuation YST defined as word characters + // Some CJK punctuation defined as word characters if (c == '\u3008' || c == '\u3009' || c == '\u300a' || c == '\u300b' || c == '\u300c' || c == '\u300d' || c == '\u300e' || c == '\u300f' || c == '\u3010' || c == '\u3011') { @@ -52,4 +52,5 @@ public class CharacterClasses { public boolean isLetterOrDigit(int c) { return isLetter(c) || isDigit(c); } + } diff --git a/linguistics/src/main/java/com/yahoo/language/process/GramSplitter.java b/linguistics/src/main/java/com/yahoo/language/process/GramSplitter.java index 94fd0e08493..aa7ae59edf9 100644 --- a/linguistics/src/main/java/com/yahoo/language/process/GramSplitter.java +++ b/linguistics/src/main/java/com/yahoo/language/process/GramSplitter.java @@ -39,12 +39,8 @@ public class GramSplitter { * @throws IllegalArgumentException if n is less than 1 */ public GramSplitterIterator split(String input, int n) { - if (input == null) { - throw new NullPointerException("input cannot be null"); - } - if (n < 1) { - throw new IllegalArgumentException("n (gram size) cannot be smaller than 1, was " + n); - } + if (input == null) throw new NullPointerException("input cannot be null"); + if (n < 1) throw new IllegalArgumentException("n (gram size) cannot be smaller than 1, was " + n); return new GramSplitterIterator(input, n, characterClasses); } @@ -52,29 +48,19 @@ public class GramSplitter { private final CharacterClasses characterClasses; - /** - * Text to split - */ + /** Text to split */ private final String input; - /** - * Gram size - */ + /** Gram size */ private final int n; - /** - * Current index - */ + /** Current index */ private int i = 0; - /** - * Whether the last thing that happened was being on a separator (including the start of the string) - */ + /** Whether the last thing that happened was being on a separator (including the start of the string) */ private boolean isFirstAfterSeparator = true; - /** - * The next gram or null if not determined yet - */ + /** The next gram or null if not determined yet */ private Gram nextGram = null; public GramSplitterIterator(String input, int n, CharacterClasses characterClasses) { @@ -85,9 +71,7 @@ public class GramSplitter { @Override public boolean hasNext() { - if (nextGram != null) { - return true; - } + if (nextGram != null) return true; nextGram = findNext(); return nextGram != null; } @@ -95,12 +79,10 @@ public class GramSplitter { @Override public Gram next() { Gram currentGram = nextGram; - if (currentGram == null) { + if (currentGram == null) currentGram = findNext(); - } - if (currentGram == null) { + if (currentGram == null) throw new NoSuchElementException("No next gram at position " + i); - } nextGram = null; return currentGram; } @@ -111,24 +93,21 @@ public class GramSplitter { i++; isFirstAfterSeparator = true; } - if (i >= input.length()) { - return null; - } + if (i >= input.length()) return null; String gram = input.substring(i, Math.min(i + n, input.length())); int nonWordChar = indexOfNonWordChar(gram); - if (nonWordChar == 0) { - throw new RuntimeException("Programming error"); - } - if (nonWordChar > 0) { + if (nonWordChar == 0) throw new RuntimeException("Programming error"); + + if (nonWordChar > 0) gram = gram.substring(0, nonWordChar); - } if (gram.length() == n) { // normal case: got a full length gram i++; isFirstAfterSeparator = false; return new Gram(i - 1, gram.length()); - } else { // gram is too short due either to a non-word separator or end of string + } + else { // gram is too short due either to a non-word separator or end of string if (isFirstAfterSeparator) { // make a gram anyway i++; isFirstAfterSeparator = false; @@ -143,9 +122,8 @@ public class GramSplitter { private int indexOfNonWordChar(String s) { for (int i = 0; i < s.length(); i++) { - if (!characterClasses.isLetterOrDigit(s.codePointAt(i))) { + if ( ! characterClasses.isLetterOrDigit(s.codePointAt(i))) return i; - } } return -1; } @@ -162,9 +140,8 @@ public class GramSplitter { */ public List toExtractedList() { List gramList = new ArrayList<>(); - while (hasNext()) { + while (hasNext()) gramList.add(next().extractFrom(input)); - } return Collections.unmodifiableList(gramList); } } @@ -189,31 +166,19 @@ public class GramSplitter { return length; } - /** - * Returns this gram as a string from the input string - */ + /** Returns this gram as a string from the input string */ public String extractFrom(String input) { return input.substring(start, start + length); } @Override public boolean equals(Object o) { - if (this == o) { - return true; - } - if (!(o instanceof Gram)) { - return false; - } + if (this == o) return true; + if ( ! (o instanceof Gram)) return false; Gram gram = (Gram)o; - - if (length != gram.length) { - return false; - } - if (start != gram.start) { - return false; - } - + if (length != gram.length) return false; + if (start != gram.start) return false; return true; } @@ -223,5 +188,7 @@ public class GramSplitter { result = 31 * result + length; return result; } + } + } diff --git a/linguistics/src/main/java/com/yahoo/language/process/Normalizer.java b/linguistics/src/main/java/com/yahoo/language/process/Normalizer.java index 0e34f88f4ca..044d249f077 100644 --- a/linguistics/src/main/java/com/yahoo/language/process/Normalizer.java +++ b/linguistics/src/main/java/com/yahoo/language/process/Normalizer.java @@ -9,11 +9,11 @@ package com.yahoo.language.process; public interface Normalizer { /** - *

NFKC normalizes a String.

+ * NFKC normalizes a String. * - * @param input String to normalize. - * @return The normalized String. - * @throws ProcessingException If underlying library throws an Exception. + * @param input the string to normalize + * @return the normalized string + * @throws ProcessingException if underlying library throws an Exception */ String normalize(String input); diff --git a/linguistics/src/main/java/com/yahoo/language/process/ProcessingException.java b/linguistics/src/main/java/com/yahoo/language/process/ProcessingException.java index 941afa07347..752992f5a26 100644 --- a/linguistics/src/main/java/com/yahoo/language/process/ProcessingException.java +++ b/linguistics/src/main/java/com/yahoo/language/process/ProcessingException.java @@ -2,7 +2,7 @@ package com.yahoo.language.process; /** - *

Exception class indicating that a fatal error occured during linguistic processing.

+ * Exception class indicating that a fatal error occured during linguistic processing. * * @author Simon Thoresen Hult */ diff --git a/linguistics/src/main/java/com/yahoo/language/process/Transformer.java b/linguistics/src/main/java/com/yahoo/language/process/Transformer.java index 46f3c060d4e..4927edc98c9 100644 --- a/linguistics/src/main/java/com/yahoo/language/process/Transformer.java +++ b/linguistics/src/main/java/com/yahoo/language/process/Transformer.java @@ -13,8 +13,8 @@ public interface Transformer { /** * Remove accents from input text. * - * @param input text to transform. - * @param language language of input text. + * @param input text to transform + * @param language language of input text * @return text with accents removed, or input-text if the feature is unavailable * @throws ProcessingException thrown if there is an exception stemming this input */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java index a9861497ca3..b6237886dc7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java @@ -128,6 +128,8 @@ public final class Node { return parentHostname.isPresent() && parentHostname.get().equals(hostname); } + public NodeResources resources() { return flavor.resources(); } + /** Returns the flavor of this node */ public Flavor flavor() { return flavor; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java index 1b2f73a2f5f..1cc2abef734 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java @@ -46,13 +46,21 @@ public class NodeList extends AbstractFilteringList { } /** Returns the subset of nodes having exactly the given resources */ - public NodeList resources(NodeResources resources) { return matching(node -> node.flavor().resources().equals(resources)); } + public NodeList resources(NodeResources resources) { return matching(node -> node.resources().equals(resources)); } + + /** Returns the subset of nodes which satisfy the given resources */ + public NodeList satisfies(NodeResources resources) { return matching(node -> node.resources().satisfies(resources)); } /** Returns the subset of nodes of the given flavor */ public NodeList flavor(String flavor) { return matching(node -> node.flavor().name().equals(flavor)); } + /** Returns the subset of nodes not in the given collection */ + public NodeList except(Collection nodes) { + return matching(node -> ! nodes.contains(node)); + } + /** Returns the subset of nodes assigned to the given cluster type */ public NodeList type(ClusterSpec.Type type) { return matching(node -> node.allocation().isPresent() && node.allocation().get().membership().cluster().type().equals(type)); @@ -109,6 +117,11 @@ public class NodeList extends AbstractFilteringList { return matching(node -> nodeTypes.contains(node.type())); } + /** Returns the subset of nodes of the host type */ + public NodeList hosts() { + return matching(node -> node.type() == NodeType.host); + } + /** Returns the subset of nodes that are parents */ public NodeList parents() { return matching(n -> n.parentHostname().isEmpty()); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java index e6cbddf96f2..267bfefa332 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java @@ -47,7 +47,7 @@ public class AllocatableClusterResources { this.nodes = nodes.size(); this.groups = (int)nodes.stream().map(node -> node.allocation().get().membership().cluster().group()).distinct().count(); this.realResources = averageRealResourcesOf(nodes, nodeRepository); // Average since we average metrics over nodes - this.advertisedResources = nodes.get(0).flavor().resources(); + this.advertisedResources = nodes.get(0).resources(); this.clusterType = nodes.get(0).allocation().get().membership().cluster().type(); this.fulfilment = 1; } @@ -125,11 +125,11 @@ public class AllocatableClusterResources { NodeResources sum = new NodeResources(0, 0, 0, 0); for (Node node : nodes) sum = sum.add(nodeRepository.resourcesCalculator().realResourcesOf(node, nodeRepository).justNumbers()); - return nodes.get(0).flavor().resources().justNonNumbers() - .withVcpu(sum.vcpu() / nodes.size()) - .withMemoryGb(sum.memoryGb() / nodes.size()) - .withDiskGb(sum.diskGb() / nodes.size()) - .withBandwidthGbps(sum.bandwidthGbps() / nodes.size()); + return nodes.get(0).resources().justNonNumbers() + .withVcpu(sum.vcpu() / nodes.size()) + .withMemoryGb(sum.memoryGb() / nodes.size()) + .withDiskGb(sum.diskGb() / nodes.size()) + .withBandwidthGbps(sum.bandwidthGbps() / nodes.size()); } public static Optional from(ClusterResources wantedResources, diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java index fa8e8375e23..c32b7854d4e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/AutoscalingMaintainer.java @@ -85,7 +85,7 @@ public class AutoscalingMaintainer extends NodeRepositoryMaintainer { int currentGroups = (int)clusterNodes.stream().map(node -> node.allocation().get().membership().cluster().group()).distinct().count(); ClusterSpec.Type clusterType = clusterNodes.get(0).allocation().get().membership().cluster().type(); log.info("Autoscaling " + application + " " + clusterType + " " + clusterId + ":" + - "\nfrom " + toString(clusterNodes.size(), currentGroups, clusterNodes.get(0).flavor().resources()) + + "\nfrom " + toString(clusterNodes.size(), currentGroups, clusterNodes.get(0).resources()) + "\nto " + toString(target.nodes(), target.groups(), target.nodeResources())); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java index 97253df900b..97810c0b329 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java @@ -136,7 +136,7 @@ public class CapacityChecker { int occupiedIps = 0; Set ipPool = host.ipAddressPool().asSet(); for (var child : nodeChildren.get(host)) { - hostResources = hostResources.subtract(child.flavor().resources().justNumbers()); + hostResources = hostResources.subtract(child.resources().justNumbers()); occupiedIps += child.ipAddresses().stream().filter(ipPool::contains).count(); } availableResources.put(host, new AllocationResources(hostResources, host.ipAddressPool().asSet().size() - occupiedIps)); @@ -250,7 +250,7 @@ public class CapacityChecker { long eligibleParents = hosts.stream().filter(h -> !violatesParentHostPolicy(node, h, containedAllocations) - && availableResources.get(h).satisfies(AllocationResources.from(node.flavor().resources()))).count(); + && availableResources.get(h).satisfies(AllocationResources.from(node.resources()))).count(); allocationHistory.addEntry(node, newParent.get(), eligibleParents + 1); } } @@ -302,7 +302,7 @@ public class CapacityChecker { reason.violatesParentHostPolicy = violatesParentHostPolicy(node, host, containedAllocations); NodeResources l = availableHostResources.nodeResources; - NodeResources r = node.allocation().map(Allocation::requestedResources).orElse(node.flavor().resources()); + NodeResources r = node.allocation().map(Allocation::requestedResources).orElse(node.resources()); if (l.vcpu() < r.vcpu()) reason.insufficientVcpu = true; @@ -391,7 +391,7 @@ public class CapacityChecker { if (node.allocation().isPresent()) return from(node.allocation().get().requestedResources()); else - return from(node.flavor().resources()); + return from(node.resources()); } public static AllocationResources from(NodeResources nodeResources) { @@ -514,7 +514,7 @@ public class CapacityChecker { public String toString() { return String.format("%-20s %-65s -> %15s [%3d valid]", tenant.hostname().replaceFirst("\\..+", ""), - tenant.flavor().resources(), + tenant.resources(), newParent == null ? "x" : newParent.hostname().replaceFirst("\\..+", ""), this.eligibleParents ); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java index b006b2f964b..db331e88d64 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java @@ -131,14 +131,19 @@ class MaintenanceDeployment implements Closeable { public static class Move { - final Node node; - final Node toHost; + private final Node node; + private final Node fromHost, toHost; - Move(Node node, Node toHost) { + Move(Node node, Node fromHost, Node toHost) { this.node = node; + this.fromHost = fromHost; this.toHost = toHost; } + public Node node() { return node; } + public Node fromHost() { return fromHost; } + public Node toHost() { return toHost; } + /** * Try to deploy to make this move. * @@ -197,10 +202,10 @@ class MaintenanceDeployment implements Closeable { @Override public String toString() { return "move " + - ( isEmpty() ? "none" : (node.hostname() + " to " + toHost)); + ( isEmpty() ? "none" : (node.hostname() + " from " + fromHost + " to " + toHost)); } - public static Move empty() { return new Move(null, null); } + public static Move empty() { return new Move(null, null, null); } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index caf845d36cb..c3f8afae4a4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -88,7 +88,7 @@ public class NodeRepositoryMaintenance extends AbstractComponent { new LoadBalancerExpirer(nodeRepository, defaults.loadBalancerExpirerInterval, lbService)); dynamicProvisioningMaintainer = provisionServiceProvider.getHostProvisioner().map(hostProvisioner -> new DynamicProvisioningMaintainer(nodeRepository, defaults.dynamicProvisionerInterval, hostProvisioner, flagSource)); - spareCapacityMaintainer = new SpareCapacityMaintainer(deployer, nodeRepository, metric, clock, defaults.spareCapacityMaintenanceInterval); + spareCapacityMaintainer = new SpareCapacityMaintainer(deployer, nodeRepository, metric, defaults.spareCapacityMaintenanceInterval); osUpgradeActivator = new OsUpgradeActivator(nodeRepository, defaults.osUpgradeActivatorInterval); rebalancer = new Rebalancer(deployer, nodeRepository, metric, clock, defaults.rebalancerInterval); nodeMetricsDbMaintainer = new NodeMetricsDbMaintainer(nodeRepository, nodeMetrics, nodeMetricsDb, defaults.nodeMetricsCollectionInterval); 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 da3b9bd0e65..e10ac68fde0 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 @@ -6,16 +6,14 @@ 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.transaction.Mutex; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Agent; -import com.yahoo.vespa.hosted.provision.provisioning.DockerHostCapacity; +import com.yahoo.vespa.hosted.provision.provisioning.HostCapacity; import java.time.Clock; import java.time.Duration; -import java.util.Optional; /** * @author bratseth @@ -53,7 +51,7 @@ public class Rebalancer extends NodeRepositoryMaintainer { /** We do this here rather than in MetricsReporter because it is expensive and frequent updates are unnecessary */ private void updateSkewMetric(NodeList allNodes) { - DockerHostCapacity capacity = new DockerHostCapacity(allNodes, nodeRepository().resourcesCalculator()); + HostCapacity capacity = new HostCapacity(allNodes, nodeRepository().resourcesCalculator()); double totalSkew = 0; int hostCount = 0; for (Node host : allNodes.nodeType((NodeType.host)).state(Node.State.active)) { @@ -75,7 +73,7 @@ public class Rebalancer extends NodeRepositoryMaintainer { * Returns Move.none if no moves can be made to reduce skew. */ private Move findBestMove(NodeList allNodes) { - DockerHostCapacity capacity = new DockerHostCapacity(allNodes, nodeRepository().resourcesCalculator()); + HostCapacity capacity = new HostCapacity(allNodes, nodeRepository().resourcesCalculator()); Move bestMove = Move.empty(); for (Node node : allNodes.nodeType(NodeType.tenant).state(Node.State.active)) { if (node.parentHostname().isEmpty()) continue; @@ -84,29 +82,29 @@ public class Rebalancer extends NodeRepositoryMaintainer { if (deployedRecently(applicationId)) continue; for (Node toHost : allNodes.matching(nodeRepository()::canAllocateTenantNodeTo)) { if (toHost.hostname().equals(node.parentHostname().get())) continue; - if ( ! capacity.freeCapacityOf(toHost).satisfies(node.flavor().resources())) continue; + if ( ! capacity.freeCapacityOf(toHost).satisfies(node.resources())) continue; double skewReductionAtFromHost = skewReductionByRemoving(node, allNodes.parentOf(node).get(), capacity); double skewReductionAtToHost = skewReductionByAdding(node, toHost, capacity); double netSkewReduction = skewReductionAtFromHost + skewReductionAtToHost; if (netSkewReduction > bestMove.netSkewReduction) - bestMove = new Move(node, toHost, netSkewReduction); + bestMove = new Move(node, nodeRepository().getNode(node.parentHostname().get()).get(), toHost, netSkewReduction); } } return bestMove; } - private double skewReductionByRemoving(Node node, Node fromHost, DockerHostCapacity capacity) { + private double skewReductionByRemoving(Node node, Node fromHost, HostCapacity capacity) { NodeResources freeHostCapacity = capacity.freeCapacityOf(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, DockerHostCapacity capacity) { + private double skewReductionByAdding(Node node, Node toHost, HostCapacity capacity) { NodeResources freeHostCapacity = capacity.freeCapacityOf(toHost); double skewBefore = Node.skew(toHost.flavor().resources(), freeHostCapacity); - double skewAfter = Node.skew(toHost.flavor().resources(), freeHostCapacity.subtract(node.flavor().resources().justNumbers())); + double skewAfter = Node.skew(toHost.flavor().resources(), freeHostCapacity.subtract(node.resources().justNumbers())); return skewBefore - skewAfter; } @@ -122,20 +120,19 @@ public class Rebalancer extends NodeRepositoryMaintainer { final double netSkewReduction; - Move(Node node, Node toHost, double netSkewReduction) { - super(node, toHost); + Move(Node node, Node fromHost, Node toHost, double netSkewReduction) { + super(node, fromHost, toHost); this.netSkewReduction = netSkewReduction; } @Override public String toString() { - return "move " + - ( node == null ? "none" : - (node.hostname() + " to " + toHost + " [skew reduction " + netSkewReduction + "]")); + if (isEmpty()) return "move none"; + return super.toString() + " [skew reduction " + netSkewReduction + "]"; } public static Move empty() { - return new Move(null, null, 0); + return new Move(null, null, null, 0); } } 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 c0e39b39e94..4179d6d3f83 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 @@ -2,15 +2,21 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.config.provision.Deployer; +import com.yahoo.config.provision.NodeResources; import com.yahoo.jdisc.Metric; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.maintenance.MaintenanceDeployment.Move; +import com.yahoo.vespa.hosted.provision.node.Agent; +import com.yahoo.vespa.hosted.provision.provisioning.HostCapacity; -import java.time.Clock; import java.time.Duration; +import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.logging.Level; import java.util.stream.Collectors; @@ -29,19 +35,18 @@ import java.util.stream.Collectors; */ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { + private static final int maxMoves = 5; + private final Deployer deployer; private final Metric metric; - private final Clock clock; public SpareCapacityMaintainer(Deployer deployer, NodeRepository nodeRepository, Metric metric, - Clock clock, Duration interval) { super(nodeRepository, interval); this.deployer = deployer; this.metric = metric; - this.clock = clock; } @Override @@ -60,46 +65,194 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { Optional failurePath = capacityChecker.worstCaseHostLossLeadingToFailure(); if (failurePath.isPresent()) { - int worstCaseHostLoss = failurePath.get().hostsCausingFailure.size(); - metric.set("spareHostCapacity", worstCaseHostLoss - 1, null); - if (worstCaseHostLoss == 1) { // Try to get back to needing 2 hosts to fail in the worst case - Optional moveCandidate = identifyMoveCandidate(failurePath.get()); - if (moveCandidate.isPresent()) - move(moveCandidate.get()); + int spareHostCapacity = failurePath.get().hostsCausingFailure.size() - 1; + if (spareHostCapacity == 0) { + Move move = findMitigatingMove(failurePath.get()); + boolean success = move.execute(Agent.SpareCapacityMaintainer, deployer, metric, nodeRepository()); + if (success) { + // This may strictly be a (noble) lie: We succeeded in taking one step to mitigate, + // but not necessarily *all* steps needed to justify adding 1 to spare capacity. + // However, we alert on this being 0 and we don't want to do that as long as we're not stuck. + spareHostCapacity++; + } } + metric.set("spareHostCapacity", spareHostCapacity, null); } } - private Optional identifyMoveCandidate(CapacityChecker.HostFailurePath failurePath) { + private Move findMitigatingMove(CapacityChecker.HostFailurePath failurePath) { Optional nodeWhichCantMove = failurePath.failureReason.tenant; - if (nodeWhichCantMove.isEmpty()) return Optional.empty(); - return findMoveWhichMakesRoomFor(nodeWhichCantMove.get()); + if (nodeWhichCantMove.isEmpty()) return Move.empty(); + return moveTowardsSpareFor(nodeWhichCantMove.get()); } - private Optional findMoveWhichMakesRoomFor(Node node) { - return Optional.empty(); + private Move moveTowardsSpareFor(Node node) { + NodeList allNodes = nodeRepository().list(); + // Allocation will assign the two most empty nodes as "spares", which will not be allocated on + // unless needed for node failing. Our goal here is to make room on these spares for the given node + HostCapacity hostCapacity = new HostCapacity(allNodes, nodeRepository().resourcesCalculator()); + Set spareHosts = hostCapacity.findSpareHosts(allNodes.hosts().satisfies(node.resources()).asList(), 2); + List hosts = allNodes.hosts().except(spareHosts).asList(); + + CapacitySolver capacitySolver = new CapacitySolver(hostCapacity); + List shortestMitigation = null; + for (Node spareHost : spareHosts) { + List mitigation = capacitySolver.makeRoomFor(node, spareHost, hosts, List.of(), maxMoves); + if (mitigation == null) continue; + if (shortestMitigation == null || shortestMitigation.size() > mitigation.size()) + shortestMitigation = mitigation; + } + if (shortestMitigation == null || shortestMitigation.isEmpty()) return Move.empty(); + return shortestMitigation.get(0); } - private void move(Move move) { + private static class CapacitySolver { + + private final HostCapacity hostCapacity; + + CapacitySolver(HostCapacity hostCapacity) { + this.hostCapacity = hostCapacity; + } + + /** + * Finds the shortest sequence of moves which makes room for the given node on the given host, + * assuming the given moves already made over the given hosts' current allocation. + * + * @param node the node to make room for + * @param host the target host to make room on + * @param hosts the hosts onto which we can move nodes + * @param movesMade the moves already made in this scenario + * @return the list of movesMade with the moves needed for this appended, in the order they should be performed, + * or null if no sequence could be found + */ + List makeRoomFor(Node node, Node host, List hosts, List movesMade, int movesLeft) { + if ( ! host.resources().satisfies(node.resources())) return null; + NodeResources freeCapacity = freeCapacityWith(movesMade, host); + if (freeCapacity.satisfies(node.resources())) return List.of(); + if (movesLeft == 0) return null; + + List shortest = null; + for (var i = Subsets(hostCapacity.allNodes().childrenOf(host), movesLeft); i.hasNext(); ) { + List childrenToMove = i.next(); + if ( ! addResourcesOf(childrenToMove, freeCapacity).satisfies(node.resources())) continue; + List moves = move(childrenToMove, host, hosts, movesMade, movesLeft); + if (moves == null) continue; + if (shortest == null || moves.size() < shortest.size()) + shortest = moves; + } + if (shortest == null) return null; + List total = append(movesMade, shortest); + if (total.size() > movesLeft) return null; + return total; + } + + private List move(List nodes, Node host, List hosts, List movesMade, int movesLeft) { + List moves = new ArrayList<>(); + for (Node childToMove : nodes) { + List childMoves = move(childToMove, host, hosts, append(movesMade, moves), movesLeft - moves.size()); + if (childMoves == null) return null; + moves.addAll(childMoves); + if (moves.size() > movesLeft) return null; + } + return moves; + } + + private List move(Node node, Node host, List hosts, List movesMade, int movesLeft) { + List shortest = null; + for (Node target : hosts) { + List childMoves = makeRoomFor(node, target, hosts, movesMade, movesLeft - 1); + if (childMoves == null) continue; + if (shortest == null || shortest.size() > childMoves.size() + 1) { + shortest = new ArrayList<>(childMoves); + shortest.add(new Move(node, host, target)); + } + } + return shortest; + } + + private NodeResources addResourcesOf(List nodes, NodeResources resources) { + for (Node node : nodes) + resources = resources.add(node.resources()); + return resources; + } + + private Iterator> Subsets(NodeList nodes, int maxLength) { + return new SubsetIterator(nodes.asList(), maxLength); + } + + private List append(List a, List b) { + List list = new ArrayList<>(); + list.addAll(a); + list.addAll(b); + return list; + } + + private NodeResources freeCapacityWith(List moves, Node host) { + NodeResources resources = hostCapacity.freeCapacityOf(host); + for (Move move : moves) { + if ( ! move.toHost().equals(host)) continue; + resources = resources.subtract(move.node().resources()); + } + for (Move move : moves) { + if ( ! move.fromHost().equals(host)) continue; + resources = resources.add(move.fromHost().resources()); + } + return resources; + } } - private static class Move { + private static class SubsetIterator implements Iterator> { + + private final List nodes; + private final int maxLength; + + // A number whose binary representation determines which items of list we'll include + private int i = 0; // first "previous" = 0 -> skip the empty set + private List next = null; - static final Move none = new Move(null, null); + public SubsetIterator(List nodes, int maxLength) { + this.nodes = new ArrayList<>(nodes.subList(0, Math.min(nodes.size(), 31))); + this.maxLength = maxLength; + } + + @Override + public boolean hasNext() { + if (next != null) return true; - final Node node; - final Node toHost; + // find next + while (++i < 1< maxLength) continue; - Move(Node node, Node toHost) { - this.node = node; - this.toHost = toHost; + next = new ArrayList<>(ones); + for (int position = 0; position < nodes.size(); position++) { + if (hasOneAtPosition(position, i)) + next.add(nodes.get(position)); + } + return true; + } + return false; } @Override - public String toString() { - return "move " + - ( node == null ? "none" : (node.hostname() + " to " + toHost)); + public List next() { + next = null; + if ( ! hasNext()) throw new IllegalStateException("No more elements"); + return next; + } + + private boolean hasOneAtPosition(int position, int number) { + return (number & (1 << position)) > 0; + } + + private int onesIn(int number) { + int ones = 0; + for (int position = 0; Math.pow(2, position) <= number; position++) { + if (hasOneAtPosition(position, number)) + ones++; + } + return ones; } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java index 31b7181a58a..eba9e4a1ac9 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Agent.java @@ -21,6 +21,7 @@ public enum Agent { ProvisionedExpirer, ReservationExpirer, DynamicProvisioningMaintainer, - RetiringUpgrader; + RetiringUpgrader, + SpareCapacityMaintainer } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java index ebe9327967e..7ad69d673e7 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/Activator.java @@ -184,12 +184,12 @@ class Activator { for (Node node : nodes) { HostSpec hostSpec = getHost(node.hostname(), hosts); node = hostSpec.membership().get().retired() ? node.retire(nodeRepository.clock().instant()) : node.unretire(); - if (! hostSpec.advertisedResources().equals(node.flavor().resources())) // A resized node + if (! hostSpec.advertisedResources().equals(node.resources())) // A resized node node = node.with(new Flavor(hostSpec.advertisedResources())); Allocation allocation = node.allocation().get() .with(hostSpec.membership().get()) .withRequestedResources(hostSpec.requestedResources() - .orElse(node.flavor().resources())); + .orElse(node.resources())); if (hostSpec.networkPorts().isPresent()) allocation = allocation.withNetworkPorts(hostSpec.networkPorts().get()); node = node.with(allocation); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java deleted file mode 100644 index b508198db3a..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacity.java +++ /dev/null @@ -1,84 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.provisioning; - -import com.yahoo.config.provision.NodeResources; -import com.yahoo.config.provision.NodeType; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.NodeList; - -import java.util.Objects; - -/** - * Capacity calculation for docker hosts. - *

- * The calculations are based on an immutable copy of nodes that represents - * all capacities in the system - i.e. all nodes in the node repo. - * - * @author smorgrav - */ -public class DockerHostCapacity { - - private final NodeList allNodes; - private final HostResourcesCalculator hostResourcesCalculator; - - public DockerHostCapacity(NodeList allNodes, HostResourcesCalculator hostResourcesCalculator) { - this.allNodes = Objects.requireNonNull(allNodes, "allNodes must be non-null"); - this.hostResourcesCalculator = Objects.requireNonNull(hostResourcesCalculator, "hostResourcesCalculator must be non-null"); - } - - int compareWithoutInactive(Node hostA, Node hostB) { - int result = compare(freeCapacityOf(hostB, true), freeCapacityOf(hostA, true)); - if (result != 0) return result; - - // If resources are equal we want to assign to the one with the most IP addresses free - return freeIPs(hostB) - freeIPs(hostA); - } - - private int compare(NodeResources a, NodeResources b) { - return NodeResourceComparator.defaultOrder().compare(a, b); - } - - /** - * Checks the node capacity and free ip addresses to see - * if we could allocate a flavor on the docker host. - */ - boolean hasCapacity(Node dockerHost, NodeResources requestedCapacity) { - return freeCapacityOf(dockerHost, false).satisfies(requestedCapacity) && freeIPs(dockerHost) > 0; - } - - /** - * Number of free (not allocated) IP addresses assigned to the dockerhost. - */ - int freeIPs(Node dockerHost) { - return dockerHost.ipAddressPool().findUnused(allNodes).size(); - } - - /** - * Calculate the remaining capacity of a host. - * - * @param host the host to find free capacity of. - * @return a default (empty) capacity if not a docker host, otherwise the free/unallocated/rest capacity - */ - public NodeResources freeCapacityOf(Node host) { - return freeCapacityOf(host, false); - } - - NodeResources freeCapacityOf(Node host, boolean excludeInactive) { - // Only hosts have free capacity - if ( ! host.type().canRun(NodeType.tenant)) return new NodeResources(0, 0, 0, 0); - - NodeResources hostResources = hostResourcesCalculator.advertisedResourcesOf(host.flavor()); - return allNodes.childrenOf(host).asList().stream() - .filter(node -> !(excludeInactive && isInactiveOrRetired(node))) - .map(node -> node.flavor().resources().justNumbers()) - .reduce(hostResources.justNumbers(), NodeResources::subtract) - .with(host.flavor().resources().diskSpeed()).with(host.flavor().resources().storageType()); - } - - private static boolean isInactiveOrRetired(Node node) { - if (node.state() == Node.State.inactive) return true; - if (node.allocation().isPresent() && node.allocation().get().membership().retired()) return true; - return false; - } - -} diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/EmptyProvisionServiceProvider.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/EmptyProvisionServiceProvider.java index e4b1e0fcbc0..004c74b5f70 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/EmptyProvisionServiceProvider.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/EmptyProvisionServiceProvider.java @@ -34,7 +34,7 @@ public class EmptyProvisionServiceProvider implements ProvisionServiceProvider { private static class IdentityHostResourcesCalculator implements HostResourcesCalculator { @Override - public NodeResources realResourcesOf(Node node, NodeRepository repository) { return node.flavor().resources(); } + public NodeResources realResourcesOf(Node node, NodeRepository repository) { return node.resources(); } @Override public NodeResources advertisedResourcesOf(Flavor flavor) { return flavor.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 new file mode 100644 index 00000000000..4125345d492 --- /dev/null +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java @@ -0,0 +1,109 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.provisioning; + +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; +import com.yahoo.vespa.hosted.provision.LockedNodeList; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeList; + +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Capacity calculation for docker hosts. + *

+ * The calculations are based on an immutable copy of nodes that represents + * all capacities in the system - i.e. all nodes in the node repo. + * + * @author smorgrav + */ +public class HostCapacity { + + private final NodeList allNodes; + private final HostResourcesCalculator hostResourcesCalculator; + + public HostCapacity(NodeList allNodes, HostResourcesCalculator hostResourcesCalculator) { + this.allNodes = Objects.requireNonNull(allNodes, "allNodes must be non-null"); + this.hostResourcesCalculator = Objects.requireNonNull(hostResourcesCalculator, "hostResourcesCalculator must be non-null"); + } + + public NodeList allNodes() { return allNodes; } + + /** + * Spare hosts are the two hosts in the system with the most free capacity. + * + * We do not count retired or inactive nodes as used capacity (as they could have been + * moved to create space for the spare node in the first place). + * + * @param candidates the candidates to consider. This list may contain all kinds of nodes. + * @param count the max number of spare hosts to return + */ + public Set findSpareHosts(List candidates, int count) { + return candidates.stream() + .filter(node -> node.type() == NodeType.host) + .filter(dockerHost -> dockerHost.state() == Node.State.active) + .filter(dockerHost -> freeIPs(dockerHost) > 0) + .sorted(this::compareWithoutInactive) + .limit(count) + .collect(Collectors.toSet()); + } + + private int compareWithoutInactive(Node hostA, Node hostB) { + int result = compare(freeCapacityOf(hostB, true), freeCapacityOf(hostA, true)); + if (result != 0) return result; + + // If resources are equal we want to assign to the one with the most IP addresses free + return freeIPs(hostB) - freeIPs(hostA); + } + + private int compare(NodeResources a, NodeResources b) { + return NodeResourceComparator.defaultOrder().compare(a, b); + } + + /** + * Checks the node capacity and free ip addresses to see + * if we could allocate a flavor on the docker host. + */ + boolean hasCapacity(Node dockerHost, NodeResources requestedCapacity) { + return freeCapacityOf(dockerHost, false).satisfies(requestedCapacity) && freeIPs(dockerHost) > 0; + } + + /** + * Number of free (not allocated) IP addresses assigned to the dockerhost. + */ + int freeIPs(Node dockerHost) { + return dockerHost.ipAddressPool().findUnused(allNodes).size(); + } + + /** + * Calculate the remaining capacity of a host. + * + * @param host the host to find free capacity of. + * @return a default (empty) capacity if not a docker host, otherwise the free/unallocated/rest capacity + */ + public NodeResources freeCapacityOf(Node host) { + return freeCapacityOf(host, false); + } + + NodeResources freeCapacityOf(Node host, boolean excludeInactive) { + // Only hosts have free capacity + if ( ! host.type().canRun(NodeType.tenant)) return new NodeResources(0, 0, 0, 0); + + NodeResources hostResources = hostResourcesCalculator.advertisedResourcesOf(host.flavor()); + return allNodes.childrenOf(host).asList().stream() + .filter(node -> !(excludeInactive && isInactiveOrRetired(node))) + .map(node -> node.flavor().resources().justNumbers()) + .reduce(hostResources.justNumbers(), NodeResources::subtract) + .with(host.flavor().resources().diskSpeed()).with(host.flavor().resources().storageType()); + } + + private static boolean isInactiveOrRetired(Node node) { + if (node.state() == Node.State.inactive) return true; + if (node.allocation().isPresent() && node.allocation().get().membership().retired()) return true; + return false; + } + +} 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 47d1b30a8e7..df8a7e45917 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 @@ -148,7 +148,7 @@ class NodeAllocation { } node.node = offered.allocate(application, ClusterMembership.from(cluster, highestIndex.add(1)), - requestedNodes.resources().orElse(node.node.flavor().resources()), + requestedNodes.resources().orElse(node.node.resources()), nodeRepository.clock().instant()); accepted.add(acceptNode(node, false, false)); } @@ -242,7 +242,7 @@ class NodeAllocation { Node node = prioritizableNode.node; if (node.allocation().isPresent()) // Record the currently requested resources - node = node.with(node.allocation().get().withRequestedResources(requestedNodes.resources().orElse(node.flavor().resources()))); + node = node.with(node.allocation().get().withRequestedResources(requestedNodes.resources().orElse(node.resources()))); if (! wantToRetire) { accepted++; 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 5e297900767..8560dd424e7 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 @@ -13,7 +13,6 @@ import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.IP; -import com.yahoo.vespa.hosted.provision.persistence.NameResolver; import java.util.EnumSet; import java.util.HashMap; @@ -21,7 +20,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.function.Predicate; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -39,7 +37,7 @@ public class NodePrioritizer { private final Map nodes = new HashMap<>(); private final LockedNodeList allNodes; - private final DockerHostCapacity capacity; + private final HostCapacity capacity; private final NodeSpec requestedNodes; private final ApplicationId application; private final ClusterSpec clusterSpec; @@ -55,11 +53,11 @@ public class NodePrioritizer { NodePrioritizer(LockedNodeList allNodes, ApplicationId application, ClusterSpec clusterSpec, NodeSpec nodeSpec, int spares, int wantedGroups, boolean allocateFully, NodeRepository nodeRepository) { this.allNodes = allNodes; - this.capacity = new DockerHostCapacity(allNodes, nodeRepository.resourcesCalculator()); + this.capacity = new HostCapacity(allNodes, nodeRepository.resourcesCalculator()); this.requestedNodes = nodeSpec; this.clusterSpec = clusterSpec; this.application = application; - this.spareHosts = findSpareHosts(allNodes, capacity, spares); + this.spareHosts = capacity.findSpareHosts(allNodes.asList(), spares); this.allocateFully = allocateFully; this.nodeRepository = nodeRepository; @@ -83,22 +81,6 @@ public class NodePrioritizer { this.isDocker = resources(requestedNodes) != null; } - /** - * Spare hosts are the two hosts in the system with the most free capacity. - * - * We do not count retired or inactive nodes as used capacity (as they could have been - * moved to create space for the spare node in the first place). - */ - private static Set findSpareHosts(LockedNodeList nodes, DockerHostCapacity capacity, int spares) { - return nodes.asList().stream() - .filter(node -> node.type() == NodeType.host) - .filter(dockerHost -> dockerHost.state() == Node.State.active) - .filter(dockerHost -> capacity.freeIPs(dockerHost) > 0) - .sorted(capacity::compareWithoutInactive) - .limit(spares) - .collect(Collectors.toSet()); - } - /** Returns the list of nodes sorted by PrioritizableNode::compare */ List prioritize() { return nodes.values().stream().sorted().collect(Collectors.toList()); @@ -207,7 +189,7 @@ public class NodePrioritizer { if (!isNewNode) builder.resizable(! allocateFully - && requestedNodes.canResize(node.flavor().resources(), parentCapacity, isTopologyChange, currentClusterSize)); + && requestedNodes.canResize(node.resources(), parentCapacity, isTopologyChange, currentClusterSize)); if (spareHosts.contains(parent)) builder.violatesSpares(true); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java index a8abdc3f38a..9971aae1714 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java @@ -139,7 +139,7 @@ public interface NodeSpec { @Override public boolean needsResize(Node node) { - return ! node.flavor().resources().compatibleWith(requestedNodeResources); + return ! node.resources().compatibleWith(requestedNodeResources); } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java index 3fc60c1192d..0c1b396c40c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/PrioritizableNode.java @@ -126,7 +126,7 @@ class PrioritizableNode implements Comparable { double skewWithoutThis() { return skewWith(zeroResources); } /** Returns the allocation skew of the parent of this after adding this node to it */ - double skewWithThis() { return skewWith(node.flavor().resources()); } + double skewWithThis() { return skewWith(node.resources()); } private double skewWith(NodeResources resources) { if (parent.isEmpty()) return 0; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java index aa81aae84fe..a4161a318ab 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/ApplicationSerializer.java @@ -43,7 +43,7 @@ public class ApplicationSerializer { if (nodes.isEmpty()) return; int groups = (int)nodes.stream().map(node -> node.allocation().get().membership().cluster().group()).distinct().count(); - ClusterResources currentResources = new ClusterResources(nodes.size(), groups, nodes.get(0).flavor().resources()); + ClusterResources currentResources = new ClusterResources(nodes.size(), groups, nodes.get(0).resources()); toSlime(cluster.minResources(), clusterObject.setObject("min")); toSlime(cluster.maxResources(), clusterObject.setObject("max")); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/HostCapacityResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/HostCapacityResponse.java index 7e81a9cc002..e28b03d7517 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/HostCapacityResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/HostCapacityResponse.java @@ -129,7 +129,7 @@ public class HostCapacityResponse extends HttpResponse { ); failurePath.failureReason.tenant.ifPresent(tenant -> { object.setString("failedTenant", tenant.hostname()); - object.setString("failedTenantResources", tenant.flavor().resources().toString()); + object.setString("failedTenantResources", tenant.resources().toString()); tenant.allocation().ifPresent(allocation -> object.setString("failedTenantAllocation", allocation.toString()) ); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java index cc5c6851a92..a0a44e4f342 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java @@ -383,7 +383,7 @@ public class AutoscalingTest { @Override public NodeResources realResourcesOf(Node node, NodeRepository nodeRepository) { - return node.flavor().resources(); + return node.resources(); } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java index b0e394c93d3..1137ae5ce2c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java @@ -208,9 +208,9 @@ class AutoscalingTester { @Override public NodeResources realResourcesOf(Node node, NodeRepository nodeRepository) { if (zone.getCloud().dynamicProvisioning()) - return node.flavor().resources().withMemoryGb(node.flavor().resources().memoryGb() - 3); + return node.resources().withMemoryGb(node.resources().memoryGb() - 3); else - return node.flavor().resources(); + return node.resources(); } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java index a6b2f6b15ea..464bdd6405a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java @@ -129,7 +129,7 @@ public class CapacityCheckerTester { childModel.parentHostname = Optional.of(hostname); Node childNode = createNodeFromModel(childModel); - childResources.add(childNode.flavor().resources()); + childResources.add(childNode.resources()); hosts.add(childNode); } @@ -275,7 +275,7 @@ public class CapacityCheckerTester { nodeModel.parentHostname, f, Optional.empty(), nodeModel.type); if (membership != null) { - return node.allocate(owner, membership, node.flavor().resources(), Instant.now()); + return node.allocate(owner, membership, node.resources(), Instant.now()); } else { return node; } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java index 9fc2f666d27..727232e5c7c 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporterTest.java @@ -223,7 +223,7 @@ public class MetricsReporterTest { if (tenant.isPresent()) { Allocation allocation = new Allocation(app(tenant.get()), ClusterMembership.from("container/id1/0/3", new Version(), Optional.empty()), - owner.flavor().resources(), + owner.resources(), Generation.initial(), false); return Optional.of(allocation); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java index e2fd8a8721c..51f70e8b640 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java @@ -301,9 +301,9 @@ public class NodeFailerTest { // Two ready nodes and a ready docker node die, but only 2 of those are failed out tester.clock.advance(Duration.ofMinutes(180)); - Node dockerNode = ready.stream().filter(node -> node.flavor().resources().equals(newNodeResources)).findFirst().get(); + Node dockerNode = ready.stream().filter(node -> node.resources().equals(newNodeResources)).findFirst().get(); List otherNodes = ready.stream() - .filter(node -> ! node.flavor().resources().equals(newNodeResources)) + .filter(node -> ! node.resources().equals(newNodeResources)) .collect(Collectors.toList()); tester.allNodesMakeAConfigRequestExcept(otherNodes.get(0), otherNodes.get(2), dockerNode); tester.failer.run(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationVisualizer.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationVisualizer.java index ea4386f2fd5..644b2338a5a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationVisualizer.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/AllocationVisualizer.java @@ -102,13 +102,13 @@ public class AllocationVisualizer extends JPanel { if (isHost) { g.setColor(Color.GRAY); - for (int i = 0; i < node.flavor().resources().memoryGb(); i++) { + for (int i = 0; i < node.resources().memoryGb(); i++) { g.fillRect(x, y - nodeHeight, nodeWidth, nodeHeight); y = y - (nodeHeight + 2); } } else { g.setColor(Color.YELLOW); - int multi = (int) node.flavor().resources().memoryGb(); + int multi = (int) node.resources().memoryGb(); int height = multi * nodeHeight + ((multi - 1) * 2); g.fillRect(x, y - height, nodeWidth, height); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java deleted file mode 100644 index aef25daa659..00000000000 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerHostCapacityTest.java +++ /dev/null @@ -1,138 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.provisioning; - -import com.yahoo.config.provision.Flavor; -import com.yahoo.config.provision.NodeFlavors; -import com.yahoo.config.provision.NodeResources; -import com.yahoo.config.provision.NodeType; -import com.yahoo.vespa.hosted.provision.LockedNodeList; -import com.yahoo.vespa.hosted.provision.Node; -import com.yahoo.vespa.hosted.provision.node.IP; -import org.junit.Before; -import org.junit.Test; - -import java.util.ArrayList; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Optional; -import java.util.Set; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.mock; - -/** - * @author smorgrav - */ -public class DockerHostCapacityTest { - - private final HostResourcesCalculator hostResourcesCalculator = mock(HostResourcesCalculator.class); - private DockerHostCapacity capacity; - private List nodes; - private Node host1, host2, host3; - private final NodeResources resources1 = new NodeResources(1, 30, 20, 1.5); - private final NodeResources resources2 = new NodeResources(2, 40, 40, 0.5); - - @Before - public void setup() { - doAnswer(invocation -> ((Flavor)invocation.getArguments()[0]).resources()).when(hostResourcesCalculator).advertisedResourcesOf(any()); - - // Create flavors - NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("host", "docker", "docker2"); - - // Create three docker hosts - host1 = Node.create("host1", new IP.Config(Set.of("::1"), generateIPs(2, 4)), "host1", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), Optional.empty(), NodeType.host); - host2 = Node.create("host2", new IP.Config(Set.of("::11"), generateIPs(12, 3)), "host2", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), Optional.empty(), NodeType.host); - host3 = Node.create("host3", new IP.Config(Set.of("::21"), generateIPs(22, 1)), "host3", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), Optional.empty(), NodeType.host); - - // Add two containers to host1 - var nodeA = Node.createDockerNode(Set.of("::2"), "nodeA", "host1", resources1, NodeType.tenant); - var nodeB = Node.createDockerNode(Set.of("::3"), "nodeB", "host1", resources1, NodeType.tenant); - - // Add two containers to host 2 (same as host 1) - var nodeC = Node.createDockerNode(Set.of("::12"), "nodeC", "host2", resources1, NodeType.tenant); - var nodeD = Node.createDockerNode(Set.of("::13"), "nodeD", "host2", resources1, NodeType.tenant); - - // Add a larger container to host3 - var nodeE = Node.createDockerNode(Set.of("::22"), "nodeE", "host3", resources2, NodeType.tenant); - - // init docker host capacity - nodes = new ArrayList<>(List.of(host1, host2, host3, nodeA, nodeB, nodeC, nodeD, nodeE)); - capacity = new DockerHostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); - } - - @Test - public void hasCapacity() { - assertTrue(capacity.hasCapacity(host1, resources1)); - assertTrue(capacity.hasCapacity(host1, resources2)); - assertTrue(capacity.hasCapacity(host2, resources1)); - assertTrue(capacity.hasCapacity(host2, resources2)); - assertFalse(capacity.hasCapacity(host3, resources1)); // No ip available - assertFalse(capacity.hasCapacity(host3, resources2)); // No ip available - - // Add a new node to host1 to deplete the memory resource - Node nodeF = Node.createDockerNode(Set.of("::6"), "nodeF", "host1", resources1, NodeType.tenant); - nodes.add(nodeF); - capacity = new DockerHostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); - assertFalse(capacity.hasCapacity(host1, resources1)); - assertFalse(capacity.hasCapacity(host1, resources2)); - } - - @Test - public void freeIPs() { - assertEquals(2, capacity.freeIPs(host1)); - assertEquals(1, capacity.freeIPs(host2)); - assertEquals(0, capacity.freeIPs(host3)); - } - - @Test - public void freeCapacityOf() { - assertEquals(new NodeResources(5, 40, 80, 2, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), - capacity.freeCapacityOf(host1, false)); - assertEquals(new NodeResources(5, 60, 80, 4.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), - capacity.freeCapacityOf(host3, false)); - - doAnswer(invocation -> { - NodeResources totalHostResources = ((Flavor) invocation.getArguments()[0]).resources(); - return totalHostResources.subtract(new NodeResources(1, 2, 3, 0.5, NodeResources.DiskSpeed.any)); - }).when(hostResourcesCalculator).advertisedResourcesOf(any()); - - assertEquals(new NodeResources(4, 38, 77, 1.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), - capacity.freeCapacityOf(host1, false)); - assertEquals(new NodeResources(4, 58, 77, 4, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), - capacity.freeCapacityOf(host3, false)); - } - - @Test - public void devhostCapacityTest() { - // Dev host can assign both configserver and tenant containers. - - var nodeFlavors = FlavorConfigBuilder.createDummies("devhost", "container"); - var devHost = Node.create("devhost", new IP.Config(Set.of("::1"), generateIPs(2, 10)), "devhost", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("devhost"), Optional.empty(), NodeType.devhost); - - var cfg = Node.createDockerNode(Set.of("::2"), "cfg", "devhost", resources1, NodeType.config); - - var nodes = new ArrayList<>(List.of(cfg)); - var capacity = new DockerHostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); - assertTrue(capacity.hasCapacity(devHost, resources1)); - - var container1 = Node.createDockerNode(Set.of("::3"), "container1", "devhost", resources1, NodeType.tenant); - nodes = new ArrayList<>(List.of(cfg, container1)); - capacity = new DockerHostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); - assertFalse(capacity.hasCapacity(devHost, resources1)); - - } - - private Set generateIPs(int start, int count) { - // Allow 4 containers - Set ipAddressPool = new LinkedHashSet<>(); - for (int i = start; i < (start + count); i++) { - ipAddressPool.add("::" + i); - } - return ipAddressPool; - } - -} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java index 3ffb0dc34f0..0c5a682c3c5 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java @@ -60,7 +60,7 @@ public class DockerProvisioningTest { NodeList nodes = tester.getNodes(application1, Node.State.active); assertEquals(nodeCount, nodes.size()); - assertEquals(dockerResources, nodes.asList().get(0).flavor().resources()); + assertEquals(dockerResources, nodes.asList().get(0).resources()); // Upgrade Vespa version on nodes Version upgradedWantedVespaVersion = Version.fromString("6.40"); @@ -70,7 +70,7 @@ public class DockerProvisioningTest { tester.activate(application1, new HashSet<>(upgradedHosts)); NodeList upgradedNodes = tester.getNodes(application1, Node.State.active); assertEquals(nodeCount, upgradedNodes.size()); - assertEquals(dockerResources, upgradedNodes.asList().get(0).flavor().resources()); + assertEquals(dockerResources, upgradedNodes.asList().get(0).resources()); assertEquals(hosts, upgradedHosts); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java index 7350df40718..98ec01e8e95 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicDockerAllocationTest.java @@ -425,7 +425,7 @@ public class DynamicDockerAllocationTest { ); ClusterMembership clusterMembership1 = ClusterMembership.from( clusterSpec.with(Optional.of(ClusterSpec.Group.from(0))), index); // Need to add group here so that group is serialized in node allocation - Node node1aAllocation = node1a.allocate(id, clusterMembership1, node1a.flavor().resources(), Instant.now()); + Node node1aAllocation = node1a.allocate(id, clusterMembership1, node1a.resources(), Instant.now()); tester.nodeRepository().addNodes(Collections.singletonList(node1aAllocation), Agent.system); NestedTransaction transaction = new NestedTransaction().add(new CuratorTransaction(tester.getCurator())); 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 new file mode 100644 index 00000000000..da78aff493e --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacityTest.java @@ -0,0 +1,138 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.provisioning; + +import com.yahoo.config.provision.Flavor; +import com.yahoo.config.provision.NodeFlavors; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; +import com.yahoo.vespa.hosted.provision.LockedNodeList; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.node.IP; +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; + +/** + * @author smorgrav + */ +public class HostCapacityTest { + + private final HostResourcesCalculator hostResourcesCalculator = mock(HostResourcesCalculator.class); + private HostCapacity capacity; + private List nodes; + private Node host1, host2, host3; + private final NodeResources resources1 = new NodeResources(1, 30, 20, 1.5); + private final NodeResources resources2 = new NodeResources(2, 40, 40, 0.5); + + @Before + public void setup() { + doAnswer(invocation -> ((Flavor)invocation.getArguments()[0]).resources()).when(hostResourcesCalculator).advertisedResourcesOf(any()); + + // Create flavors + NodeFlavors nodeFlavors = FlavorConfigBuilder.createDummies("host", "docker", "docker2"); + + // Create three docker hosts + host1 = Node.create("host1", new IP.Config(Set.of("::1"), generateIPs(2, 4)), "host1", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), Optional.empty(), NodeType.host); + host2 = Node.create("host2", new IP.Config(Set.of("::11"), generateIPs(12, 3)), "host2", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), Optional.empty(), NodeType.host); + host3 = Node.create("host3", new IP.Config(Set.of("::21"), generateIPs(22, 1)), "host3", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("host"), Optional.empty(), NodeType.host); + + // Add two containers to host1 + var nodeA = Node.createDockerNode(Set.of("::2"), "nodeA", "host1", resources1, NodeType.tenant); + var nodeB = Node.createDockerNode(Set.of("::3"), "nodeB", "host1", resources1, NodeType.tenant); + + // Add two containers to host 2 (same as host 1) + var nodeC = Node.createDockerNode(Set.of("::12"), "nodeC", "host2", resources1, NodeType.tenant); + var nodeD = Node.createDockerNode(Set.of("::13"), "nodeD", "host2", resources1, NodeType.tenant); + + // Add a larger container to host3 + var nodeE = Node.createDockerNode(Set.of("::22"), "nodeE", "host3", resources2, NodeType.tenant); + + // init docker host capacity + nodes = new ArrayList<>(List.of(host1, host2, host3, nodeA, nodeB, nodeC, nodeD, nodeE)); + capacity = new HostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); + } + + @Test + public void hasCapacity() { + assertTrue(capacity.hasCapacity(host1, resources1)); + assertTrue(capacity.hasCapacity(host1, resources2)); + assertTrue(capacity.hasCapacity(host2, resources1)); + assertTrue(capacity.hasCapacity(host2, resources2)); + assertFalse(capacity.hasCapacity(host3, resources1)); // No ip available + assertFalse(capacity.hasCapacity(host3, resources2)); // No ip available + + // Add a new node to host1 to deplete the memory resource + Node nodeF = Node.createDockerNode(Set.of("::6"), "nodeF", "host1", resources1, NodeType.tenant); + nodes.add(nodeF); + capacity = new HostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); + assertFalse(capacity.hasCapacity(host1, resources1)); + assertFalse(capacity.hasCapacity(host1, resources2)); + } + + @Test + public void freeIPs() { + assertEquals(2, capacity.freeIPs(host1)); + assertEquals(1, capacity.freeIPs(host2)); + assertEquals(0, capacity.freeIPs(host3)); + } + + @Test + public void freeCapacityOf() { + assertEquals(new NodeResources(5, 40, 80, 2, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + capacity.freeCapacityOf(host1, false)); + assertEquals(new NodeResources(5, 60, 80, 4.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + capacity.freeCapacityOf(host3, false)); + + doAnswer(invocation -> { + NodeResources totalHostResources = ((Flavor) invocation.getArguments()[0]).resources(); + return totalHostResources.subtract(new NodeResources(1, 2, 3, 0.5, NodeResources.DiskSpeed.any)); + }).when(hostResourcesCalculator).advertisedResourcesOf(any()); + + assertEquals(new NodeResources(4, 38, 77, 1.5, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + capacity.freeCapacityOf(host1, false)); + assertEquals(new NodeResources(4, 58, 77, 4, NodeResources.DiskSpeed.fast, NodeResources.StorageType.remote), + capacity.freeCapacityOf(host3, false)); + } + + @Test + public void devhostCapacityTest() { + // Dev host can assign both configserver and tenant containers. + + var nodeFlavors = FlavorConfigBuilder.createDummies("devhost", "container"); + var devHost = Node.create("devhost", new IP.Config(Set.of("::1"), generateIPs(2, 10)), "devhost", Optional.empty(), Optional.empty(), nodeFlavors.getFlavorOrThrow("devhost"), Optional.empty(), NodeType.devhost); + + var cfg = Node.createDockerNode(Set.of("::2"), "cfg", "devhost", resources1, NodeType.config); + + var nodes = new ArrayList<>(List.of(cfg)); + var capacity = new HostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); + assertTrue(capacity.hasCapacity(devHost, resources1)); + + var container1 = Node.createDockerNode(Set.of("::3"), "container1", "devhost", resources1, NodeType.tenant); + nodes = new ArrayList<>(List.of(cfg, container1)); + capacity = new HostCapacity(new LockedNodeList(nodes, () -> {}), hostResourcesCalculator); + assertFalse(capacity.hasCapacity(devHost, resources1)); + + } + + private Set generateIPs(int start, int count) { + // Allow 4 containers + Set ipAddressPool = new LinkedHashSet<>(); + for (int i = start; i < (start + count); i++) { + ipAddressPool.add("::" + i); + } + return ipAddressPool; + } + +} diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java index b2ee298c19d..71c3ec37d65 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InPlaceResizeProvisionTest.java @@ -150,9 +150,9 @@ public class InPlaceResizeProvisionTest { assertEquals(6, appNodes.size()); // 4 nodes with large resources + 2 retired nodes with medium resources appNodes.forEach(node -> { if (node.allocation().get().membership().retired()) - assertEquals(new NodeResources(4, 8, 160, 1, fast, local), node.flavor().resources()); + assertEquals(new NodeResources(4, 8, 160, 1, fast, local), node.resources()); else - assertEquals(new NodeResources(8, 16, 320, 1, fast, local), node.flavor().resources()); + assertEquals(new NodeResources(8, 16, 320, 1, fast, local), node.resources()); initialHostnames.remove(node.hostname()); }); assertTrue("All initial nodes should still be allocated to the application", initialHostnames.isEmpty()); @@ -254,7 +254,7 @@ public class InPlaceResizeProvisionTest { private void assertSizeAndResources(NodeList nodes, int size, NodeResources resources) { assertEquals(size, nodes.size()); - nodes.forEach(n -> assertEquals(resources, n.flavor().resources())); + nodes.forEach(n -> assertEquals(resources, n.resources())); } private NodeList listCluster(ClusterSpec cluster) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java index 48bd091011e..e45ea09d372 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/InfraDeployerImplTest.java @@ -140,7 +140,7 @@ public class InfraDeployerImplTest { Optional nodeWithAllocation = wantedVespaVersion.map(version -> { ClusterSpec clusterSpec = application.getClusterSpecWithVersion(version).with(Optional.of(ClusterSpec.Group.from(0))); ClusterMembership membership = ClusterMembership.from(clusterSpec, 1); - Allocation allocation = new Allocation(application.getApplicationId(), membership, node.flavor().resources(), Generation.initial(), false); + Allocation allocation = new Allocation(application.getApplicationId(), membership, node.resources(), Generation.initial(), false); return node.with(allocation); }); return nodeRepository.database().writeTo(state, nodeWithAllocation.orElse(node), Agent.system, Optional.empty()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java index 2427c0303c6..e73aeb05ce3 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTester.java @@ -262,8 +262,8 @@ public class ProvisioningTester { nodeList.stream().map(n -> n.allocation().get().membership().cluster().group().get()).distinct().count()); for (Node node : nodeList) { var expected = new NodeResources(vcpu, memory, disk, bandwidth, diskSpeed, storageType); - assertTrue(explanation + ": Resources: Expected " + expected + " but was " + node.flavor().resources(), - expected.compatibleWith(node.flavor().resources())); + assertTrue(explanation + ": Resources: Expected " + expected + " but was " + node.resources(), + expected.compatibleWith(node.resources())); } } @@ -658,7 +658,7 @@ public class ProvisioningTester { @Override public NodeResources realResourcesOf(Node node, NodeRepository nodeRepository) { - NodeResources resources = node.flavor().resources(); + NodeResources resources = node.resources(); if (node.type() == NodeType.host) return resources; return resources.withMemoryGb(resources.memoryGb() - memoryTaxGb) .withDiskGb(resources.diskGb() - ( resources.storageType() == local ? localDiskTax : 0)); diff --git a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisitHandler.java b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisitHandler.java index ff84f5117dd..e652dc86ab2 100644 --- a/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisitHandler.java +++ b/vespaclient-java/src/main/java/com/yahoo/vespavisit/VdsVisitHandler.java @@ -19,9 +19,10 @@ import java.text.SimpleDateFormat; /** * An abstract class that can be subclassed by different visitor handlers. * - * @author Thomas Gundersen + * @author Thomas Gundersen */ public abstract class VdsVisitHandler { + boolean showProgress; boolean showStatistics; boolean abortOnClusterDown; @@ -33,8 +34,7 @@ public abstract class VdsVisitHandler { final VisitorControlHandler controlHandler = new ControlHandler(); - public VdsVisitHandler(boolean showProgress, boolean showStatistics, boolean abortOnClusterDown) - { + public VdsVisitHandler(boolean showProgress, boolean showStatistics, boolean abortOnClusterDown) { this.showProgress = showProgress; this.showStatistics = showStatistics; this.abortOnClusterDown = abortOnClusterDown; @@ -72,8 +72,7 @@ public abstract class VdsVisitHandler { return printLock; } - public void onDone() { - } + public void onDone() { } public String getProgressFileName() { return progressFileName; @@ -127,8 +126,7 @@ public abstract class VdsVisitHandler { } private String getDateTime() { - DateFormat dateFormat = - new SimpleDateFormat("yyyy-MM-dd HH:mm:ss zzz"); + DateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss zzz"); dateFormat.setTimeZone(TimeZone.getTimeZone("UTC")); Date date = new Date(); return dateFormat.format(date); @@ -140,14 +138,10 @@ public abstract class VdsVisitHandler { System.err.print('\r'); lastLineIsProgress = false; } - System.err.println("Visitor error (" + getDateTime() + "): " + - message); - if (abortOnClusterDown && - !isDone() && - (message.lastIndexOf("Could not resolve")>=0 || - message.lastIndexOf("don't allow external load")>=0)) { - System.err.println("Aborting visitor as " + - "--abortonclusterdown flag is set."); + System.err.println("Visitor error (" + getDateTime() + "): " + message); + if (abortOnClusterDown && !isDone() && (message.lastIndexOf("Could not resolve")>=0 || + message.lastIndexOf("don't allow external load")>=0)) { + System.err.println("Aborting visitor as --abortonclusterdown flag is set."); abort(); } } @@ -160,11 +154,12 @@ public abstract class VdsVisitHandler { if (code != CompletionCode.SUCCESS) { if (code == CompletionCode.ABORTED) { System.err.println("Visitor aborted: " + message); - } else if (code == CompletionCode.TIMEOUT) { + } + else if (code == CompletionCode.TIMEOUT) { System.err.println("Visitor timed out: " + message); - } else { - System.err.println("Visitor aborted due to unknown issue " - + code + ": " + message); + } + else { + System.err.println("Visitor aborted due to unknown issue " + code + ": " + message); } } else { if (showProgress) { -- cgit v1.2.3 From 57cbdcc67d9b0f35f05325d6d72d4a0cc8187a4b Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 10:27:06 +0200 Subject: Test SpareCapacityMaintainer --- .../main/java/com/yahoo/prelude/query/Item.java | 2 +- .../yahoo/prelude/query/WordAlternativesItem.java | 21 +- .../com/yahoo/vespa/hosted/provision/NodeList.java | 14 +- .../provision/maintenance/CapacityChecker.java | 37 ++-- .../maintenance/MaintenanceDeployment.java | 24 ++- .../maintenance/NodeRepositoryMaintenance.java | 25 ++- .../hosted/provision/maintenance/Rebalancer.java | 2 +- .../maintenance/SpareCapacityMaintainer.java | 20 +- .../provision/persistence/NodeSerializer.java | 14 +- .../hosted/provision/testutils/MockDeployer.java | 65 ++++++- .../provision/maintenance/CapacityCheckerTest.java | 1 + .../maintenance/CapacityCheckerTester.java | 20 +- .../maintenance/SpareCapacityMaintainerTest.java | 214 +++++++++++++++++++++ 13 files changed, 376 insertions(+), 83 deletions(-) create mode 100644 node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java (limited to 'node-repository') diff --git a/container-search/src/main/java/com/yahoo/prelude/query/Item.java b/container-search/src/main/java/com/yahoo/prelude/query/Item.java index ea65bc7d7d2..475a80f7ae0 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/Item.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/Item.java @@ -32,7 +32,7 @@ public abstract class Item implements Cloneable { /** * The definitions in Item.ItemType must match the ones in - * searchlib/src/searchlib/parsequery/parse.h + * searchlib/src/vespa/searchlib/parsequery/parse.h */ public static enum ItemType { OR(0), diff --git a/container-search/src/main/java/com/yahoo/prelude/query/WordAlternativesItem.java b/container-search/src/main/java/com/yahoo/prelude/query/WordAlternativesItem.java index 97c68ee3da8..2c017410109 100644 --- a/container-search/src/main/java/com/yahoo/prelude/query/WordAlternativesItem.java +++ b/container-search/src/main/java/com/yahoo/prelude/query/WordAlternativesItem.java @@ -12,8 +12,7 @@ import com.google.common.collect.ImmutableList; import com.yahoo.compress.IntegerCompressor; /** - * A set words with differing exactness scores to be used for literal boost - * ranking. + * A set of words with differing exactness scores to be used for literal boost ranking. * * @author Steinar Knutsen */ @@ -145,17 +144,14 @@ public class WordAlternativesItem extends TermItem { } /** - * Return an immutable snapshot of the contained terms. This list will not - * reflect later changes to the item. + * Return an immutable snapshot of the contained terms. This list will not reflect later changes to the item. * - * @return an immutable list of word alternatives and their respective - * scores + * @return an immutable list of word alternatives and their respective scores */ public List getAlternatives() { return alternatives; } - @Override public void encodeThis(ByteBuffer target) { super.encodeThis(target); @@ -172,17 +168,14 @@ public class WordAlternativesItem extends TermItem { * equal or higher exactness score. If the term string is present with a * lower exactness score, the new, higher score will take precedence. * - * @param term - * one of several string interpretations of the input word - * @param exactness - * how close the term string matches what the user input + * @param term one of several string interpretations of the input word + * @param exactness how close the term string matches what the user input */ public void addTerm(String term, double exactness) { // do note, Item is Cloneable, and overwriting the reference is what // saves us from overriding the method - if (alternatives.stream().anyMatch((a) -> a.word.equals(term) && a.exactness >= exactness )) { - return; - } + if (alternatives.stream().anyMatch((a) -> a.word.equals(term) && a.exactness >= exactness )) return; + List newTerms = new ArrayList<>(alternatives.size() + 1); newTerms.addAll(alternatives); newTerms.add(new Alternative(term, exactness)); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java index 1cc2abef734..2a6db1ffe6e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java @@ -32,7 +32,7 @@ public class NodeList extends AbstractFilteringList { /** Returns the subset of nodes which are retired */ public NodeList retired() { - return matching(node -> node.allocation().get().membership().retired()); + return matching(node -> node.allocation().isPresent() && node.allocation().get().membership().retired()); } /** Returns the subset of nodes that are being deprovisioned */ @@ -42,7 +42,7 @@ public class NodeList extends AbstractFilteringList { /** Returns the subset of nodes which are removable */ public NodeList removable() { - return matching(node -> node.allocation().get().isRemovable()); + return matching(node -> node.allocation().isPresent() && node.allocation().get().isRemovable()); } /** Returns the subset of nodes having exactly the given resources */ @@ -146,6 +146,16 @@ public class NodeList extends AbstractFilteringList { return matching(node -> nodeStates.contains(node.state())); } + /** Returns the subset of nodes in the active state */ + public NodeList active() { + return matching(node -> node.state()== Node.State.active); + } + + /** Returns the subset of nodes which wantToRetire set true */ + public NodeList wantToRetire() { + return matching((node -> node.status().wantToRetire())); + } + /** Returns the parent nodes of the given child nodes */ public NodeList parentsOf(Collection children) { return children.stream() diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java index 97810c0b329..1c078dbd0c4 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java @@ -206,20 +206,24 @@ public class CapacityChecker { var containedAllocations = collateAllocations(nodechildren); var resourceMap = new HashMap<>(availableResources); List validAllocationTargets = allHosts.stream() - .filter(h -> !hostsToRemove.contains(h)) - .collect(Collectors.toList()); - if (validAllocationTargets.size() == 0) { + .filter(h -> !hostsToRemove.contains(h)) + .collect(Collectors.toList()); + if (validAllocationTargets.size() == 0) return Optional.of(HostRemovalFailure.none()); - } allocationHistory = new AllocationHistory(); for (var host : hostsToRemove) { Optional unallocatedNode = tryAllocateNodes(nodechildren.get(host), - validAllocationTargets, resourceMap, containedAllocations, true); + validAllocationTargets, + resourceMap, + containedAllocations, + true); if (unallocatedNode.isPresent()) { AllocationFailureReasonList failures = collateAllocationFailures(unallocatedNode.get(), - validAllocationTargets, resourceMap, containedAllocations); + validAllocationTargets, + resourceMap, + containedAllocations); return Optional.of(HostRemovalFailure.create(host, unallocatedNode.get(), failures)); } } @@ -332,6 +336,11 @@ public class CapacityChecker { public List hostsCausingFailure; public HostRemovalFailure failureReason; + @Override + public String toString() { + return "failure path: " + failureReason + " upon removing " + hostsCausingFailure; + } + } /** @@ -346,17 +355,15 @@ public class CapacityChecker { public AllocationFailureReasonList allocationFailures; public static HostRemovalFailure none() { - return new HostRemovalFailure( - Optional.empty(), - Optional.empty(), - new AllocationFailureReasonList(List.of())); + return new HostRemovalFailure(Optional.empty(), + Optional.empty(), + new AllocationFailureReasonList(List.of())); } public static HostRemovalFailure create(Node host, Node tenant, AllocationFailureReasonList failureReasons) { - return new HostRemovalFailure( - Optional.of(host), - Optional.of(tenant), - failureReasons); + return new HostRemovalFailure(Optional.of(host), + Optional.of(tenant), + failureReasons); } private HostRemovalFailure(Optional host, Optional tenant, AllocationFailureReasonList allocationFailures) { @@ -367,7 +374,7 @@ public class CapacityChecker { @Override public String toString() { - if (host.isEmpty() || tenant.isEmpty()) return "No removal candidates exists."; + if (host.isEmpty() || tenant.isEmpty()) return "No removal candidates exists"; return String.format( "Failure to remove host %s" + "\n\tNo new host found for tenant %s:" + diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java index db331e88d64..2b4775cbeb1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java @@ -147,9 +147,12 @@ class MaintenanceDeployment implements Closeable { /** * Try to deploy to make this move. * + * @param verifyTarget true to only make this move if the node ends up at the expected target host, + * false if we should perform it as long as it moves from the source host * @return true if the move was done, false if it couldn't be */ - public boolean execute(Agent agent, Deployer deployer, Metric metric, NodeRepository nodeRepository) { + public boolean execute(boolean verifyTarget, + Agent agent, Deployer deployer, Metric metric, NodeRepository nodeRepository) { if (isEmpty()) return false; ApplicationId application = node.allocation().get().owner(); try (MaintenanceDeployment deployment = new MaintenanceDeployment(application, deployer, metric, nodeRepository)) { @@ -161,16 +164,19 @@ class MaintenanceDeployment implements Closeable { Optional expectedNewNode = Optional.empty(); try { if ( ! deployment.prepare()) return false; - expectedNewNode = - nodeRepository.getNodes(application, Node.State.reserved).stream() - .filter(n -> !n.hostname().equals(node.hostname())) - .filter(n -> n.allocation().get().membership().cluster().id().equals(node.allocation().get().membership().cluster().id())) - .findAny(); - if (expectedNewNode.isEmpty()) return false; - if ( ! expectedNewNode.get().hasParent(toHost.hostname())) return false; + if (verifyTarget) { + expectedNewNode = + nodeRepository.getNodes(application, Node.State.reserved).stream() + .filter(n -> !n.hostname().equals(node.hostname())) + .filter(n -> n.allocation().get().membership().cluster().id().equals(node.allocation().get().membership().cluster().id())) + .findAny(); + if (expectedNewNode.isEmpty()) return false; + if (!expectedNewNode.get().hasParent(toHost.hostname())) return false; + } if ( ! deployment.activate()) return false; - log.info(agent + " redeployed " + application + " to " + this); + log.info(agent + " redeployed " + application + " to " + + ( verifyTarget ? this : "move " + (node.hostname() + " from " + fromHost))); return true; } finally { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java index c3f8afae4a4..7875afeb28c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRepositoryMaintenance.java @@ -168,25 +168,24 @@ public class NodeRepositoryMaintenance extends AbstractComponent { private final NodeFailer.ThrottlePolicy throttlePolicy; DefaultTimes(Zone zone) { - failGrace = Duration.ofMinutes(30); - periodicRedeployInterval = Duration.ofMinutes(30); - // Don't redeploy in test environments - redeployMaintainerInterval = Duration.ofMinutes(1); - operatorChangeRedeployInterval = Duration.ofMinutes(1); + autoscalingInterval = Duration.ofMinutes(5); + dynamicProvisionerInterval = Duration.ofMinutes(5); failedExpirerInterval = Duration.ofMinutes(10); - provisionedExpiry = Duration.ofHours(4); - spareCapacityMaintenanceInterval = Duration.ofMinutes(10); - metricsInterval = Duration.ofMinutes(1); + failGrace = Duration.ofMinutes(30); infrastructureProvisionInterval = Duration.ofMinutes(1); - throttlePolicy = NodeFailer.ThrottlePolicy.hosted; loadBalancerExpirerInterval = Duration.ofMinutes(5); - reservationExpiry = Duration.ofMinutes(15); // Need to be long enough for deployment to be finished for all config model versions - dynamicProvisionerInterval = Duration.ofMinutes(5); + metricsInterval = Duration.ofMinutes(1); + nodeMetricsCollectionInterval = Duration.ofMinutes(1); + operatorChangeRedeployInterval = Duration.ofMinutes(1); osUpgradeActivatorInterval = zone.system().isCd() ? Duration.ofSeconds(30) : Duration.ofMinutes(5); + periodicRedeployInterval = Duration.ofMinutes(30); + provisionedExpiry = Duration.ofHours(4); rebalancerInterval = Duration.ofMinutes(40); - nodeMetricsCollectionInterval = Duration.ofMinutes(1); - autoscalingInterval = Duration.ofMinutes(5); + redeployMaintainerInterval = Duration.ofMinutes(1); + reservationExpiry = Duration.ofMinutes(15); // Need to be long enough for deployment to be finished for all config model versions scalingSuggestionsInterval = Duration.ofMinutes(31); + spareCapacityMaintenanceInterval = Duration.ofMinutes(10); + throttlePolicy = NodeFailer.ThrottlePolicy.hosted; if (zone.environment().equals(Environment.prod) && ! zone.system().isCd()) { inactiveExpiry = Duration.ofHours(4); // enough time for the application owner to discover and redeploy 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 e10ac68fde0..3df20fa9d08 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 @@ -46,7 +46,7 @@ public class Rebalancer extends NodeRepositoryMaintainer { NodeList allNodes = nodeRepository().list(); updateSkewMetric(allNodes); if ( ! zoneIsStable(allNodes)) return; - findBestMove(allNodes).execute(Agent.Rebalancer, deployer, metric, nodeRepository()); + findBestMove(allNodes).execute(true, Agent.Rebalancer, deployer, metric, nodeRepository()); } /** We do this here rather than in MetricsReporter because it is expensive and frequent updates are unnecessary */ 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 4179d6d3f83..a396bbda5c9 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 @@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.maintenance; 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; import com.yahoo.vespa.hosted.provision.NodeList; @@ -68,11 +69,9 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { int spareHostCapacity = failurePath.get().hostsCausingFailure.size() - 1; if (spareHostCapacity == 0) { Move move = findMitigatingMove(failurePath.get()); - boolean success = move.execute(Agent.SpareCapacityMaintainer, deployer, metric, nodeRepository()); - if (success) { - // This may strictly be a (noble) lie: We succeeded in taking one step to mitigate, - // but not necessarily *all* steps needed to justify adding 1 to spare capacity. - // However, we alert on this being 0 and we don't want to do that as long as we're not stuck. + if (moving(move)) { + // We succeeded or are in the process of taking a step to mitigate. + // Report with the assumption this will eventually succeed to avoid alerting before we're stuck spareHostCapacity++; } } @@ -80,6 +79,12 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { } } + private boolean moving(Move move) { + if (move.isEmpty()) return false; + if (move.node().allocation().get().membership().retired()) return true; // Move already in progress + return move.execute(false, Agent.SpareCapacityMaintainer, deployer, metric, nodeRepository()); + } + private Move findMitigatingMove(CapacityChecker.HostFailurePath failurePath) { Optional nodeWhichCantMove = failurePath.failureReason.tenant; if (nodeWhichCantMove.isEmpty()) return Move.empty(); @@ -237,9 +242,10 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { @Override public List next() { - next = null; if ( ! hasNext()) throw new IllegalStateException("No more elements"); - return next; + var current = next; + next = null; + return current; } private boolean hasOneAtPosition(int position, int number) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java index 15be7796187..37842115949 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializer.java @@ -386,15 +386,16 @@ public class NodeSerializer { case "operator" : return Agent.operator; case "application" : return Agent.application; case "system" : return Agent.system; - case "NodeFailer" : return Agent.NodeFailer; - case "Rebalancer" : return Agent.Rebalancer; case "DirtyExpirer" : return Agent.DirtyExpirer; + case "DynamicProvisioningMaintainer" : return Agent.DynamicProvisioningMaintainer; case "FailedExpirer" : return Agent.FailedExpirer; case "InactiveExpirer" : return Agent.InactiveExpirer; + case "NodeFailer" : return Agent.NodeFailer; case "ProvisionedExpirer" : return Agent.ProvisionedExpirer; + case "Rebalancer" : return Agent.Rebalancer; case "ReservationExpirer" : return Agent.ReservationExpirer; - case "DynamicProvisioningMaintainer" : return Agent.DynamicProvisioningMaintainer; case "RetiringUpgrader" : return Agent.RetiringUpgrader; + case "SpareCapacityMaintainer": return Agent.SpareCapacityMaintainer; } throw new IllegalArgumentException("Unknown node event agent '" + eventAgentField.asString() + "'"); } @@ -403,15 +404,16 @@ public class NodeSerializer { case operator : return "operator"; case application : return "application"; case system : return "system"; - case NodeFailer : return "NodeFailer"; - case Rebalancer : return "Rebalancer"; case DirtyExpirer : return "DirtyExpirer"; + case DynamicProvisioningMaintainer : return "DynamicProvisioningMaintainer"; case FailedExpirer : return "FailedExpirer"; case InactiveExpirer : return "InactiveExpirer"; + case NodeFailer : return "NodeFailer"; case ProvisionedExpirer : return "ProvisionedExpirer"; + case Rebalancer : return "Rebalancer"; case ReservationExpirer : return "ReservationExpirer"; - case DynamicProvisioningMaintainer : return "DynamicProvisioningMaintainer"; case RetiringUpgrader: return "RetiringUpgrader"; + case SpareCapacityMaintainer: return "SpareCapacityMaintainer"; } throw new IllegalArgumentException("Serialized form of '" + agent + "' not defined"); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java index 5ec5c2c08e8..95f98a351a1 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java @@ -10,6 +10,9 @@ import com.yahoo.config.provision.Deployment; import com.yahoo.config.provision.HostFilter; import com.yahoo.config.provision.HostSpec; import com.yahoo.transaction.NestedTransaction; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.node.Agent; import com.yahoo.vespa.hosted.provision.provisioning.NodeRepositoryProvisioner; import java.time.Clock; @@ -27,18 +30,22 @@ import java.util.stream.Collectors; */ public class MockDeployer implements Deployer { + // For actual deploy mode private final NodeRepositoryProvisioner provisioner; private final Map applications; - private final Map lastDeployTimes = new HashMap<>(); + // For mock deploy anything, changing wantToRetire to retired only + private final NodeRepository nodeRepository; /** The number of redeployments done to this */ public int redeployments = 0; + private final Map lastDeployTimes = new HashMap<>(); private final Clock clock; private final ReentrantLock lock = new ReentrantLock(); private boolean failActivate = false; + /** Create a mock deployer which returns empty on every deploy request. */ @Inject @SuppressWarnings("unused") public MockDeployer() { @@ -46,15 +53,30 @@ public class MockDeployer implements Deployer { } /** - * Create a mock deployer which contains a substitute for an application repository, fullfilled to + * Create a mock deployer which returns a deployment on every request, + * and fulfills it by not actually deploying but only changing any wantToRetire nodes + * for the application to retired. + */ + public MockDeployer(NodeRepository nodeRepository) { + this.provisioner = null; + this.applications = Map.of(); + this.nodeRepository = nodeRepository; + + this.clock = nodeRepository.clock(); + } + + /** + * Create a mock deployer which contains a substitute for an application repository, filled to * be able to call provision with the right parameters. */ public MockDeployer(NodeRepositoryProvisioner provisioner, Clock clock, Map applications) { this.provisioner = provisioner; - this.clock = clock; this.applications = new HashMap<>(applications); + this.nodeRepository = null; + + this.clock = clock; } public ReentrantLock lock() { return lock; } @@ -74,9 +96,13 @@ public class MockDeployer implements Deployer { throw new RuntimeException(e); } try { - return Optional.ofNullable(applications.get(id)) - .map(application -> new MockDeployment(provisioner, application)); - } finally { + if (provisioner != null) + return Optional.ofNullable(applications.get(id)) + .map(application -> new MockDeployment(provisioner, application)); + else + return Optional.of(new RetiringOnlyMockDeployment(nodeRepository, id)); + } + finally { lock.unlock(); } } @@ -135,6 +161,33 @@ public class MockDeployer implements Deployer { } + public class RetiringOnlyMockDeployment implements Deployment { + + private final NodeRepository nodeRepository; + private final ApplicationId applicationId; + + private RetiringOnlyMockDeployment(NodeRepository nodeRepository, ApplicationId applicationId) { + this.nodeRepository = nodeRepository; + this.applicationId = applicationId; + } + + @Override + public void prepare() { } + + @Override + public void activate() { + redeployments++; + lastDeployTimes.put(applicationId, clock.instant()); + + for (Node node : nodeRepository.list().owner(applicationId).active().wantToRetire().asList()) + nodeRepository.write(node.retire(nodeRepository.clock().instant()), nodeRepository.lock(node)); + } + + @Override + public void restart(HostFilter filter) {} + + } + /** An application context which substitutes for an application repository */ public static class ApplicationContext { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java index 5813585554d..5e72cfc53ac 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTest.java @@ -29,6 +29,7 @@ public class CapacityCheckerTest { var failurePath = tester.capacityChecker.worstCaseHostLossLeadingToFailure(); assertTrue(failurePath.isPresent()); assertTrue(tester.nodeRepository.getNodes(NodeType.host).containsAll(failurePath.get().hostsCausingFailure)); + assertEquals(5, failurePath.get().hostsCausingFailure.size()); } @Test diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java index 464bdd6405a..62e9a227109 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityCheckerTester.java @@ -45,6 +45,7 @@ import java.util.stream.IntStream; * @author mgimle */ public class CapacityCheckerTester { + public static final Zone zone = new Zone(Environment.prod, RegionName.from("us-east")); // Components with state @@ -138,8 +139,7 @@ public class CapacityCheckerTester { .mapToObj(n -> String.format("%04X::%04X", hostindex, n)) .collect(Collectors.toSet()); - NodeResources nr = containingNodeResources(childResources, - excessCapacity); + NodeResources nr = containingNodeResources(childResources, excessCapacity); Node node = nodeRepository.createNode(hostname, hostname, new IP.Config(Set.of("::"), availableIps), Optional.empty(), new Flavor(nr), Optional.empty(), NodeType.host); @@ -159,7 +159,8 @@ public class CapacityCheckerTester { Set availableIps = IntStream.range(0, ips) .mapToObj(n -> String.format("%04X::%04X", hostid, n)) .collect(Collectors.toSet()); - Node node = nodeRepository.createNode(hostname, hostname, + Node node = nodeRepository.createNode(hostname, + hostname, new IP.Config(Set.of("::"), availableIps), Optional.empty(), new Flavor(capacity), Optional.empty(), NodeType.host); hosts.add(node); @@ -175,8 +176,8 @@ public class CapacityCheckerTester { ); createNodes(childrenPerHost, numDistinctChildren, childResources, - numHosts, hostExcessCapacity, hostExcessIps, - numEmptyHosts, emptyHostExcessCapacity, emptyHostExcessIps); + numHosts, hostExcessCapacity, hostExcessIps, + numEmptyHosts, emptyHostExcessCapacity, emptyHostExcessIps); } void createNodes(int childrenPerHost, int numDistinctChildren, List childResources, int numHosts, NodeResources hostExcessCapacity, int hostExcessIps, @@ -264,10 +265,11 @@ public class CapacityCheckerTester { owner = ApplicationId.from(nodeModel.owner.tenant, nodeModel.owner.application, nodeModel.owner.instance); } - NodeResources.DiskSpeed diskSpeed; - NodeResources nr = new NodeResources(nodeModel.minCpuCores, nodeModel.minMainMemoryAvailableGb, - nodeModel.minDiskAvailableGb, nodeModel.bandwidth * 1000, - nodeModel.fastDisk ? NodeResources.DiskSpeed.fast : NodeResources.DiskSpeed.slow); + NodeResources nr = new NodeResources(nodeModel.minCpuCores, + nodeModel.minMainMemoryAvailableGb, + nodeModel.minDiskAvailableGb, + nodeModel.bandwidth * 1000, + nodeModel.fastDisk ? NodeResources.DiskSpeed.fast : NodeResources.DiskSpeed.slow); Flavor f = new Flavor(nr); Node node = nodeRepository.createNode(nodeModel.id, nodeModel.hostname, diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java new file mode 100644 index 00000000000..7d7b3910cdb --- /dev/null +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java @@ -0,0 +1,214 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.provision.maintenance; + +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ClusterMembership; +import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.Environment; +import com.yahoo.config.provision.Flavor; +import com.yahoo.config.provision.NodeFlavors; +import com.yahoo.config.provision.NodeResources; +import com.yahoo.config.provision.NodeType; +import com.yahoo.config.provision.RegionName; +import com.yahoo.config.provision.Zone; +import com.yahoo.test.ManualClock; +import com.yahoo.transaction.NestedTransaction; +import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.hosted.provision.Node; +import com.yahoo.vespa.hosted.provision.NodeRepository; +import com.yahoo.vespa.hosted.provision.node.Agent; +import com.yahoo.vespa.hosted.provision.node.IP; +import com.yahoo.vespa.hosted.provision.provisioning.EmptyProvisionServiceProvider; +import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; +import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; +import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; +import org.junit.Test; + +import java.time.Duration; +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.junit.Assert.assertEquals; + +/** + * @author bratseth + */ +public class SpareCapacityMaintainerTest { + + @Test + public void testEmpty() { + var tester = new SpareCapacityMaintainerTester(); + tester.maintainer.maintain(); + assertEquals(0, tester.deployer.redeployments); + assertEquals(0, tester.nodeRepository.list().retired().size()); + } + + @Test + public void testOneSpare() { + var tester = new SpareCapacityMaintainerTester(); + tester.addHosts(2, new NodeResources(10, 100, 1000, 1)); + tester.addNodes(0, 1, new NodeResources(10, 100, 1000, 1), 0); + tester.maintainer.maintain(); + assertEquals(0, tester.deployer.redeployments); + assertEquals(0, tester.nodeRepository.list().retired().size()); + assertEquals(1, tester.metric.values.get("spareHostCapacity")); + } + + @Test + public void testTwoSpares() { + var tester = new SpareCapacityMaintainerTester(); + tester.addHosts(3, new NodeResources(10, 100, 1000, 1)); + tester.addNodes(0, 1, new NodeResources(10, 100, 1000, 1), 0); + tester.maintainer.maintain(); + assertEquals(0, tester.deployer.redeployments); + assertEquals(0, tester.nodeRepository.list().retired().size()); + assertEquals(2, tester.metric.values.get("spareHostCapacity")); + } + + @Test + public void testNoSpares() { + var tester = new SpareCapacityMaintainerTester(); + tester.addHosts(2, new NodeResources(10, 100, 1000, 1)); + tester.addNodes(0, 2, new NodeResources(10, 100, 1000, 1), 0); + tester.maintainer.maintain(); + assertEquals(0, tester.deployer.redeployments); + assertEquals(0, tester.nodeRepository.list().retired().size()); + assertEquals(0, tester.metric.values.get("spareHostCapacity")); + } + + @Test + public void testAllWorksAsSpares() { + var tester = new SpareCapacityMaintainerTester(); + tester.addHosts(4, new NodeResources(10, 100, 1000, 1)); + tester.addNodes(0, 2, new NodeResources(5, 50, 500, 0.5), 0); + tester.addNodes(1, 2, new NodeResources(5, 50, 500, 0.5), 2); + tester.maintainer.maintain(); + assertEquals(0, tester.deployer.redeployments); + assertEquals(0, tester.nodeRepository.list().retired().size()); + assertEquals(2, tester.metric.values.get("spareHostCapacity")); + } + + @Test + public void testMoveIsNeededToHaveSpares() { + // Moving application id 1 and 2 to the same nodes frees up spares for application 0 + var tester = new SpareCapacityMaintainerTester(); + tester.addHosts(6, new NodeResources(10, 100, 1000, 1)); + tester.addNodes(0, 2, new NodeResources(10, 100, 1000, 1), 0); + tester.addNodes(1, 2, new NodeResources(5, 50, 500, 0.5), 2); + tester.addNodes(2, 2, new NodeResources(5, 50, 500, 0.5), 4); + tester.maintainer.maintain(); + assertEquals(1, tester.deployer.redeployments); + assertEquals(1, tester.nodeRepository.list().retired().size()); + assertEquals(1, tester.metric.values.get("spareHostCapacity")); + + // Maintaining again is a no-op since the node to move is already retired + tester.maintainer.maintain(); + assertEquals(1, tester.deployer.redeployments); + assertEquals(1, tester.nodeRepository.list().retired().size()); + assertEquals(1, tester.metric.values.get("spareHostCapacity")); + } + + private static class SpareCapacityMaintainerTester { + + NodeRepository nodeRepository; + MockDeployer deployer; + TestMetric metric = new TestMetric(); + SpareCapacityMaintainer maintainer; + private int hostIndex = 0; + private int nodeIndex = 0; + + private SpareCapacityMaintainerTester() { + NodeFlavors flavors = new NodeFlavors(new FlavorConfigBuilder().build()); + nodeRepository = new NodeRepository(flavors, + new EmptyProvisionServiceProvider().getHostResourcesCalculator(), + new MockCurator(), + new ManualClock(), + new Zone(Environment.prod, RegionName.from("us-east-3")), + new MockNameResolver().mockAnyLookup(), + DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), true, false); + deployer = new MockDeployer(nodeRepository); + maintainer = new SpareCapacityMaintainer(deployer, nodeRepository, metric, Duration.ofMinutes(1)); + } + + private void addHosts(int count, NodeResources resources) { + List hosts = new ArrayList<>(); + for (int i = 0; i < count; i++) { + Node host = nodeRepository.createNode("host" + hostIndex, + "host" + hostIndex + ".yahoo.com", + ipConfig(hostIndex + nodeIndex, true), + Optional.empty(), + new Flavor(resources), + Optional.empty(), + NodeType.host); + hosts.add(host); + hostIndex++; + } + hosts = nodeRepository.addNodes(hosts, Agent.system); + hosts = nodeRepository.setReady(hosts, Agent.system, "Test"); + var transaction = new NestedTransaction(); + nodeRepository.activate(hosts, transaction); + transaction.commit(); + } + + private void addNodes(int id, int count, NodeResources resources, int hostOffset) { + List nodes = new ArrayList<>(); + ApplicationId application = ApplicationId.from("tenant" + id, "application" + id, "default"); + for (int i = 0; i < count; i++) { + ClusterMembership membership = ClusterMembership.from(ClusterSpec.specification(ClusterSpec.Type.content, ClusterSpec.Id.from("cluster" + id)) + .group(ClusterSpec.Group.from(0)) + .vespaVersion("7") + .build(), + i); + Node node = nodeRepository.createNode("node" + nodeIndex, + "node" + nodeIndex + ".yahoo.com", + ipConfig(hostIndex + nodeIndex, false), + Optional.of("host" + ( hostOffset + i) + ".yahoo.com"), + new Flavor(resources), + Optional.empty(), + NodeType.tenant); + node = node.allocate(application, membership, node.resources(), Instant.now()); + nodes.add(node); + nodeIndex++; + } + nodes = nodeRepository.addNodes(nodes, Agent.system); + for (int i = 0; i < count; i++) { + Node node = nodes.get(i); + ClusterMembership membership = ClusterMembership.from(ClusterSpec.specification(ClusterSpec.Type.content, ClusterSpec.Id.from("cluster" + id)) + .group(ClusterSpec.Group.from(0)) + .vespaVersion("7") + .build(), + i); + node = node.allocate(application, membership, node.resources(), Instant.now()); + nodes.set(i, node); + } + nodes = nodeRepository.reserve(nodes); + var transaction = new NestedTransaction(); + nodes = nodeRepository.activate(nodes, transaction); + transaction.commit(); + } + + private IP.Config ipConfig(int id, boolean host) { + return new IP.Config(Set.of(String.format("%04X::%04X", id, 0)), + host ? IntStream.range(0, 10) + .mapToObj(n -> String.format("%04X::%04X", id, n)) + .collect(Collectors.toSet()) + : Set.of()); + } + + private void dumpState() { + for (Node host : nodeRepository.list().hosts().asList()) { + System.out.println("Host " + host.hostname() + " " + host.resources()); + for (Node node : nodeRepository.list().childrenOf(host).asList()) + System.out.println(" Node " + node.hostname() + " " + node.resources() + " allocation " +node.allocation()); + } + } + + } + +} -- cgit v1.2.3 From a5fca70c8a0269db0a164db781746e16b0b772c3 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 14:41:00 +0200 Subject: More tests --- .../provision/maintenance/CapacityChecker.java | 11 +++- .../maintenance/SpareCapacityMaintainer.java | 9 ++- .../maintenance/SpareCapacityMaintainerTest.java | 72 +++++++++++++++++++++- 3 files changed, 87 insertions(+), 5 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java index 1c078dbd0c4..4d21a92d10c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.node.Allocation; +import com.yahoo.vespa.hosted.provision.provisioning.NodeResourceComparator; import java.util.*; import java.util.function.Function; @@ -95,7 +96,8 @@ public class CapacityChecker { if (hosts.size() == 0) return Optional.empty(); List parentRemovalPriorityList = heuristic.entrySet().stream() - .sorted(Comparator.comparingInt(Map.Entry::getValue)) + .sorted(this::hostMitigationOrder) +// .sorted(Comparator.comparingInt(Map.Entry::getValue)) .map(Map.Entry::getKey) .collect(Collectors.toList()); @@ -113,6 +115,13 @@ public class CapacityChecker { throw new IllegalStateException("No path to failure found. This should be impossible!"); } + private int hostMitigationOrder(Map.Entry entry1, Map.Entry entry2) { + int result = Integer.compare(entry1.getValue(), entry2.getValue()); + if (result != 0) return result; + // Mitigate the largest hosts first + return NodeResourceComparator.defaultOrder().compare(entry2.getKey().resources(), entry1.getKey().resources()); + } + private Map constructHostnameToNodeMap(List nodes) { return nodes.stream().collect(Collectors.toMap(Node::hostname, n -> n)); } 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 a396bbda5c9..4f17a0f624b 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 @@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.provision.maintenance; 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; import com.yahoo.vespa.hosted.provision.NodeList; @@ -92,6 +91,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { } private Move moveTowardsSpareFor(Node node) { + System.out.println("Trying to find mitigation for " + node); NodeList allNodes = nodeRepository().list(); // Allocation will assign the two most empty nodes as "spares", which will not be allocated on // unless needed for node failing. Our goal here is to make room on these spares for the given node @@ -108,6 +108,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { shortestMitigation = mitigation; } if (shortestMitigation == null || shortestMitigation.isEmpty()) return Move.empty(); + System.out.println("Shortest mitigation to create spare for " + node + ":\n " + shortestMitigation); return shortestMitigation.get(0); } @@ -137,11 +138,12 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { if (movesLeft == 0) return null; List shortest = null; - for (var i = Subsets(hostCapacity.allNodes().childrenOf(host), movesLeft); i.hasNext(); ) { + for (var i = subsets(hostCapacity.allNodes().childrenOf(host), movesLeft); i.hasNext(); ) { List childrenToMove = i.next(); if ( ! addResourcesOf(childrenToMove, freeCapacity).satisfies(node.resources())) continue; List moves = move(childrenToMove, host, hosts, movesMade, movesLeft); if (moves == null) continue; + if (shortest == null || moves.size() < shortest.size()) shortest = moves; } @@ -165,6 +167,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { private List move(Node node, Node host, List hosts, List movesMade, int movesLeft) { List shortest = null; for (Node target : hosts) { + if (target.equals(host)) continue; List childMoves = makeRoomFor(node, target, hosts, movesMade, movesLeft - 1); if (childMoves == null) continue; if (shortest == null || shortest.size() > childMoves.size() + 1) { @@ -181,7 +184,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { return resources; } - private Iterator> Subsets(NodeList nodes, int maxLength) { + private Iterator> subsets(NodeList nodes, int maxLength) { return new SubsetIterator(nodes.asList(), maxLength); } diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java index 7d7b3910cdb..8ee8565410e 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java @@ -95,7 +95,7 @@ public class SpareCapacityMaintainerTest { } @Test - public void testMoveIsNeededToHaveSpares() { + public void testMoveIsNeeded() { // Moving application id 1 and 2 to the same nodes frees up spares for application 0 var tester = new SpareCapacityMaintainerTester(); tester.addHosts(6, new NodeResources(10, 100, 1000, 1)); @@ -114,6 +114,76 @@ public class SpareCapacityMaintainerTest { assertEquals(1, tester.metric.values.get("spareHostCapacity")); } + @Test + public void testMultipleMovesAreNeeded() { + // Moving application id 1 and 2 to the same nodes frees up spares for application 0 + // so that it can be moved from size 12 to size 10 hosts, clearing up spare room for the size 12 application + var tester = new SpareCapacityMaintainerTester(); + tester.addHosts(4, new NodeResources(12, 120, 1200, 1.2)); + tester.addHosts(4, new NodeResources(10, 100, 1000, 1)); + tester.addNodes(0, 2, new NodeResources(10, 100, 1000, 1.0), 0); + tester.addNodes(1, 2, new NodeResources(12, 120, 1200, 1.2), 2); + tester.addNodes(2, 2, new NodeResources(5, 50, 500, 0.5), 4); + tester.addNodes(3, 2, new NodeResources(5, 50, 500, 0.5), 6); + tester.maintainer.maintain(); + assertEquals(1, tester.deployer.redeployments); + assertEquals(1, tester.nodeRepository.list().retired().size()); + assertEquals(1, tester.metric.values.get("spareHostCapacity")); + } + + @Test + public void testMultipleNodesMustMoveFromOneHost() { + // By moving the 4 small nodes from host 2 we free up sufficient space on the third host to act as a spare for + // application 0 + var tester = new SpareCapacityMaintainerTester(); + + tester.addHosts(2, new NodeResources(10, 100, 1000, 1)); + tester.addNodes(0, 2, new NodeResources(10, 100, 1000, 1.0), 0); + + tester.addHosts(1, new NodeResources(16, 160, 1600, 1.6)); + tester.addNodes(1, 1, new NodeResources(1, 10, 100, 0.1), 2); + tester.addNodes(2, 1, new NodeResources(1, 10, 100, 0.1), 2); + tester.addNodes(3, 1, new NodeResources(1, 10, 100, 0.1), 2); + tester.addNodes(4, 1, new NodeResources(1, 10, 100, 0.1), 2); + tester.addNodes(5, 1, new NodeResources(2, 20, 200, 2.0), 2); + tester.addNodes(6, 1, new NodeResources(2, 20, 200, 2.0), 2); + tester.addNodes(7, 1, new NodeResources(2, 20, 200, 2.0), 2); + + tester.addHosts(5, new NodeResources(2, 20, 200, 2.0)); + + tester.maintainer.maintain(); + assertEquals(1, tester.deployer.redeployments); + assertEquals(1, tester.nodeRepository.list().retired().size()); + assertEquals(1, tester.metric.values.get("spareHostCapacity")); + } + + @Test + public void testTooManyMovesAreNeeded() { + // 6 nodes must move to the next host, which is more than the max limit + var tester = new SpareCapacityMaintainerTester(); + + tester.addHosts(2, new NodeResources(10, 100, 1000, 1)); + tester.addHosts(1, new NodeResources(9, 90, 900, 0.9)); + tester.addHosts(1, new NodeResources(8, 80, 800, 0.8)); + tester.addHosts(1, new NodeResources(7, 70, 700, 0.7)); + tester.addHosts(1, new NodeResources(6, 60, 600, 0.6)); + tester.addHosts(1, new NodeResources(5, 50, 500, 0.5)); + tester.addHosts(1, new NodeResources(4, 40, 400, 0.4)); + + tester.addNodes(0, 1, new NodeResources(10, 100, 1000, 1.0), 0); + tester.addNodes(1, 1, new NodeResources( 9, 90, 900, 0.9), 1); + tester.addNodes(2, 1, new NodeResources( 8, 80, 800, 0.8), 2); + tester.addNodes(3, 1, new NodeResources( 7, 70, 700, 0.7), 3); + tester.addNodes(4, 1, new NodeResources( 6, 60, 600, 0.6), 4); + tester.addNodes(5, 1, new NodeResources( 5, 50, 500, 0.5), 5); + tester.addNodes(6, 1, new NodeResources( 4, 40, 400, 0.4), 6); + + tester.maintainer.maintain(); + assertEquals(0, tester.deployer.redeployments); + assertEquals(0, tester.nodeRepository.list().retired().size()); + assertEquals(0, tester.metric.values.get("spareHostCapacity")); + } + private static class SpareCapacityMaintainerTester { NodeRepository nodeRepository; -- cgit v1.2.3 From 1d90c58f2e07cf2fc183717607110a012e5a02e6 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 14:45:14 +0200 Subject: More tests --- .../provision/maintenance/CapacityChecker.java | 1 - .../maintenance/SpareCapacityMaintainerTest.java | 26 +++++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java index 4d21a92d10c..f583728f9b8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/CapacityChecker.java @@ -97,7 +97,6 @@ public class CapacityChecker { List parentRemovalPriorityList = heuristic.entrySet().stream() .sorted(this::hostMitigationOrder) -// .sorted(Comparator.comparingInt(Map.Entry::getValue)) .map(Map.Entry::getKey) .collect(Collectors.toList()); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java index 8ee8565410e..ae8d363934a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java @@ -136,7 +136,26 @@ public class SpareCapacityMaintainerTest { // By moving the 4 small nodes from host 2 we free up sufficient space on the third host to act as a spare for // application 0 var tester = new SpareCapacityMaintainerTester(); + setupMultipleHosts(tester, 5); + tester.maintainer.maintain(); + assertEquals(1, tester.deployer.redeployments); + assertEquals(1, tester.nodeRepository.list().retired().size()); + assertEquals(1, tester.metric.values.get("spareHostCapacity")); + } + + @Test + public void testMultipleNodesMustMoveFromOneHostButInsufficientCapacity() { + var tester = new SpareCapacityMaintainerTester(); + setupMultipleHosts(tester, 4); + + tester.maintainer.maintain(); + assertEquals(0, tester.deployer.redeployments); + assertEquals(0, tester.nodeRepository.list().retired().size()); + assertEquals(0, tester.metric.values.get("spareHostCapacity")); + } + + private void setupMultipleHosts(SpareCapacityMaintainerTester tester, int smallNodeCount) { tester.addHosts(2, new NodeResources(10, 100, 1000, 1)); tester.addNodes(0, 2, new NodeResources(10, 100, 1000, 1.0), 0); @@ -149,12 +168,7 @@ public class SpareCapacityMaintainerTest { tester.addNodes(6, 1, new NodeResources(2, 20, 200, 2.0), 2); tester.addNodes(7, 1, new NodeResources(2, 20, 200, 2.0), 2); - tester.addHosts(5, new NodeResources(2, 20, 200, 2.0)); - - tester.maintainer.maintain(); - assertEquals(1, tester.deployer.redeployments); - assertEquals(1, tester.nodeRepository.list().retired().size()); - assertEquals(1, tester.metric.values.get("spareHostCapacity")); + tester.addHosts(smallNodeCount, new NodeResources(2, 20, 200, 2.0)); } @Test -- cgit v1.2.3 From 916e2e9c19025ba8002bb17025b3d975125e4bb7 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 14:46:52 +0200 Subject: Cleanup --- .../vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'node-repository') 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 4f17a0f624b..a67f0b55b14 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 @@ -87,11 +87,8 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { private Move findMitigatingMove(CapacityChecker.HostFailurePath failurePath) { Optional nodeWhichCantMove = failurePath.failureReason.tenant; if (nodeWhichCantMove.isEmpty()) return Move.empty(); - return moveTowardsSpareFor(nodeWhichCantMove.get()); - } - private Move moveTowardsSpareFor(Node node) { - System.out.println("Trying to find mitigation for " + node); + Node node = nodeWhichCantMove.get(); NodeList allNodes = nodeRepository().list(); // Allocation will assign the two most empty nodes as "spares", which will not be allocated on // unless needed for node failing. Our goal here is to make room on these spares for the given node @@ -108,7 +105,6 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { shortestMitigation = mitigation; } if (shortestMitigation == null || shortestMitigation.isEmpty()) return Move.empty(); - System.out.println("Shortest mitigation to create spare for " + node + ":\n " + shortestMitigation); return shortestMitigation.get(0); } -- cgit v1.2.3 From b4c0044429359f984b7927ae04ffbe416ce6bc4a Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 16:25:30 +0200 Subject: Memoize --- .../maintenance/MaintenanceDeployment.java | 18 +++++ .../maintenance/SpareCapacityMaintainer.java | 80 +++++++++++++++++++++- .../maintenance/SpareCapacityMaintainerTest.java | 25 +++++++ 3 files changed, 120 insertions(+), 3 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java index 2b4775cbeb1..1a15dce283a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java @@ -7,6 +7,8 @@ import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.Deployment; import com.yahoo.config.provision.TransientException; import com.yahoo.jdisc.Metric; + +import java.util.Objects; import java.util.logging.Level; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.provision.Node; @@ -205,6 +207,22 @@ class MaintenanceDeployment implements Closeable { public boolean isEmpty() { return node == null; } + @Override + public int hashCode() { + return Objects.hash(node, fromHost, toHost); + } + + public boolean equals(Object o) { + if (o == this) return true; + if (o == null || o.getClass() != this.getClass()) return false; + + Move other = (Move)o; + if ( ! Objects.equals(other.node, this.node)) return false; + if ( ! Objects.equals(other.fromHost, this.fromHost)) return false; + if ( ! Objects.equals(other.toHost, this.toHost)) return false; + return true; + } + @Override public String toString() { return "move " + 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 a67f0b55b14..8578e07f3b3 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 @@ -13,8 +13,11 @@ import com.yahoo.vespa.hosted.provision.provisioning.HostCapacity; import java.time.Duration; import java.util.ArrayList; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.logging.Level; @@ -35,8 +38,7 @@ import java.util.stream.Collectors; */ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { - private static final int maxMoves = 5; - + private static final int maxIterations = 5; // Should take a couple of seconds private final Deployer deployer; private final Metric metric; @@ -99,7 +101,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { CapacitySolver capacitySolver = new CapacitySolver(hostCapacity); List shortestMitigation = null; for (Node spareHost : spareHosts) { - List mitigation = capacitySolver.makeRoomFor(node, spareHost, hosts, List.of(), maxMoves); + List mitigation = capacitySolver.makeRoomFor(node, spareHost, hosts, List.of(), maxIterations); if (mitigation == null) continue; if (shortestMitigation == null || shortestMitigation.size() > mitigation.size()) shortestMitigation = mitigation; @@ -110,12 +112,16 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { private static class CapacitySolver { + private int iterations = 0; private final HostCapacity hostCapacity; CapacitySolver(HostCapacity hostCapacity) { this.hostCapacity = hostCapacity; } + /** The map of subproblem solutions already found. */ + private Map solutions = new HashMap<>(); + /** * Finds the shortest sequence of moves which makes room for the given node on the given host, * assuming the given moves already made over the given hosts' current allocation. @@ -128,6 +134,22 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { * or null if no sequence could be found */ List makeRoomFor(Node node, Node host, List hosts, List movesMade, int movesLeft) { + SolutionKey solutionKey = new SolutionKey(node, host, movesMade); + Solution solution = solutions.get(solutionKey); + if (solution == null || (solution.isNone() && solution.movesLeft() < movesLeft)) { + List moves = findRoomFor(node, host, hosts, movesMade, movesLeft); + solution = new Solution(moves, movesLeft); + solutions.put(solutionKey, solution); + } + if (solution.isNone() || solution.moves().size() > movesLeft) + return null; + return solution.moves(); + } + + private List findRoomFor(Node node, Node host, List hosts, List movesMade, int movesLeft) { + iterations++; + if ((iterations % 1000) == 0) + System.out.println(" Iteration " + iterations); if ( ! host.resources().satisfies(node.resources())) return null; NodeResources freeCapacity = freeCapacityWith(movesMade, host); if (freeCapacity.satisfies(node.resources())) return List.of(); @@ -206,6 +228,58 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { } + private static class SolutionKey { + + private final Node node; + private final Node host; + private final List movesMade; + + private final int hash; + + public SolutionKey(Node node, Node host, List movesMade) { + this.node = node; + this.host = host; + this.movesMade = movesMade; + + hash = Objects.hash(node, host, movesMade); + } + + @Override + public int hashCode() { return hash; } + + @Override + public boolean equals(Object o) { + if (o == this) return true; + if (o == null || o.getClass() != this.getClass()) return false; + + SolutionKey other = (SolutionKey)o; + if ( ! other.node.equals(this.node)) return false; + if ( ! other.host.equals(this.host)) return false; + if ( ! other.movesMade.equals(this.movesMade)) return false; + return true; + } + + } + + private static class Solution { + + /** The moves of this solution, or null if there is none */ + private final List moves; + + /** The number of moves considered when finding (or not finding) this solution */ + private final int movesLeft; + + public Solution(List moves, int movesLeft) { + this.moves = moves; + this.movesLeft = movesLeft; + } + + public List moves() { return moves; } + public int movesLeft() { return movesLeft; } + public boolean isNone() { return moves == null; } + + } + private static class SubsetIterator implements Iterator> { private final List nodes; diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java index ae8d363934a..05d318dbd69 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java @@ -23,6 +23,7 @@ import com.yahoo.vespa.hosted.provision.provisioning.EmptyProvisionServiceProvid import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; import com.yahoo.vespa.hosted.provision.testutils.MockDeployer; import com.yahoo.vespa.hosted.provision.testutils.MockNameResolver; +import org.junit.Ignore; import org.junit.Test; import java.time.Duration; @@ -198,6 +199,30 @@ public class SpareCapacityMaintainerTest { assertEquals(0, tester.metric.values.get("spareHostCapacity")); } + /** Microbenchmark */ + @Test + @Ignore + public void testLargeNodeRepo() { + // Completely fill 200 hosts with 2000 nodes + int hosts = 200; + var tester = new SpareCapacityMaintainerTester(); + tester.addHosts(hosts, new NodeResources(100, 1000, 10000, 10)); + int hostOffset = 0; + for (int i = 0; i < 200; i++) { + int applicationSize = 10; + int resourceSize = 10; + tester.addNodes(i, applicationSize, new NodeResources(resourceSize, resourceSize * 10, resourceSize * 100, 0.1), hostOffset); + hostOffset = (hostOffset + applicationSize) % hosts; + } + long startTime = System.currentTimeMillis(); + tester.maintainer.maintain(); + long totalTime = System.currentTimeMillis() - startTime; + System.out.println("Complete in " + ( totalTime / 1000) + " seconds"); + assertEquals(0, tester.deployer.redeployments); + assertEquals(0, tester.nodeRepository.list().retired().size()); + assertEquals(0, tester.metric.values.get("spareHostCapacity")); + } + private static class SpareCapacityMaintainerTester { NodeRepository nodeRepository; -- cgit v1.2.3 From d589e157fce08e509011ba3ede109c7d71b327d6 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 17:02:30 +0200 Subject: Limit by iterations instead of depth --- .../maintenance/SpareCapacityMaintainer.java | 112 +++++++++++---------- .../maintenance/SpareCapacityMaintainerTest.java | 10 +- 2 files changed, 68 insertions(+), 54 deletions(-) (limited to 'node-repository') 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 8578e07f3b3..7fb6f929cb7 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 @@ -38,7 +38,7 @@ import java.util.stream.Collectors; */ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { - private static final int maxIterations = 5; // Should take a couple of seconds + private final int maxIterations; private final Deployer deployer; private final Metric metric; @@ -46,9 +46,20 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { NodeRepository nodeRepository, Metric metric, Duration interval) { + this(deployer, nodeRepository, metric, interval, + 1_000_000 // Should take a couple of seconds + ); + } + + public SpareCapacityMaintainer(Deployer deployer, + NodeRepository nodeRepository, + Metric metric, + Duration interval, + int maxIterations) { super(nodeRepository, interval); this.deployer = deployer; this.metric = metric; + this.maxIterations = maxIterations; } @Override @@ -98,10 +109,10 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { Set spareHosts = hostCapacity.findSpareHosts(allNodes.hosts().satisfies(node.resources()).asList(), 2); List hosts = allNodes.hosts().except(spareHosts).asList(); - CapacitySolver capacitySolver = new CapacitySolver(hostCapacity); + CapacitySolver capacitySolver = new CapacitySolver(hostCapacity, maxIterations); List shortestMitigation = null; for (Node spareHost : spareHosts) { - List mitigation = capacitySolver.makeRoomFor(node, spareHost, hosts, List.of(), maxIterations); + List mitigation = capacitySolver.makeRoomFor(node, spareHost, hosts, List.of(), List.of()); if (mitigation == null) continue; if (shortestMitigation == null || shortestMitigation.size() > mitigation.size()) shortestMitigation = mitigation; @@ -112,15 +123,18 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { private static class CapacitySolver { - private int iterations = 0; private final HostCapacity hostCapacity; + private final int maxIterations; - CapacitySolver(HostCapacity hostCapacity) { + private int iterations = 0; + + CapacitySolver(HostCapacity hostCapacity, int maxIterations) { this.hostCapacity = hostCapacity; + this.maxIterations = maxIterations; } - /** The map of subproblem solutions already found. */ - private Map solutions = new HashMap<>(); + /** The map of subproblem solutions already found. The value is null when there is no solution. */ + private Map> solutions = new HashMap<>(); /** * Finds the shortest sequence of moves which makes room for the given node on the given host, @@ -129,37 +143,38 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { * @param node the node to make room for * @param host the target host to make room on * @param hosts the hosts onto which we can move nodes + * @param movesConsidered the moves already being considered to add as part of this scenario + * (after any moves made by this) * @param movesMade the moves already made in this scenario * @return the list of movesMade with the moves needed for this appended, in the order they should be performed, * or null if no sequence could be found */ - List makeRoomFor(Node node, Node host, List hosts, List movesMade, int movesLeft) { - SolutionKey solutionKey = new SolutionKey(node, host, movesMade); - Solution solution = solutions.get(solutionKey); - if (solution == null || (solution.isNone() && solution.movesLeft() < movesLeft)) { - List moves = findRoomFor(node, host, hosts, movesMade, movesLeft); - solution = new Solution(moves, movesLeft); + List makeRoomFor(Node node, Node host, List hosts, List movesConsidered, List movesMade) { + SolutionKey solutionKey = new SolutionKey(node, host, movesConsidered, movesMade); + List solution = solutions.get(solutionKey); + if (solution == null) { + solution = findRoomFor(node, host, hosts, movesConsidered, movesMade); solutions.put(solutionKey, solution); } - if (solution.isNone() || solution.moves().size() > movesLeft) - return null; - return solution.moves(); + return solution; } - private List findRoomFor(Node node, Node host, List hosts, List movesMade, int movesLeft) { - iterations++; + private List findRoomFor(Node node, Node host, List hosts, + List movesConsidered, List movesMade) { + if (iterations++ > maxIterations) + return null; + if ((iterations % 1000) == 0) System.out.println(" Iteration " + iterations); if ( ! host.resources().satisfies(node.resources())) return null; NodeResources freeCapacity = freeCapacityWith(movesMade, host); if (freeCapacity.satisfies(node.resources())) return List.of(); - if (movesLeft == 0) return null; List shortest = null; - for (var i = subsets(hostCapacity.allNodes().childrenOf(host), movesLeft); i.hasNext(); ) { + for (var i = subsets(hostCapacity.allNodes().childrenOf(host), 5); i.hasNext(); ) { List childrenToMove = i.next(); if ( ! addResourcesOf(childrenToMove, freeCapacity).satisfies(node.resources())) continue; - List moves = move(childrenToMove, host, hosts, movesMade, movesLeft); + List moves = move(childrenToMove, host, hosts, movesConsidered, movesMade); if (moves == null) continue; if (shortest == null || moves.size() < shortest.size()) @@ -167,43 +182,48 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { } if (shortest == null) return null; List total = append(movesMade, shortest); - if (total.size() > movesLeft) return null; return total; } - private List move(List nodes, Node host, List hosts, List movesMade, int movesLeft) { + private List move(List nodes, Node host, List hosts, List movesConsidered, List movesMade) { List moves = new ArrayList<>(); for (Node childToMove : nodes) { - List childMoves = move(childToMove, host, hosts, append(movesMade, moves), movesLeft - moves.size()); + List childMoves = move(childToMove, host, hosts, movesConsidered, append(movesMade, moves)); if (childMoves == null) return null; moves.addAll(childMoves); - if (moves.size() > movesLeft) return null; } return moves; } - private List move(Node node, Node host, List hosts, List movesMade, int movesLeft) { + private List move(Node node, Node host, List hosts, List movesConsidered, List movesMade) { + if (contains(node, movesConsidered)) return null; + if (contains(node, movesMade)) return null; List shortest = null; for (Node target : hosts) { if (target.equals(host)) continue; - List childMoves = makeRoomFor(node, target, hosts, movesMade, movesLeft - 1); + Move move = new Move(node, host, target); + List childMoves = makeRoomFor(node, target, hosts, append(movesConsidered, move), movesMade); if (childMoves == null) continue; if (shortest == null || shortest.size() > childMoves.size() + 1) { shortest = new ArrayList<>(childMoves); - shortest.add(new Move(node, host, target)); + shortest.add(move); } } return shortest; } + private boolean contains(Node node, List moves) { + return moves.stream().anyMatch(move -> move.node().equals(node)); + } + private NodeResources addResourcesOf(List nodes, NodeResources resources) { for (Node node : nodes) resources = resources.add(node.resources()); return resources; } - private Iterator> subsets(NodeList nodes, int maxLength) { - return new SubsetIterator(nodes.asList(), maxLength); + private Iterator> subsets(NodeList nodes, int maxSize) { + return new SubsetIterator(nodes.asList(), maxSize); } private List append(List a, List b) { @@ -213,6 +233,12 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { return list; } + private List append(List moves, Move move) { + List list = new ArrayList<>(moves); + list.add(move); + return list; + } + private NodeResources freeCapacityWith(List moves, Node host) { NodeResources resources = hostCapacity.freeCapacityOf(host); for (Move move : moves) { @@ -232,16 +258,18 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { private final Node node; private final Node host; + private final List movesConsidered; private final List movesMade; private final int hash; - public SolutionKey(Node node, Node host, List movesMade) { + public SolutionKey(Node node, Node host, List movesConsidered, List movesMade) { this.node = node; this.host = host; + this.movesConsidered = movesConsidered; this.movesMade = movesMade; - hash = Objects.hash(node, host, movesMade); + hash = Objects.hash(node, host, movesConsidered, movesMade); } @Override @@ -255,31 +283,13 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { SolutionKey other = (SolutionKey)o; if ( ! other.node.equals(this.node)) return false; if ( ! other.host.equals(this.host)) return false; + if ( ! other.movesConsidered.equals(this.movesConsidered)) return false; if ( ! other.movesMade.equals(this.movesMade)) return false; return true; } } - private static class Solution { - - /** The moves of this solution, or null if there is none */ - private final List moves; - - /** The number of moves considered when finding (or not finding) this solution */ - private final int movesLeft; - - public Solution(List moves, int movesLeft) { - this.moves = moves; - this.movesLeft = movesLeft; - } - - public List moves() { return moves; } - public int movesLeft() { return movesLeft; } - public boolean isNone() { return moves == null; } - - } - private static class SubsetIterator implements Iterator> { private final List nodes; diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java index 05d318dbd69..8c945ad4aaa 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java @@ -173,9 +173,9 @@ public class SpareCapacityMaintainerTest { } @Test - public void testTooManyMovesAreNeeded() { + public void testTooManyIterationsAreNeeded() { // 6 nodes must move to the next host, which is more than the max limit - var tester = new SpareCapacityMaintainerTester(); + var tester = new SpareCapacityMaintainerTester(5); tester.addHosts(2, new NodeResources(10, 100, 1000, 1)); tester.addHosts(1, new NodeResources(9, 90, 900, 0.9)); @@ -233,6 +233,10 @@ public class SpareCapacityMaintainerTest { private int nodeIndex = 0; private SpareCapacityMaintainerTester() { + this(1000_000); + } + + private SpareCapacityMaintainerTester(int maxIterations) { NodeFlavors flavors = new NodeFlavors(new FlavorConfigBuilder().build()); nodeRepository = new NodeRepository(flavors, new EmptyProvisionServiceProvider().getHostResourcesCalculator(), @@ -242,7 +246,7 @@ public class SpareCapacityMaintainerTest { new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), true, false); deployer = new MockDeployer(nodeRepository); - maintainer = new SpareCapacityMaintainer(deployer, nodeRepository, metric, Duration.ofMinutes(1)); + maintainer = new SpareCapacityMaintainer(deployer, nodeRepository, metric, Duration.ofMinutes(1), maxIterations); } private void addHosts(int count, NodeResources resources) { -- cgit v1.2.3 From 28cf15bac333eadb4f3d133c9102c32bd0d72770 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 17:21:01 +0200 Subject: Less max iterations as each do more exporation --- .../hosted/provision/maintenance/SpareCapacityMaintainer.java | 7 ++----- .../hosted/provision/maintenance/SpareCapacityMaintainerTest.java | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) (limited to 'node-repository') 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 7fb6f929cb7..e436d3b926a 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 @@ -47,7 +47,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { Metric metric, Duration interval) { this(deployer, nodeRepository, metric, interval, - 1_000_000 // Should take a couple of seconds + 10_000 // Should take less than a few minutes ); } @@ -164,8 +164,6 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { if (iterations++ > maxIterations) return null; - if ((iterations % 1000) == 0) - System.out.println(" Iteration " + iterations); if ( ! host.resources().satisfies(node.resources())) return null; NodeResources freeCapacity = freeCapacityWith(movesMade, host); if (freeCapacity.satisfies(node.resources())) return List.of(); @@ -181,8 +179,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { shortest = moves; } if (shortest == null) return null; - List total = append(movesMade, shortest); - return total; + return append(movesMade, shortest); } private List move(List nodes, Node host, List hosts, List movesConsidered, List movesMade) { diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java index 8c945ad4aaa..fb84dc0a32a 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/SpareCapacityMaintainerTest.java @@ -233,7 +233,7 @@ public class SpareCapacityMaintainerTest { private int nodeIndex = 0; private SpareCapacityMaintainerTester() { - this(1000_000); + this(1000); } private SpareCapacityMaintainerTester(int maxIterations) { @@ -246,7 +246,7 @@ public class SpareCapacityMaintainerTest { new MockNameResolver().mockAnyLookup(), DockerImage.fromString("docker-registry.domain.tld:8080/dist/vespa"), true, false); deployer = new MockDeployer(nodeRepository); - maintainer = new SpareCapacityMaintainer(deployer, nodeRepository, metric, Duration.ofMinutes(1), maxIterations); + maintainer = new SpareCapacityMaintainer(deployer, nodeRepository, metric, Duration.ofDays(1), maxIterations); } private void addHosts(int count, NodeResources resources) { -- cgit v1.2.3 From fc959df05f0214e60e60a2e35ee8acaf0eb49b6c Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 22:56:25 +0200 Subject: Update node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java Co-authored-by: Valerij Fredriksen --- .../yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java index 1a15dce283a..4e1be9c486c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MaintenanceDeployment.java @@ -186,7 +186,7 @@ class MaintenanceDeployment implements Closeable { // Immediately clean up if we reserved the node but could not activate or reserved a node on the wrong host expectedNewNode.flatMap(node -> nodeRepository.getNode(node.hostname(), Node.State.reserved)) - .ifPresent(node -> nodeRepository.setDirty(node, Agent.Rebalancer, "Expired by " + agent)); + .ifPresent(node -> nodeRepository.setDirty(node, agent, "Expired by " + agent)); } } } -- cgit v1.2.3 From 75fba523c4d2070443213b839fea2664fde4326a Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 23:02:09 +0200 Subject: Use bitCount --- .../hosted/provision/maintenance/SpareCapacityMaintainer.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) (limited to 'node-repository') 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 e436d3b926a..a7c1517deb5 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 @@ -307,7 +307,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { // find next while (++i < 1< maxLength) continue; next = new ArrayList<>(ones); @@ -332,15 +332,6 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { return (number & (1 << position)) > 0; } - private int onesIn(int number) { - int ones = 0; - for (int position = 0; Math.pow(2, position) <= number; position++) { - if (hasOneAtPosition(position, number)) - ones++; - } - return ones; - } - } } -- cgit v1.2.3 From 32a0cf905b2c42f1ac31cd72cb2529fe396135da Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 17 Jun 2020 23:21:28 +0200 Subject: No easy finding active nodes --- .../src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java | 5 ----- .../vespa/hosted/provision/maintenance/SpareCapacityMaintainer.java | 2 +- .../com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) (limited to 'node-repository') diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java index 2a6db1ffe6e..cbc5a44ae94 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/NodeList.java @@ -146,11 +146,6 @@ public class NodeList extends AbstractFilteringList { return matching(node -> nodeStates.contains(node.state())); } - /** Returns the subset of nodes in the active state */ - public NodeList active() { - return matching(node -> node.state()== Node.State.active); - } - /** Returns the subset of nodes which wantToRetire set true */ public NodeList wantToRetire() { return matching((node -> node.status().wantToRetire())); 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 a7c1517deb5..54899372397 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 @@ -244,7 +244,7 @@ public class SpareCapacityMaintainer extends NodeRepositoryMaintainer { } for (Move move : moves) { if ( ! move.fromHost().equals(host)) continue; - resources = resources.add(move.fromHost().resources()); + resources = resources.add(move.node().resources()); } return resources; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java index 95f98a351a1..ae3d6ebf815 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java @@ -179,7 +179,7 @@ public class MockDeployer implements Deployer { redeployments++; lastDeployTimes.put(applicationId, clock.instant()); - for (Node node : nodeRepository.list().owner(applicationId).active().wantToRetire().asList()) + for (Node node : nodeRepository.list().owner(applicationId).state(Node.State.active).wantToRetire().asList()) nodeRepository.write(node.retire(nodeRepository.clock().instant()), nodeRepository.lock(node)); } -- cgit v1.2.3 From ed30906d441364b95b99f52355d218a085246fa6 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Thu, 18 Jun 2020 09:03:25 +0200 Subject: Update node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java Co-authored-by: Valerij Fredriksen --- .../com/yahoo/vespa/hosted/provision/provisioning/HostCapacity.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'node-repository') 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 4125345d492..fd16e61417f 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 @@ -33,7 +33,7 @@ public class HostCapacity { public NodeList allNodes() { return allNodes; } /** - * Spare hosts are the two hosts in the system with the most free capacity. + * Spare hosts are the hosts in the system with the most free capacity. * * We do not count retired or inactive nodes as used capacity (as they could have been * moved to create space for the spare node in the first place). -- cgit v1.2.3