diff options
20 files changed, 120 insertions, 66 deletions
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java index 33193bafc66..837cbd4605a 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/OrchestratorMock.java @@ -11,8 +11,10 @@ import com.yahoo.vespa.orchestrator.status.HostStatus; import java.time.Instant; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Function; @@ -24,7 +26,7 @@ import java.util.function.Function; */ public class OrchestratorMock implements Orchestrator { - private final Set<HostName> suspendedHosts = new HashSet<>(); + private final Map<HostName, HostInfo> hostInfos = new HashMap<>(); private final Set<ApplicationId> suspendedApplications = new HashSet<>(); @Override @@ -34,14 +36,13 @@ public class OrchestratorMock implements Orchestrator { @Override public HostStatus getNodeStatus(HostName hostName) { - return suspendedHosts.contains(hostName) ? HostStatus.ALLOWED_TO_BE_DOWN : HostStatus.NO_REMARKS; + HostInfo hostInfo = hostInfos.get(hostName); + return hostInfo == null ? HostStatus.NO_REMARKS : hostInfo.status(); } @Override - public Function<HostName, Optional<HostInfo>> getNodeStatuses() { - return hostName -> Optional.of(getNodeStatus(hostName)) - .map(status -> status.isSuspended() ? HostInfo.createSuspended(status, Instant.EPOCH) - : HostInfo.createNoRemarks()); + public Function<HostName, Optional<HostInfo>> getHostResolver() { + return hostName -> Optional.of(hostInfos.getOrDefault(hostName, HostInfo.createNoRemarks())); } @Override @@ -49,12 +50,12 @@ public class OrchestratorMock implements Orchestrator { @Override public void resume(HostName hostName) { - suspendedHosts.remove(hostName); + hostInfos.remove(hostName); } @Override public void suspend(HostName hostName) { - suspendedHosts.add(hostName); + hostInfos.put(hostName, HostInfo.createSuspended(HostStatus.ALLOWED_TO_BE_DOWN, Instant.EPOCH)); } @Override diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java index 970972a5fe6..6092cd54b40 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/MetricsReporter.java @@ -51,7 +51,7 @@ public class MetricsReporter extends Maintainer { Clock clock) { super(nodeRepository, interval); this.metric = metric; - this.orchestrator = orchestrator.getNodeStatuses(); + this.orchestrator = orchestrator.getHostResolver(); this.serviceMonitor = serviceMonitor; this.pendingRedeploymentsSupplier = pendingRedeploymentsSupplier; this.clock = clock; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java index b80446b06da..ca53b215237 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java @@ -265,7 +265,7 @@ public class NodeFailer extends Maintainer { private boolean nodeSuspended(Node node) { try { - return orchestrator.getNodeStatus(new HostName(node.hostname())) == HostStatus.ALLOWED_TO_BE_DOWN; + return orchestrator.getNodeStatus(new HostName(node.hostname())).isSuspended(); } catch (HostNameNotFoundException e) { // Treat it as not suspended return false; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java index 8a2156b0ce3..8ca8dfc26f6 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/v2/NodesResponse.java @@ -16,12 +16,10 @@ import com.yahoo.slime.Slime; import com.yahoo.vespa.applicationmodel.HostName; 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.History; import com.yahoo.vespa.hosted.provision.node.filter.NodeFilter; import com.yahoo.vespa.orchestrator.Orchestrator; import com.yahoo.vespa.orchestrator.status.HostInfo; -import com.yahoo.vespa.orchestrator.status.HostStatus; import java.io.IOException; import java.io.OutputStream; @@ -59,7 +57,7 @@ class NodesResponse extends HttpResponse { this.nodeParentUrl = toNodeParentUrl(request); filter = NodesApiHandler.toNodeFilter(request); this.recursive = request.getBooleanProperty("recursive"); - this.orchestrator = orchestrator.getNodeStatuses(); + this.orchestrator = orchestrator.getHostResolver(); this.nodeRepository = nodeRepository; slime = new Slime(); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java index 3e671f03adf..a5c1789b447 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/OrchestratorMock.java @@ -12,8 +12,10 @@ import com.yahoo.vespa.orchestrator.status.HostStatus; import java.time.Instant; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Function; @@ -23,7 +25,7 @@ import java.util.function.Function; */ public class OrchestratorMock extends AbstractComponent implements Orchestrator { - private final Set<HostName> suspendedHosts = new HashSet<>(); + private final Map<HostName, HostInfo> suspendedHosts = new HashMap<>(); private final Set<ApplicationId> suspendedApplications = new HashSet<>(); @Override @@ -33,14 +35,13 @@ public class OrchestratorMock extends AbstractComponent implements Orchestrator @Override public HostStatus getNodeStatus(HostName hostName) { - return suspendedHosts.contains(hostName) ? HostStatus.ALLOWED_TO_BE_DOWN : HostStatus.NO_REMARKS; + HostInfo hostInfo = suspendedHosts.get(hostName); + return hostInfo == null ? HostStatus.NO_REMARKS : hostInfo.status(); } @Override - public Function<HostName, Optional<HostInfo>> getNodeStatuses() { - return hostName -> Optional.of(getNodeStatus(hostName)) - .map(status -> status.isSuspended() ? HostInfo.createSuspended(status, Instant.EPOCH) - : HostInfo.createNoRemarks()); + public Function<HostName, Optional<HostInfo>> getHostResolver() { + return hostName -> Optional.of(suspendedHosts.getOrDefault(hostName, HostInfo.createNoRemarks())); } @Override @@ -53,7 +54,7 @@ public class OrchestratorMock extends AbstractComponent implements Orchestrator @Override public void suspend(HostName hostName) { - suspendedHosts.add(hostName); + suspendedHosts.put(hostName, HostInfo.createSuspended(HostStatus.ALLOWED_TO_BE_DOWN, Instant.EPOCH)); } @Override @@ -78,7 +79,9 @@ public class OrchestratorMock extends AbstractComponent implements Orchestrator } @Override - public void acquirePermissionToRemove(HostName hostName) {} + public void acquirePermissionToRemove(HostName hostName) { + suspendedHosts.put(hostName, HostInfo.createSuspended(HostStatus.PERMANENTLY_DOWN, Instant.now())); + } @Override public void suspendAll(HostName parentHostname, List<HostName> hostNames) { 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 287842e56f0..672709a2f8f 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 @@ -93,7 +93,7 @@ public class MetricsReporterTest { ManualClock clock = new ManualClock(Instant.ofEpochSecond(124)); Orchestrator orchestrator = mock(Orchestrator.class); ServiceMonitor serviceMonitor = mock(ServiceMonitor.class); - when(orchestrator.getNodeStatuses()).thenReturn(hostName -> + when(orchestrator.getHostResolver()).thenReturn(hostName -> Optional.of(HostInfo.createSuspended(HostStatus.ALLOWED_TO_BE_DOWN, Instant.ofEpochSecond(1))) ); ServiceModel serviceModel = mock(ServiceModel.class); @@ -144,7 +144,7 @@ public class MetricsReporterTest { Orchestrator orchestrator = mock(Orchestrator.class); ServiceMonitor serviceMonitor = mock(ServiceMonitor.class); - when(orchestrator.getNodeStatuses()).thenReturn(hostName -> Optional.of(HostInfo.createNoRemarks())); + when(orchestrator.getHostResolver()).thenReturn(hostName -> Optional.of(HostInfo.createNoRemarks())); ServiceModel serviceModel = mock(ServiceModel.class); when(serviceMonitor.getServiceModelSnapshot()).thenReturn(serviceModel); when(serviceModel.getServiceInstancesByHostName()).thenReturn(Map.of()); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java index d1331b70251..10fa10f1150 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/Orchestrator.java @@ -50,12 +50,15 @@ public interface Orchestrator { HostStatus getNodeStatus(HostName hostName) throws HostNameNotFoundException; /** - * Returns a not necessarily consistent mapping from host names to their statuses, for hosts known by this. + * Returns a lambda, which when invoked with a hostname, returns its current host info. + * + * <p>When invoked multiple times, the hostname/host info mapping is not necessarily consistent. + * Prefer this to {@link #getNodeStatus(HostName)} when consistency is not required, + * and when doing bulk reads.</p> * - * Prefer this to {@link #getNodeStatus(HostName)} when consistency is not required, and when doing bulk reads. * @return a mapping from host names to their statuses. Unknown hosts map to {@code Optional.empty()}. */ - Function<HostName, Optional<HostInfo>> getNodeStatuses(); + Function<HostName, Optional<HostInfo>> getHostResolver(); void setNodeStatus(HostName hostName, HostStatus state) throws OrchestrationException; diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java index f0b7c2eead1..d5a797ac665 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java @@ -28,6 +28,7 @@ import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy; import com.yahoo.vespa.orchestrator.policy.Policy; import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus; import com.yahoo.vespa.orchestrator.status.HostInfo; +import com.yahoo.vespa.orchestrator.status.HostInfos; import com.yahoo.vespa.orchestrator.status.HostStatus; import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry; import com.yahoo.vespa.orchestrator.status.StatusService; @@ -113,7 +114,7 @@ public class OrchestratorImpl implements Orchestrator { } @Override - public Function<HostName, Optional<HostInfo>> getNodeStatuses() { + public Function<HostName, Optional<HostInfo>> getHostResolver() { return hostName -> instanceLookupService.findInstanceByHost(hostName) .map(application -> statusService.getHostInfo(application.reference(), hostName)); } @@ -334,9 +335,15 @@ public class OrchestratorImpl implements Orchestrator { if (status == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) { ApplicationInstance application = getApplicationInstance(appRef); + HostInfos hostInfosSnapshot = statusRegistry.getHostInfosSnapshot(); + // Mark it allowed to be down before we manipulate the clustercontroller OrchestratorUtil.getHostsUsedByApplicationInstance(application) - .forEach(h -> statusRegistry.setHostState(h, HostStatus.ALLOWED_TO_BE_DOWN)); + .stream() + // This filter also ensures host status is not modified if a suspended host + // has status != ALLOWED_TO_BE_DOWN. + .filter(hostname -> !hostInfosSnapshot.getOrNoRemarks(hostname).status().isSuspended()) + .forEach(hostname -> statusRegistry.setHostState(hostname, HostStatus.ALLOWED_TO_BE_DOWN)); // If the clustercontroller throws an error the nodes will be marked as allowed to be down // and be set back up on next resume invocation. diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImpl.java index eb5dcf790ba..3e43e3e4f73 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImpl.java @@ -10,6 +10,7 @@ import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.OrchestratorUtil; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus; +import com.yahoo.vespa.orchestrator.status.HostInfos; import com.yahoo.vespa.orchestrator.status.HostStatus; import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry; @@ -34,7 +35,7 @@ public class ApplicationApiImpl implements ApplicationApi { private final NodeGroup nodeGroup; private final MutableStatusRegistry hostStatusService; private final List<ClusterApi> clusterInOrder; - private final Map<HostName, HostStatus> hostStatusMap; + private final HostInfos hostInfos; public ApplicationApiImpl(NodeGroup nodeGroup, MutableStatusRegistry hostStatusService, @@ -44,10 +45,8 @@ public class ApplicationApiImpl implements ApplicationApi { this.nodeGroup = nodeGroup; this.hostStatusService = hostStatusService; Collection<HostName> hosts = getHostsUsedByApplicationInstance(applicationInstance); - this.hostStatusMap = hosts.stream().collect(Collectors.toMap(Function.identity(), - hostName -> hostStatusService.getSuspendedHosts().contains(hostName) - ? HostStatus.ALLOWED_TO_BE_DOWN : HostStatus.NO_REMARKS)); - this.clusterInOrder = makeClustersInOrder(nodeGroup, hostStatusMap, clusterControllerClientFactory, numberOfConfigServers); + this.hostInfos = hostStatusService.getHostInfosSnapshot(); + this.clusterInOrder = makeClustersInOrder(nodeGroup, hostInfos, clusterControllerClientFactory, numberOfConfigServers); } @Override @@ -56,7 +55,7 @@ public class ApplicationApiImpl implements ApplicationApi { } private HostStatus getHostStatus(HostName hostName) { - return hostStatusMap.getOrDefault(hostName, HostStatus.NO_REMARKS); + return hostInfos.getOrNoRemarks(hostName).status(); } @Override @@ -67,6 +66,7 @@ public class ApplicationApiImpl implements ApplicationApi { @Override public List<StorageNode> getStorageNodesAllowedToBeDownInGroupInReverseClusterOrder() { return getStorageNodesInGroupInClusterOrder().stream() + // PERMANENTLY_DOWN nodes are NOT included. .filter(storageNode -> getHostStatus(storageNode.hostName()) == HostStatus.ALLOWED_TO_BE_DOWN) .sorted(Comparator.reverseOrder()) .collect(Collectors.toList()); @@ -112,7 +112,8 @@ public class ApplicationApiImpl implements ApplicationApi { .collect(Collectors.toList()); } - private List<ClusterApi> makeClustersInOrder(NodeGroup nodeGroup, Map<HostName, HostStatus> hostStatusMap, + private List<ClusterApi> makeClustersInOrder(NodeGroup nodeGroup, + HostInfos hostInfos, ClusterControllerClientFactory clusterControllerClientFactory, int numberOfConfigServers) { Set<ServiceCluster> clustersInGroup = getServiceClustersInGroup(nodeGroup); @@ -121,7 +122,7 @@ public class ApplicationApiImpl implements ApplicationApi { this, serviceCluster, nodeGroup, - hostStatusMap, + hostInfos, clusterControllerClientFactory, numberOfConfigServers)) .sorted(ApplicationApiImpl::compareClusters) diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java index 2aaebb18aa6..b747d8c2e22 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApiImpl.java @@ -8,6 +8,7 @@ import com.yahoo.vespa.applicationmodel.ServiceInstance; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.applicationmodel.ServiceType; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; +import com.yahoo.vespa.orchestrator.status.HostInfos; import com.yahoo.vespa.orchestrator.status.HostStatus; import java.util.Collections; @@ -27,7 +28,7 @@ class ClusterApiImpl implements ClusterApi { private final ApplicationApi applicationApi; private final ServiceCluster serviceCluster; private final NodeGroup nodeGroup; - private final Map<HostName, HostStatus> hostStatusMap; + private final HostInfos hostInfos; private final ClusterControllerClientFactory clusterControllerClientFactory; private final Set<ServiceInstance> servicesInGroup; private final Set<ServiceInstance> servicesDownInGroup; @@ -50,13 +51,13 @@ class ClusterApiImpl implements ClusterApi { public ClusterApiImpl(ApplicationApi applicationApi, ServiceCluster serviceCluster, NodeGroup nodeGroup, - Map<HostName, HostStatus> hostStatusMap, + HostInfos hostInfos, ClusterControllerClientFactory clusterControllerClientFactory, int numberOfConfigServers) { this.applicationApi = applicationApi; this.serviceCluster = serviceCluster; this.nodeGroup = nodeGroup; - this.hostStatusMap = hostStatusMap; + this.hostInfos = hostInfos; this.clusterControllerClientFactory = clusterControllerClientFactory; Map<Boolean, Set<ServiceInstance>> serviceInstancesByLocality = @@ -144,7 +145,7 @@ class ClusterApiImpl implements ClusterApi { public String nodesAllowedToBeDownNotInGroupDescription() { return servicesNotInGroup.stream() .map(ServiceInstance::hostName) - .filter(hostName -> hostStatus(hostName) == HostStatus.ALLOWED_TO_BE_DOWN) + .filter(hostName -> hostStatus(hostName).isSuspended()) .sorted() .distinct() .collect(Collectors.toList()) @@ -206,11 +207,11 @@ class ClusterApiImpl implements ClusterApi { } private HostStatus hostStatus(HostName hostName) { - return hostStatusMap.getOrDefault(hostName, HostStatus.NO_REMARKS); + return hostInfos.getOrNoRemarks(hostName).status(); } private boolean serviceEffectivelyDown(ServiceInstance service) { - if (hostStatus(service.hostName()) == HostStatus.ALLOWED_TO_BE_DOWN) { + if (hostStatus(service.hostName()).isSuspended()) { return true; } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceResource.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceResource.java index bd265ed39e4..7b8a74d7fe2 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceResource.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceResource.java @@ -14,7 +14,7 @@ import com.yahoo.vespa.applicationmodel.ServiceType; import com.yahoo.vespa.orchestrator.InstanceLookupService; import com.yahoo.vespa.orchestrator.OrchestratorUtil; import com.yahoo.vespa.orchestrator.restapi.wire.SlobrokEntryResponse; -import com.yahoo.vespa.orchestrator.status.HostStatus; +import com.yahoo.vespa.orchestrator.status.HostInfos; import com.yahoo.vespa.orchestrator.status.StatusService; import com.yahoo.vespa.service.manager.MonitorManager; import com.yahoo.vespa.service.manager.UnionMonitorManager; @@ -81,12 +81,11 @@ public class InstanceResource { = instanceLookupService.findInstanceById(instanceId) .orElseThrow(() -> new WebApplicationException(Response.status(Response.Status.NOT_FOUND).build())); - Set<HostName> suspendedHosts = statusService.getSuspendedHostsByApplication().apply(applicationInstance.reference()); + HostInfos hostInfos = statusService.getHostInfosByApplicationResolver().apply(applicationInstance.reference()); Map<HostName, String> hostStatusMap = getHostsUsedByApplicationInstance(applicationInstance) .stream() .collect(Collectors.toMap(hostName -> hostName, - hostName -> suspendedHosts.contains(hostName) ? HostStatus.ALLOWED_TO_BE_DOWN.name() - : HostStatus.NO_REMARKS.name())); + hostName -> hostInfos.getOrNoRemarks(hostName).status().asString())); return InstanceStatusResponse.create(applicationInstance, hostStatusMap); } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceStatusResponse.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceStatusResponse.java index c2ea0c9eddf..068423f7d24 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceStatusResponse.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/InstanceStatusResponse.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.orchestrator.resources; import com.fasterxml.jackson.annotation.JsonProperty; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.status.HostInfo; import java.util.Map; import java.util.Objects; diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfos.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfos.java index fe78a41672b..e644ef4c103 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfos.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfos.java @@ -8,10 +8,11 @@ import java.util.Set; import java.util.stream.Collectors; /** - * Collection of suspended hosts. + * Collection of the suspended hosts of an application. * * @author hakonhall */ +// @Immutable public class HostInfos { private final Map<HostName, HostInfo> hostInfos; @@ -24,7 +25,7 @@ public class HostInfos { } /** Get all suspended hostnames. */ - public Set<HostName> suspendedHostsnames() { + public Set<HostName> suspendedHostnames() { return hostInfos.entrySet().stream() .filter(entry -> entry.getValue().status().isSuspended()) .map(entry -> entry.getKey()) @@ -32,7 +33,7 @@ public class HostInfos { } /** Get host info for hostname, returning a NO_REMARKS HostInfo if unknown. */ - public HostInfo get(HostName hostname) { + public HostInfo getOrNoRemarks(HostName hostname) { return hostInfos.getOrDefault(hostname, HostInfo.createNoRemarks()); } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfosCache.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfosCache.java index 9998e7de918..680f3cbcc6d 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfosCache.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostInfosCache.java @@ -28,17 +28,24 @@ public class HostInfosCache implements HostInfosService { this.cacheGeneration = new AtomicLong(counter.get()); } - @Override - public HostInfos getHostInfos(ApplicationInstanceReference application) { + public void refreshCache() { long newCacheGeneration = counter.get(); if (cacheGeneration.getAndSet(newCacheGeneration) != newCacheGeneration) { suspendedHosts.clear(); } + } + public HostInfos getCachedHostInfos(ApplicationInstanceReference application) { return suspendedHosts.computeIfAbsent(application, wrappedService::getHostInfos); } @Override + public HostInfos getHostInfos(ApplicationInstanceReference application) { + refreshCache(); + return getCachedHostInfos(application); + } + + @Override public boolean setHostStatus(ApplicationInstanceReference application, HostName hostName, HostStatus hostStatus) { boolean isException = true; boolean modified = false; diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostStatus.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostStatus.java index 6764ffb48ea..330e53f9586 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostStatus.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/HostStatus.java @@ -23,4 +23,5 @@ public enum HostStatus { HostStatus(boolean suspended) { this.suspended = suspended; } public boolean isSuspended() { return suspended; } + public String asString() { return name(); } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/MutableStatusRegistry.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/MutableStatusRegistry.java index 92b0ec50011..7703b50b8f3 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/MutableStatusRegistry.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/MutableStatusRegistry.java @@ -28,6 +28,11 @@ public interface MutableStatusRegistry extends AutoCloseable { Set<HostName> getSuspendedHosts(); /** + * Returns a snapshot of all host infos for this application. + */ + HostInfos getHostInfosSnapshot(); + + /** * Sets the state for the given host. */ void setHostState(HostName hostName, HostStatus status); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java index 9e3ee84e1d9..e2be5ec7eb6 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/StatusService.java @@ -57,12 +57,12 @@ public interface StatusService { Set<ApplicationInstanceReference> getAllSuspendedApplications(); /** - * Returns a fresh, but not necessarily consistent mapping from applications to their set of suspended hosts. + * Returns a lambda, which when invoked for an application, returns an up-to-date snapshot of {@link HostInfos host infos}. * - * If the lock for an application is held when this mapping is acquired, new sets returned for that application - * are consistent and up to date for as long as the lock is held. (The sets themselves don't reflect changes.) + * <p>Unless the lock for the application is held, the returned snapshot may already be out of date. + * (The snapshot itself is immutable.)</p> */ - Function<ApplicationInstanceReference, Set<HostName>> getSuspendedHostsByApplication(); + Function<ApplicationInstanceReference, HostInfos> getHostInfosByApplicationResolver(); /** Returns the status of the given application. This is consistent if its lock is held.*/ ApplicationInstanceStatus getApplicationInstanceStatus(ApplicationInstanceReference application); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java index c4a23baae2e..d791c76ba6b 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java @@ -99,12 +99,13 @@ public class ZookeeperStatusService implements StatusService { /** * Cache is checked for freshness when this mapping is created, and may be invalidated again later - * by other users of the cache. Since this function is backed by the cache, any such invalidations + * by other users of the cache. Since this function is backed by the cache, any such invalidation * will be reflected in the returned mapping; all users of the cache collaborate in repopulating it. */ @Override - public Function<ApplicationInstanceReference, Set<HostName>> getSuspendedHostsByApplication() { - return application -> hostInfosCache.getHostInfos(application).suspendedHostsnames(); + public Function<ApplicationInstanceReference, HostInfos> getHostInfosByApplicationResolver() { + hostInfosCache.refreshCache(); + return hostInfosCache::getCachedHostInfos; } @@ -255,7 +256,7 @@ public class ZookeeperStatusService implements StatusService { @Override public HostInfo getHostInfo(ApplicationInstanceReference applicationInstanceReference, HostName hostName) { - return hostInfosCache.getHostInfos(applicationInstanceReference).get(hostName); + return hostInfosCache.getHostInfos(applicationInstanceReference).getOrNoRemarks(hostName); } /** Do not call this directly: should be called behind a cache. */ @@ -363,7 +364,12 @@ public class ZookeeperStatusService implements StatusService { @Override public Set<HostName> getSuspendedHosts() { - return hostInfosCache.getHostInfos(applicationInstanceReference).suspendedHostsnames(); + return hostInfosCache.getHostInfos(applicationInstanceReference).suspendedHostnames(); + } + + @Override + public HostInfos getHostInfosSnapshot() { + return hostInfosCache.getHostInfos(applicationInstanceReference); } @Override diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java index 8e42ccdc79d..62925dc003e 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java @@ -13,6 +13,7 @@ import com.yahoo.vespa.applicationmodel.TenantId; import com.yahoo.vespa.orchestrator.OrchestratorUtil; import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; import com.yahoo.vespa.orchestrator.policy.HostedVespaClusterPolicy; +import com.yahoo.vespa.orchestrator.status.HostInfos; import com.yahoo.vespa.orchestrator.status.HostStatus; import org.junit.Test; @@ -75,7 +76,7 @@ public class ClusterApiImplTest { applicationApi, serviceCluster, new NodeGroup(modelUtils.createApplicationInstance(new ArrayList<>()), hostName5), - modelUtils.getHostStatusMap(), + modelUtils.getHostInfos(), modelUtils.getClusterControllerClientFactory(), ModelTestUtils.NUMBER_OF_CONFIG_SERVERS); assertEquals("{ clusterId=cluster, serviceType=service-type }", clusterApi.clusterInfo()); @@ -184,7 +185,7 @@ public class ClusterApiImplTest { applicationApi, serviceCluster, new NodeGroup(modelUtils.createApplicationInstance(new ArrayList<>()), groupNodes), - modelUtils.getHostStatusMap(), + modelUtils.getHostInfos(), modelUtils.getClusterControllerClientFactory(), ModelTestUtils.NUMBER_OF_CONFIG_SERVERS); assertEquals(expectedNoServicesInGroupIsUp, clusterApi.noServicesInGroupIsUp()); @@ -214,7 +215,7 @@ public class ClusterApiImplTest { applicationApi, serviceCluster, new NodeGroup(applicationInstance, hostName1, hostName3), - new HashMap<>(), + new HostInfos(), modelUtils.getClusterControllerClientFactory(), ModelTestUtils.NUMBER_OF_CONFIG_SERVERS); assertTrue(clusterApi.isStorageCluster()); @@ -254,7 +255,7 @@ public class ClusterApiImplTest { applicationApi, serviceCluster, new NodeGroup(application, hostnames.get(0)), - modelUtils.getHostStatusMap(), + modelUtils.getHostInfos(), modelUtils.getClusterControllerClientFactory(), clusterSize); assertEquals(clusterSize - serviceStatusList.size(), clusterApi.missingServices()); diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java index 729b3ae79ff..87e5f226c42 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ModelTestUtils.java @@ -25,6 +25,8 @@ import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactoryMock; import com.yahoo.vespa.orchestrator.policy.HostedVespaClusterPolicy; import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy; +import com.yahoo.vespa.orchestrator.status.HostInfo; +import com.yahoo.vespa.orchestrator.status.HostInfos; import com.yahoo.vespa.orchestrator.status.HostStatus; import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry; import com.yahoo.vespa.orchestrator.status.StatusService; @@ -33,11 +35,13 @@ import com.yahoo.vespa.service.monitor.ServiceModel; import com.yahoo.yolean.Exceptions; import java.time.Clock; +import java.time.Instant; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import static org.mockito.Mockito.mock; @@ -64,8 +68,23 @@ class ModelTestUtils { return new ApplicationApiFactory(NUMBER_OF_CONFIG_SERVERS); } - Map<HostName, HostStatus> getHostStatusMap() { - return hostStatusMap; + HostInfos getHostInfos() { + Instant now = Instant.now(); + + Map<HostName, HostInfo> hostInfosMap = hostStatusMap.entrySet().stream() + .collect(Collectors.toMap( + entry -> entry.getKey(), + entry -> { + HostStatus status = entry.getValue(); + if (status == HostStatus.NO_REMARKS) { + return HostInfo.createNoRemarks(); + } else { + return HostInfo.createSuspended(status, now); + } + } + )); + + return new HostInfos(hostInfosMap); } HostName createNode(String name, HostStatus hostStatus) { |