diff options
author | Håkon Hallingstad <hakon@verizonmedia.com> | 2020-02-07 15:51:35 +0100 |
---|---|---|
committer | Håkon Hallingstad <hakon@verizonmedia.com> | 2020-02-07 15:51:35 +0100 |
commit | 8cfc11479049405eb66ec3de50496c0a39ab9775 (patch) | |
tree | afae115a9c428e420920f26bcf50e168542df7d3 | |
parent | 5d8695688086c344f5778bf682301f883cd4e9dc (diff) |
Support setting PERMANENTLY_DOWN at end of retirement
8 files changed, 75 insertions, 27 deletions
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index a8ee5c7fbd7..0d9e45a1115 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -122,6 +122,13 @@ public class Flags { "Takes immediate effect for new batch suspensions.", HOSTNAME); + public static final UnboundBooleanFlag RETIRE_WITH_PERMANENTLY_DOWN = defineFeatureFlag( + "retire-with-permanently-down", true, + "If enabled, retirement will end with setting the host status to PERMANENTLY_DOWN, " + + "instead of ALLOWED_TO_BE_DOWN (old behavior).", + "Takes effect on the first run of RetiredExpirer.", + HOSTNAME); + public static final UnboundBooleanFlag ENABLE_DYNAMIC_PROVISIONING = defineFeatureFlag( "enable-dynamic-provisioning", false, "Provision a new docker host when we otherwise can't allocate a docker node", diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java index 17dfb924973..eb6a4119f8a 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java @@ -30,6 +30,7 @@ public class OrchestratorContext implements AutoCloseable { private final TimeBudget timeBudget; private final boolean probe; private final boolean largeLocks; + private final boolean usePermanentlyDownStatus; // The key set is the set of applications locked by this context tree: Only the // root context has a non-empty set. The value is an unlock callback to be called @@ -38,24 +39,32 @@ public class OrchestratorContext implements AutoCloseable { /** Create an OrchestratorContext for operations on multiple applications. */ public static OrchestratorContext createContextForMultiAppOp(Clock clock, boolean largeLocks) { - return new OrchestratorContext(null, clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_BATCH_OP), false, largeLocks); + return new OrchestratorContext(null, clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_BATCH_OP), + false, largeLocks, false); } /** Create an OrchestratorContext for an operation on a single application. */ public static OrchestratorContext createContextForSingleAppOp(Clock clock) { - return new OrchestratorContext(null, clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_SINGLE_OP), false, false); + return createContextForSingleAppOp(clock, false); + } + + public static OrchestratorContext createContextForSingleAppOp(Clock clock, boolean usePermanentlyDownStatus) { + return new OrchestratorContext(null, clock, TimeBudget.fromNow(clock, DEFAULT_TIMEOUT_FOR_SINGLE_OP), + false, false, usePermanentlyDownStatus); } private OrchestratorContext(OrchestratorContext parentOrNull, Clock clock, TimeBudget timeBudget, boolean probe, - boolean largeLocks) { + boolean largeLocks, + boolean usePermanentlyDownStatus) { this.parent = Optional.ofNullable(parentOrNull); this.clock = clock; this.timeBudget = timeBudget; this.probe = probe; this.largeLocks = largeLocks; + this.usePermanentlyDownStatus = usePermanentlyDownStatus; } public Duration getTimeLeft() { @@ -74,6 +83,9 @@ public class OrchestratorContext implements AutoCloseable { /** Whether application locks acquired during probing of a batch suspend should be closed after the non-probe is done. */ public boolean largeLocks() { return largeLocks; } + /** Whether the PERMANENTLY_DOWN host status should be used (where appropriate). */ + public boolean usePermanentlyDownStatus() { return usePermanentlyDownStatus; } + /** * Returns true if 1. large locks is enabled, and 2. * {@link #registerLockAcquisition(ApplicationInstanceReference, Runnable) registerLockAcquisition} @@ -111,7 +123,7 @@ public class OrchestratorContext implements AutoCloseable { // Move deadline towards past by a fixed amount to ensure there's time to process exceptions and // access ZooKeeper before the lock times out. TimeBudget subTimeBudget = timeBudget.withDeadline(timeBudget.deadline().get().minus(TIMEOUT_OVERHEAD)); - return new OrchestratorContext(this, clock, subTimeBudget, probe, largeLocks); + return new OrchestratorContext(this, clock, subTimeBudget, probe, largeLocks, usePermanentlyDownStatus); } /** Create an OrchestratorContext for an operation on a single application, but limited to current timeout. */ @@ -124,7 +136,7 @@ public class OrchestratorContext implements AutoCloseable { } TimeBudget timeBudget = TimeBudget.from(clock, now, Optional.of(Duration.between(now, deadline))); - return new OrchestratorContext(this, clock, timeBudget, probe, largeLocks); + return new OrchestratorContext(this, clock, timeBudget, probe, largeLocks, usePermanentlyDownStatus); } @Override 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 244fd4d9b5d..414548f8bdc 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java @@ -65,6 +65,7 @@ public class OrchestratorImpl implements Orchestrator { private final Clock clock; private final ApplicationApiFactory applicationApiFactory; private final BooleanFlag enableLargeOrchestratorLocks; + private final BooleanFlag retireWithPermanentlyDownFlag; @Inject public OrchestratorImpl(ClusterControllerClientFactory clusterControllerClientFactory, @@ -101,6 +102,7 @@ public class OrchestratorImpl implements Orchestrator { this.clock = clock; this.applicationApiFactory = applicationApiFactory; this.enableLargeOrchestratorLocks = Flags.ENABLE_LARGE_ORCHESTRATOR_LOCKS.bindTo(flagSource); + this.retireWithPermanentlyDownFlag = Flags.RETIRE_WITH_PERMANENTLY_DOWN.bindTo(flagSource); } @Override @@ -150,6 +152,12 @@ public class OrchestratorImpl implements Orchestrator { * monitoring will have had time to catch up. Since we don't want do the delay with the lock held, * and the host status service's locking functionality does not support something like condition * variables or Object.wait(), we break out here, releasing the lock before delaying. + * + * 2020-02-07: We should utilize suspendedSince timestamp on the HostInfo: The above + * is equivalent to guaranteeing a minimum time after suspendedSince, before checking + * the health with service monitor. This should for all practical purposes remove + * the amount of time in this sleep. + * Caveat: Cannot be implemented before lingering HostInfo has been fixed (VESPA-17546). */ sleep(serviceMonitorConvergenceLatencySeconds, TimeUnit.SECONDS); @@ -159,15 +167,21 @@ public class OrchestratorImpl implements Orchestrator { try (MutableStatusRegistry statusRegistry = statusService .lockApplicationInstance_forCurrentThreadOnly(context, appInstance.reference())) { HostStatus currentHostState = statusRegistry.getHostInfo(hostName).status(); - - if (HostStatus.NO_REMARKS == currentHostState) { + if (currentHostState == HostStatus.NO_REMARKS) { return; } - ApplicationInstanceStatus appStatus = statusRegistry.getStatus(); - if (appStatus == ApplicationInstanceStatus.NO_REMARKS) { - policy.releaseSuspensionGrant(context.createSubcontextWithinLock(), appInstance, hostName, statusRegistry); + // In 2 cases the resume will appear to succeed (no exception thrown), + // but the host status and content cluster states will not be changed accordingly: + // 1. When host is permanently down: the host will be removed from the application asap. + // 2. The whole application is down: the content cluster states are set to maintenance, + // and the host may be taken down manually at any moment. + if (currentHostState == HostStatus.PERMANENTLY_DOWN || + statusRegistry.getStatus() == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) { + return; } + + policy.releaseSuspensionGrant(context.createSubcontextWithinLock(), appInstance, hostName, statusRegistry); } } @@ -183,7 +197,10 @@ public class OrchestratorImpl implements Orchestrator { ApplicationInstance appInstance = getApplicationInstance(hostName); NodeGroup nodeGroup = new NodeGroup(appInstance, hostName); - OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock); + boolean usePermanentlyDownStatus = retireWithPermanentlyDownFlag + .with(FetchVector.Dimension.HOSTNAME, hostName.s()) + .value(); + OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock, usePermanentlyDownStatus); try (MutableStatusRegistry statusRegistry = statusService .lockApplicationInstance_forCurrentThreadOnly(context, appInstance.reference())) { ApplicationApi applicationApi = applicationApiFactory.create(nodeGroup, statusRegistry, diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApi.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApi.java index 2e85713d323..a6353e39610 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApi.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/ApplicationApi.java @@ -8,6 +8,7 @@ import com.yahoo.vespa.orchestrator.status.ApplicationInstanceStatus; import com.yahoo.vespa.orchestrator.status.HostStatus; import java.util.List; +import java.util.function.Predicate; /** * The API a Policy has access to @@ -29,10 +30,12 @@ public interface ApplicationApi { ApplicationInstanceStatus getApplicationStatus(); void setHostState(OrchestratorContext context, HostName hostName, HostStatus status); - List<HostName> getNodesInGroupWithStatus(HostStatus status); + List<HostName> getNodesInGroupWith(Predicate<HostStatus> statusPredicate); + default List<HostName> getNodesInGroupWithStatus(HostStatus requiredStatus) { + return getNodesInGroupWith(status -> status == requiredStatus); + } List<StorageNode> getStorageNodesInGroupInClusterOrder(); List<StorageNode> getUpStorageNodesInGroupInClusterOrder(); - List<StorageNode> getStorageNodesAllowedToBeDownInGroupInReverseClusterOrder(); - + List<StorageNode> getSuspendedStorageNodesInGroupInReverseClusterOrder(); } 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 def5edd0a97..cf6946fa7f8 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 @@ -20,6 +20,7 @@ import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.Predicate; import java.util.stream.Collectors; import static com.yahoo.vespa.orchestrator.OrchestratorUtil.getHostsUsedByApplicationInstance; @@ -62,10 +63,9 @@ public class ApplicationApiImpl implements ApplicationApi { } @Override - public List<StorageNode> getStorageNodesAllowedToBeDownInGroupInReverseClusterOrder() { + public List<StorageNode> getSuspendedStorageNodesInGroupInReverseClusterOrder() { return getStorageNodesInGroupInClusterOrder().stream() - // PERMANENTLY_DOWN nodes are NOT included. - .filter(storageNode -> getHostStatus(storageNode.hostName()) == HostStatus.ALLOWED_TO_BE_DOWN) + .filter(storageNode -> getHostStatus(storageNode.hostName()).isSuspended()) .sorted(Comparator.reverseOrder()) .collect(Collectors.toList()); } @@ -104,9 +104,9 @@ public class ApplicationApiImpl implements ApplicationApi { } @Override - public List<HostName> getNodesInGroupWithStatus(HostStatus status) { + public List<HostName> getNodesInGroupWith(Predicate<HostStatus> statusPredicate) { return nodeGroup.getHostNames().stream() - .filter(hostName -> getHostStatus(hostName) == status) + .filter(hostName -> statusPredicate.test(getHostStatus(hostName))) .collect(Collectors.toList()); } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java index a9b9736ebfb..f6a1e4f91f0 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicy.java @@ -30,7 +30,9 @@ public class HostedVespaPolicy implements Policy { private final ClusterControllerClientFactory clusterControllerClientFactory; private final ApplicationApiFactory applicationApiFactory; - public HostedVespaPolicy(HostedVespaClusterPolicy clusterPolicy, ClusterControllerClientFactory clusterControllerClientFactory, ApplicationApiFactory applicationApiFactory) { + public HostedVespaPolicy(HostedVespaClusterPolicy clusterPolicy, + ClusterControllerClientFactory clusterControllerClientFactory, + ApplicationApiFactory applicationApiFactory) { this.clusterPolicy = clusterPolicy; this.clusterControllerClientFactory = clusterControllerClientFactory; this.applicationApiFactory = applicationApiFactory; @@ -60,10 +62,11 @@ public class HostedVespaPolicy implements Policy { public void releaseSuspensionGrant(OrchestratorContext context, ApplicationApi application) throws HostStateChangeDeniedException { // Always defer to Cluster Controller whether it's OK to resume storage node - for (StorageNode storageNode : application.getStorageNodesAllowedToBeDownInGroupInReverseClusterOrder()) { + for (StorageNode storageNode : application.getSuspendedStorageNodesInGroupInReverseClusterOrder()) { storageNode.setNodeState(context, ClusterControllerNodeState.UP); } + // In particular, we're not modifying the state of PERMANENTLY_DOWN nodes. for (HostName hostName : application.getNodesInGroupWithStatus(HostStatus.ALLOWED_TO_BE_DOWN)) { application.setHostState(context, hostName, HostStatus.NO_REMARKS); } @@ -92,9 +95,15 @@ public class HostedVespaPolicy implements Policy { storageNode.setNodeState(context, ClusterControllerNodeState.DOWN); } - // Ensure all nodes in the group are marked as allowed to be down - for (HostName hostName : applicationApi.getNodesInGroupWithStatus(HostStatus.NO_REMARKS)) { - applicationApi.setHostState(context, hostName, HostStatus.ALLOWED_TO_BE_DOWN); + if (context.usePermanentlyDownStatus()) { + // Ensure all nodes in the group are marked as permanently down + for (HostName hostName : applicationApi.getNodesInGroupWith(status -> status != HostStatus.PERMANENTLY_DOWN)) { + applicationApi.setHostState(context, hostName, HostStatus.PERMANENTLY_DOWN); + } + } else { + for (HostName hostName : applicationApi.getNodesInGroupWith(status -> status != HostStatus.ALLOWED_TO_BE_DOWN)) { + applicationApi.setHostState(context, hostName, HostStatus.ALLOWED_TO_BE_DOWN); + } } } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImplTest.java index d5734a73de0..ee33a92367b 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImplTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ApplicationApiImplTest.java @@ -327,7 +327,7 @@ public class ApplicationApiImplTest { private void verifyStorageNodesAllowedToBeDown( ApplicationApi applicationApi, HostName... hostNames) { List<HostName> actualStorageNodes = - applicationApi.getStorageNodesAllowedToBeDownInGroupInReverseClusterOrder().stream() + applicationApi.getSuspendedStorageNodesInGroupInReverseClusterOrder().stream() .map(storageNode -> storageNode.hostName()) .collect(Collectors.toList()); assertEquals(Arrays.asList(hostNames), actualStorageNodes); diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java index b27e37ac034..ed6917a3a4e 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaPolicyTest.java @@ -120,7 +120,7 @@ public class HostedVespaPolicyTest { when(applicationApi.getStorageNodesInGroupInClusterOrder()).thenReturn(upStorageNodes); List<HostName> noRemarksHostNames = Arrays.asList(hostName1, hostName2, hostName3); - when(applicationApi.getNodesInGroupWithStatus(HostStatus.NO_REMARKS)).thenReturn(noRemarksHostNames); + when(applicationApi.getNodesInGroupWith(any())).thenReturn(noRemarksHostNames); InOrder order = inOrder(applicationApi, clusterPolicy, storageNode1, storageNode3); @@ -136,7 +136,7 @@ public class HostedVespaPolicyTest { order.verify(storageNode1).setNodeState(context, ClusterControllerNodeState.DOWN); order.verify(storageNode3).setNodeState(context, ClusterControllerNodeState.DOWN); - order.verify(applicationApi).getNodesInGroupWithStatus(HostStatus.NO_REMARKS); + order.verify(applicationApi).getNodesInGroupWith(any()); order.verify(applicationApi).setHostState(context, hostName1, HostStatus.ALLOWED_TO_BE_DOWN); order.verify(applicationApi).setHostState(context, hostName2, HostStatus.ALLOWED_TO_BE_DOWN); order.verify(applicationApi).setHostState(context, hostName3, HostStatus.ALLOWED_TO_BE_DOWN); |