From b2bb08d5bb71aca0dfc6416084715d5b820b2133 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Mon, 24 Feb 2020 21:52:42 +0100 Subject: Improve suspension denied reason --- .../yahoo/vespa/orchestrator/model/ClusterApi.java | 3 +- .../vespa/orchestrator/model/ClusterApiImpl.java | 61 ++++++++++++++++------ .../policy/HostStateChangeDeniedException.java | 18 ++----- .../policy/HostedVespaClusterPolicy.java | 10 ++-- 4 files changed, 53 insertions(+), 39 deletions(-) (limited to 'orchestrator/src/main/java') diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApi.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApi.java index e9bb4984c2e..65c45c8df76 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApi.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ClusterApi.java @@ -26,7 +26,6 @@ public interface ClusterApi { Optional storageNodeInGroup(); Optional upStorageNodeInGroup(); - String servicesDownAndNotInGroupDescription(); - String nodesAllowedToBeDownNotInGroupDescription(); + String downDescription(); } 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 b747d8c2e22..24f56eac85d 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 @@ -131,25 +131,56 @@ class ClusterApiImpl implements ClusterApi { return numberOfServicesDown * 100 / (serviceCluster.serviceInstances().size() + missingServices); } + /** + * A description of the hosts outside the group that are allowed to be down, + * and a description of the services outside the group and outside of the allowed services + * that are down. + */ @Override - public String servicesDownAndNotInGroupDescription() { - // Sort these for readability and testing stability - return Stream - .concat(servicesDownAndNotInGroup.stream().map(ServiceInstance::toString).sorted(), - missingServices > 0 ? Stream.of(descriptionOfMissingServices) : Stream.of()) - .collect(Collectors.toList()) - .toString(); - } + public String downDescription() { + StringBuilder description = new StringBuilder(); - @Override - public String nodesAllowedToBeDownNotInGroupDescription() { - return servicesNotInGroup.stream() + Set suspended = servicesNotInGroup.stream() .map(ServiceInstance::hostName) .filter(hostName -> hostStatus(hostName).isSuspended()) - .sorted() - .distinct() - .collect(Collectors.toList()) - .toString(); + .collect(Collectors.toSet()); + + if (suspended.size() > 0) { + description.append(" "); + + final int nodeLimit = 3; + description.append("Suspended hosts: "); + description.append(suspended.stream().sorted().distinct().limit(nodeLimit).collect(Collectors.toList()).toString()); + if (suspended.size() > nodeLimit) { + description.append(", and " + (suspended.size() - nodeLimit) + " more"); + } + description.append("."); + } + + Set downElsewhere = servicesDownAndNotInGroup.stream() + .filter(serviceInstance -> !suspended.contains(serviceInstance.hostName())) + .collect(Collectors.toSet()); + + final int downElsewhereTotal = downElsewhere.size() + missingServices; + if (downElsewhereTotal > 0) { + description.append(" "); + + final int serviceLimit = 2; // services info is verbose + description.append("Services down on resumed hosts: "); + description.append(Stream.concat( + downElsewhere.stream().map(ServiceInstance::toString).sorted(), + missingServices > 0 ? Stream.of(descriptionOfMissingServices) : Stream.of()) + .limit(serviceLimit) + .collect(Collectors.toList()) + .toString()); + + if (downElsewhereTotal > serviceLimit) { + description.append(", and " + (downElsewhereTotal - serviceLimit) + " more"); + } + description.append("."); + } + + return description.toString(); } private Optional storageNodeInGroup(Predicate storageServicePredicate) { diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostStateChangeDeniedException.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostStateChangeDeniedException.java index e13cf17d420..a6b3cbc87dc 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostStateChangeDeniedException.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostStateChangeDeniedException.java @@ -2,55 +2,43 @@ package com.yahoo.vespa.orchestrator.policy; import com.yahoo.vespa.applicationmodel.HostName; -import com.yahoo.vespa.applicationmodel.ServiceType; import com.yahoo.vespa.orchestrator.OrchestrationException; import com.yahoo.vespa.orchestrator.model.NodeGroup; -import java.util.Optional; - /** * @author bakksjo */ public class HostStateChangeDeniedException extends OrchestrationException { private final String constraintName; - private final Optional serviceType; public HostStateChangeDeniedException(HostName hostName, String constraintName, String message) { this(hostName, constraintName, message, null); } public HostStateChangeDeniedException(HostName hostName, String constraintName, String message, Exception e) { - this(hostName.s(), constraintName, Optional.empty(), message, e); + this(hostName.s(), constraintName, message, e); } public HostStateChangeDeniedException(NodeGroup nodeGroup, String constraintName, String message) { - this(nodeGroup.toCommaSeparatedString(), constraintName, Optional.empty(), message, null); + this(nodeGroup.toCommaSeparatedString(), constraintName, message, null); } private HostStateChangeDeniedException(String nodes, String constraintName, - Optional serviceType, String message, Throwable cause) { - super(createMessage(nodes, constraintName, serviceType, message), cause); + super(createMessage(nodes, constraintName, message), cause); this.constraintName = constraintName; - this.serviceType = serviceType; } private static String createMessage(String nodes, String constraintName, - Optional serviceType, String message) { return "Changing the state of " + nodes + " would violate " + constraintName - + (serviceType.isPresent() ? " for service type " + serviceType.get() : "") + ": " + message; } public String getConstraintName() { return constraintName; } - - public Optional getServiceType() { - return serviceType; - } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java index 1e895d0e757..ccb0bb57186 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java @@ -26,13 +26,11 @@ public class HostedVespaClusterPolicy implements ClusterPolicy { throw new HostStateChangeDeniedException( clusterApi.getNodeGroup(), ENOUGH_SERVICES_UP_CONSTRAINT, - "Suspension percentage for service type " + clusterApi.serviceType() + "Suspension for service type " + clusterApi.serviceType() + " would increase from " + clusterApi.percentageOfServicesDown() + "% to " + clusterApi.percentageOfServicesDownIfGroupIsAllowedToBeDown() + "%, over the limit of " + percentageOfServicesAllowedToBeDown + "%." - + " These instances may be down: " + clusterApi.servicesDownAndNotInGroupDescription() - + " and these hosts are allowed to be down: " - + clusterApi.nodesAllowedToBeDownNotInGroupDescription()); + + clusterApi.downDescription()); } @Override @@ -56,9 +54,7 @@ public class HostedVespaClusterPolicy implements ClusterPolicy { "Down percentage for service type " + clusterApi.serviceType() + " would increase to " + clusterApi.percentageOfServicesDownIfGroupIsAllowedToBeDown() + "%, over the limit of " + percentageOfServicesAllowedToBeDown + "%." - + " These instances may be down: " + clusterApi.servicesDownAndNotInGroupDescription() - + " and these hosts are allowed to be down: " - + clusterApi.nodesAllowedToBeDownNotInGroupDescription()); + + clusterApi.downDescription()); } // Non-private for testing purposes -- cgit v1.2.3