diff options
author | Håkon Hallingstad <hakon@oath.com> | 2018-06-19 15:19:50 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@oath.com> | 2018-06-19 15:19:50 +0200 |
commit | 4b0cb1bcc2a6ee5890e59360bcb70d591a3a590e (patch) | |
tree | e82b5384a48ac78513896fdaa3bdf3b9189be58c /orchestrator | |
parent | 324d68bb381d070e8a419948d74509d4136cae3a (diff) |
Add timeout to set-node-state calls from Orchestrator
Diffstat (limited to 'orchestrator')
18 files changed, 188 insertions, 101 deletions
diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java new file mode 100644 index 00000000000..9e605561e7b --- /dev/null +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorContext.java @@ -0,0 +1,44 @@ +// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.orchestrator; + +import com.yahoo.jdisc.TimeBudget; +import com.yahoo.jdisc.Timer; + +import java.time.Duration; +import java.util.Optional; + +/** + * Context for the Orchestrator, e.g. timeout management. + * + * @author hakon + */ +public class OrchestratorContext { + private static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(10); + private static final Duration POST_OPERATION_HEADROOM = Duration.ofMillis(100); + + private TimeBudget timeBudget; + + public OrchestratorContext(Timer timer) { + this.timeBudget = TimeBudget.fromNow(timer, DEFAULT_TIMEOUT); + } + + /** Get the original timeout in seconds. */ + public long getOriginalTimeoutInSeconds() { + return timeBudget.originalTimeout().getSeconds(); + } + + /** + * Get number of seconds until the deadline, or empty if there's no deadline. + * + * <p>The returned timeout is slightly shorter than the actual timeout to ensure there's + * enough time to wrap up and return from the Orchestrator between when the operation + * times out and the actual timeout. + */ + public Optional<Float> getSuboperationTimeoutInSeconds() { + return getSuboperationTimeoutInSeconds(POST_OPERATION_HEADROOM); + } + + private Optional<Float> getSuboperationTimeoutInSeconds(Duration headroom) { + return Optional.of((float) (timeBudget.timeBeforeDeadline(headroom).toMillis() / 1000.0)); + } +} 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 095a7da8322..580d34eccf8 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.orchestrator; import com.google.inject.Inject; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.jdisc.Timer; import com.yahoo.log.LogLevel; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference; @@ -43,7 +44,6 @@ import java.util.stream.Collectors; * @author smorgrav */ public class OrchestratorImpl implements Orchestrator { - private static final Logger log = Logger.getLogger(OrchestratorImpl.class.getName()); private final Policy policy; @@ -51,32 +51,36 @@ public class OrchestratorImpl implements Orchestrator { private final InstanceLookupService instanceLookupService; private final int serviceMonitorConvergenceLatencySeconds; private final ClusterControllerClientFactory clusterControllerClientFactory; + private final Timer timer; @Inject public OrchestratorImpl(ClusterControllerClientFactory clusterControllerClientFactory, StatusService statusService, OrchestratorConfig orchestratorConfig, - InstanceLookupService instanceLookupService) + InstanceLookupService instanceLookupService, + Timer timer) { this(new HostedVespaPolicy(new HostedVespaClusterPolicy(), clusterControllerClientFactory), clusterControllerClientFactory, statusService, instanceLookupService, - orchestratorConfig.serviceMonitorConvergenceLatencySeconds()); + orchestratorConfig.serviceMonitorConvergenceLatencySeconds(), + timer); } public OrchestratorImpl(Policy policy, ClusterControllerClientFactory clusterControllerClientFactory, StatusService statusService, InstanceLookupService instanceLookupService, - int serviceMonitorConvergenceLatencySeconds) + int serviceMonitorConvergenceLatencySeconds, + Timer timer) { this.policy = policy; this.clusterControllerClientFactory = clusterControllerClientFactory; this.statusService = statusService; this.serviceMonitorConvergenceLatencySeconds = serviceMonitorConvergenceLatencySeconds; this.instanceLookupService = instanceLookupService; - + this.timer = timer; } @Override @@ -123,7 +127,10 @@ public class OrchestratorImpl implements Orchestrator { ApplicationInstance appInstance = getApplicationInstance(hostName); - try (MutableStatusRegistry statusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(appInstance.reference())) { + OrchestratorContext context = new OrchestratorContext(timer); + try (MutableStatusRegistry statusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly( + appInstance.reference(), + context.getOriginalTimeoutInSeconds())) { final HostStatus currentHostState = statusRegistry.getHostStatus(hostName); if (HostStatus.NO_REMARKS == currentHostState) { @@ -132,7 +139,7 @@ public class OrchestratorImpl implements Orchestrator { ApplicationInstanceStatus appStatus = statusService.forApplicationInstance(appInstance.reference()).getApplicationInstanceStatus(); if (appStatus == ApplicationInstanceStatus.NO_REMARKS) { - policy.releaseSuspensionGrant(appInstance, hostName, statusRegistry); + policy.releaseSuspensionGrant(context, appInstance, hostName, statusRegistry); } } } @@ -149,13 +156,16 @@ public class OrchestratorImpl implements Orchestrator { ApplicationInstance appInstance = getApplicationInstance(hostName); NodeGroup nodeGroup = new NodeGroup(appInstance, hostName); - try (MutableStatusRegistry statusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(appInstance.reference())) { + OrchestratorContext context = new OrchestratorContext(timer); + try (MutableStatusRegistry statusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly( + appInstance.reference(), + context.getOriginalTimeoutInSeconds())) { ApplicationApi applicationApi = new ApplicationApiImpl( nodeGroup, statusRegistry, clusterControllerClientFactory); - policy.acquirePermissionToRemove(applicationApi); + policy.acquirePermissionToRemove(context, applicationApi); } } @@ -164,7 +174,11 @@ public class OrchestratorImpl implements Orchestrator { public void suspendGroup(NodeGroup nodeGroup) throws HostStateChangeDeniedException, HostNameNotFoundException { ApplicationInstanceReference applicationReference = nodeGroup.getApplicationReference(); - try (MutableStatusRegistry hostStatusRegistry = statusService.lockApplicationInstance_forCurrentThreadOnly(applicationReference)) { + OrchestratorContext context = new OrchestratorContext(timer); + try (MutableStatusRegistry hostStatusRegistry = + statusService.lockApplicationInstance_forCurrentThreadOnly( + applicationReference, + context.getOriginalTimeoutInSeconds())) { ApplicationInstanceStatus appStatus = statusService.forApplicationInstance(applicationReference).getApplicationInstanceStatus(); if (appStatus == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) { return; @@ -174,7 +188,7 @@ public class OrchestratorImpl implements Orchestrator { nodeGroup, hostStatusRegistry, clusterControllerClientFactory); - policy.grantSuspensionRequest(applicationApi); + policy.grantSuspensionRequest(context, applicationApi); } } @@ -287,9 +301,12 @@ public class OrchestratorImpl implements Orchestrator { private void setApplicationStatus(ApplicationId appId, ApplicationInstanceStatus status) throws ApplicationStateChangeDeniedException, ApplicationIdNotFoundException{ + OrchestratorContext context = new OrchestratorContext(timer); ApplicationInstanceReference appRef = OrchestratorUtil.toApplicationInstanceReference(appId, instanceLookupService); try (MutableStatusRegistry statusRegistry = - statusService.lockApplicationInstance_forCurrentThreadOnly(appRef)) { + statusService.lockApplicationInstance_forCurrentThreadOnly( + appRef, + context.getOriginalTimeoutInSeconds())) { // Short-circuit if already in wanted state if (status == statusRegistry.getApplicationInstanceStatus()) return; @@ -304,14 +321,15 @@ public class OrchestratorImpl implements Orchestrator { // 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. - setClusterStateInController(application, ClusterControllerNodeState.MAINTENANCE); + setClusterStateInController(context, application, ClusterControllerNodeState.MAINTENANCE); } statusRegistry.setApplicationInstanceStatus(status); } } - private void setClusterStateInController(ApplicationInstance application, + private void setClusterStateInController(OrchestratorContext context, + ApplicationInstance application, ClusterControllerNodeState state) throws ApplicationStateChangeDeniedException, ApplicationIdNotFoundException { // Get all content clusters for this application @@ -329,7 +347,7 @@ public class OrchestratorImpl implements Orchestrator { clusterControllers, clusterId.s()); try { - ClusterControllerStateResponse response = client.setApplicationState(state); + ClusterControllerStateResponse response = client.setApplicationState(context, state); if (!response.wasModified) { String msg = String.format("Fail to set application %s, cluster name %s to cluster state %s due to: %s", application.applicationInstanceId(), clusterId, state, response.reason); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java index 38cabd5d86d..c2559bdd0da 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java @@ -1,6 +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.vespa.orchestrator.controller; +import com.yahoo.vespa.orchestrator.OrchestratorContext; + import java.io.IOException; /** @@ -13,13 +15,13 @@ public interface ClusterControllerClient { * * @throws IOException if there was a problem communicating with the cluster controller */ - ClusterControllerStateResponse setNodeState(int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException; + ClusterControllerStateResponse setNodeState(OrchestratorContext context, int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException; /** * Requests that a cluster controller sets all nodes in the cluster to the requested state. * * @throws IOException if there was a problem communicating with the cluster controller */ - ClusterControllerStateResponse setApplicationState(ClusterControllerNodeState wantedState) throws IOException; + ClusterControllerStateResponse setApplicationState(OrchestratorContext context, ClusterControllerNodeState wantedState) throws IOException; } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java index b70e1a56aea..7d7a9b36ff9 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.orchestrator.controller; import com.yahoo.vespa.jaxrs.client.JaxRsStrategy; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import java.io.IOException; @@ -29,12 +30,19 @@ public class ClusterControllerClientImpl implements ClusterControllerClient{ * @throws IOException if there was a problem communicating with the cluster controller */ @Override - public ClusterControllerStateResponse setNodeState(int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException { + public ClusterControllerStateResponse setNodeState(OrchestratorContext context, + int storageNodeIndex, + ClusterControllerNodeState wantedState) throws IOException { ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, REQUEST_REASON); ClusterControllerStateRequest stateRequest = new ClusterControllerStateRequest(state, ClusterControllerStateRequest.Condition.SAFE); try { - return clusterControllerApi.apply(api -> api.setNodeState(clusterName, storageNodeIndex, stateRequest)); + return clusterControllerApi.apply(api -> api.setNodeState( + clusterName, + storageNodeIndex, + context.getSuboperationTimeoutInSeconds().orElse(null), + stateRequest) + ); } catch (IOException e) { String message = String.format( "Giving up setting %s for storage node with index %d in cluster %s", @@ -52,12 +60,17 @@ public class ClusterControllerClientImpl implements ClusterControllerClient{ * @throws IOException if there was a problem communicating with the cluster controller */ @Override - public ClusterControllerStateResponse setApplicationState(final ClusterControllerNodeState wantedState) throws IOException { + public ClusterControllerStateResponse setApplicationState( + OrchestratorContext context, + final ClusterControllerNodeState wantedState) throws IOException { final ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, REQUEST_REASON); final ClusterControllerStateRequest stateRequest = new ClusterControllerStateRequest(state, ClusterControllerStateRequest.Condition.FORCE); try { - return clusterControllerApi.apply(api -> api.setClusterState(clusterName, stateRequest)); + return clusterControllerApi.apply(api -> api.setClusterState( + clusterName, + context.getSuboperationTimeoutInSeconds().orElse(null), + stateRequest)); } catch (IOException e) { final String message = String.format( "Giving up setting %s for cluster %s", diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java index 7059706c569..4831f4c121a 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java @@ -6,6 +6,7 @@ import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; import javax.ws.rs.core.MediaType; /** @@ -20,6 +21,7 @@ public interface ClusterControllerJaxRsApi { ClusterControllerStateResponse setNodeState( @PathParam("clusterName") String clusterName, @PathParam("storageNodeIndex") int storageNodeIndex, + @QueryParam("timeout") Float timeoutSeconds, ClusterControllerStateRequest request); @POST @@ -28,6 +30,7 @@ public interface ClusterControllerJaxRsApi { @Produces(MediaType.APPLICATION_JSON) ClusterControllerStateResponse setClusterState( @PathParam("clusterName") String clusterName, + @QueryParam("timeout") Float timeoutSeconds, ClusterControllerStateRequest request); } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNode.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNode.java index 210f68e8fda..a54d829a029 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNode.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNode.java @@ -2,10 +2,11 @@ package com.yahoo.vespa.orchestrator.model; import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; public interface StorageNode extends Comparable<StorageNode> { HostName hostName(); - void setNodeState(ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException; + void setNodeState(OrchestratorContext context, ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException; } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java index 109acbc6486..a2732bca88a 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java @@ -7,6 +7,7 @@ import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.ConfigId; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.applicationmodel.ServiceInstance; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; @@ -43,7 +44,7 @@ public class StorageNodeImpl implements StorageNode { } @Override - public void setNodeState(ClusterControllerNodeState wantedNodeState) + public void setNodeState(OrchestratorContext context, ClusterControllerNodeState wantedNodeState) throws HostStateChangeDeniedException { // The "cluster name" used by the Cluster Controller IS the cluster ID. String clusterId = this.clusterId.s(); @@ -66,7 +67,7 @@ public class StorageNodeImpl implements StorageNode { ClusterControllerStateResponse response; try { - response = client.setNodeState(nodeIndex, wantedNodeState); + response = client.setNodeState(context, nodeIndex, wantedNodeState); } catch (IOException e) { throw new HostStateChangeDeniedException( hostName(), 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 8e02f940127..e1664466283 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 @@ -1,10 +1,10 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.policy; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.log.LogLevel; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; import com.yahoo.vespa.orchestrator.model.ApplicationApi; @@ -40,7 +40,7 @@ public class HostedVespaPolicy implements Policy { } @Override - public void grantSuspensionRequest(ApplicationApi application) + public void grantSuspensionRequest(OrchestratorContext context, ApplicationApi application) throws HostStateChangeDeniedException { // Apply per-cluster policy for (ClusterApi cluster : application.getClusters()) { @@ -50,7 +50,7 @@ public class HostedVespaPolicy implements Policy { // Ask Cluster Controller to set UP storage nodes in maintenance. // These storage nodes are guaranteed to be NO_REMARKS for (StorageNode storageNode : application.getUpStorageNodesInGroupInClusterOrder()) { - storageNode.setNodeState(ClusterControllerNodeState.MAINTENANCE); + storageNode.setNodeState(context, ClusterControllerNodeState.MAINTENANCE); log.log(LogLevel.INFO, "The storage node on " + storageNode.hostName() + " has been set to MAINTENANCE"); } @@ -62,10 +62,11 @@ public class HostedVespaPolicy implements Policy { } @Override - public void releaseSuspensionGrant(ApplicationApi application) throws HostStateChangeDeniedException { + 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()) { - storageNode.setNodeState(ClusterControllerNodeState.UP); + storageNode.setNodeState(context, ClusterControllerNodeState.UP); log.log(LogLevel.INFO, "The storage node on " + storageNode.hostName() + " has been set to UP"); } @@ -76,7 +77,8 @@ public class HostedVespaPolicy implements Policy { } @Override - public void acquirePermissionToRemove(ApplicationApi applicationApi) throws HostStateChangeDeniedException { + public void acquirePermissionToRemove(OrchestratorContext context, ApplicationApi applicationApi) + throws HostStateChangeDeniedException { ApplicationInstanceStatus applicationStatus = applicationApi.getApplicationStatus(); if (applicationStatus == ApplicationInstanceStatus.ALLOWED_TO_BE_DOWN) { throw new HostStateChangeDeniedException( @@ -94,7 +96,7 @@ public class HostedVespaPolicy implements Policy { // Ask Cluster Controller to set storage nodes to DOWN. // These storage nodes are guaranteed to be NO_REMARKS for (StorageNode storageNode : applicationApi.getStorageNodesInGroupInClusterOrder()) { - storageNode.setNodeState(ClusterControllerNodeState.DOWN); + storageNode.setNodeState(context, ClusterControllerNodeState.DOWN); log.log(LogLevel.INFO, "The storage node on " + storageNode.hostName() + " has been set DOWN"); } @@ -107,24 +109,14 @@ public class HostedVespaPolicy implements Policy { // TODO: Remove later - currently used for backward compatibility testing @Override - public void grantSuspensionRequest(ApplicationInstance applicationInstance, - HostName hostName, - MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException { - NodeGroup nodeGroup = new NodeGroup(applicationInstance); - nodeGroup.addNode(hostName); - ApplicationApi applicationApi = new ApplicationApiImpl(nodeGroup, hostStatusService, clusterControllerClientFactory); - grantSuspensionRequest(applicationApi); - } - - // TODO: Remove later - currently used for backward compatibility testing - @Override public void releaseSuspensionGrant( + OrchestratorContext context, ApplicationInstance applicationInstance, HostName hostName, MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException { NodeGroup nodeGroup = new NodeGroup(applicationInstance, hostName); ApplicationApi applicationApi = new ApplicationApiImpl(nodeGroup, hostStatusService, clusterControllerClientFactory); - releaseSuspensionGrant(applicationApi); + releaseSuspensionGrant(context, applicationApi); } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java index 4ea4f81182f..9938d244657 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/Policy.java @@ -3,6 +3,7 @@ package com.yahoo.vespa.orchestrator.policy; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.model.ApplicationApi; import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry; @@ -10,30 +11,20 @@ import com.yahoo.vespa.orchestrator.status.MutableStatusRegistry; * @author oyving */ public interface Policy { - - /** - * Decide whether to grant a request for temporarily suspending the services on a host. - * - * @throws HostStateChangeDeniedException if the grant was not given. - */ - void grantSuspensionRequest( - ApplicationInstance applicationInstance, - HostName hostName, - MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException; - /** * Decide whether to grant a request for temporarily suspending the services on all hosts in the group. */ - void grantSuspensionRequest(ApplicationApi applicationApi) throws HostStateChangeDeniedException; + void grantSuspensionRequest(OrchestratorContext context, ApplicationApi applicationApi) throws HostStateChangeDeniedException; - void releaseSuspensionGrant(ApplicationApi application) throws HostStateChangeDeniedException; + void releaseSuspensionGrant(OrchestratorContext context, ApplicationApi application) throws HostStateChangeDeniedException; /** * Give all hosts in a group permission to be removed from the application. * + * @param context * @param applicationApi */ - void acquirePermissionToRemove(ApplicationApi applicationApi) throws HostStateChangeDeniedException; + void acquirePermissionToRemove(OrchestratorContext context, ApplicationApi applicationApi) throws HostStateChangeDeniedException; /** * Release an earlier grant for suspension. @@ -41,7 +32,7 @@ public interface Policy { * @throws HostStateChangeDeniedException if the release failed. */ void releaseSuspensionGrant( - ApplicationInstance applicationInstance, + OrchestratorContext context, ApplicationInstance applicationInstance, HostName hostName, MutableStatusRegistry hostStatusService) throws HostStateChangeDeniedException; } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/InMemoryStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/InMemoryStatusService.java index cab14a2e77f..c5ae553a98c 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/InMemoryStatusService.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/InMemoryStatusService.java @@ -44,6 +44,13 @@ public class InMemoryStatusService implements StatusService { @Override public MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(ApplicationInstanceReference applicationInstanceReference) { + return lockApplicationInstance_forCurrentThreadOnly(applicationInstanceReference, 10); + } + + @Override + public MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly( + ApplicationInstanceReference applicationInstanceReference, + long timeoutSeconds) { Lock lock = instanceLockService.get(applicationInstanceReference); return new InMemoryMutableStatusRegistry(lock, applicationInstanceReference); } 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 cf7b40ce0ef..db58544df55 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 @@ -54,6 +54,11 @@ public interface StatusService { */ MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly(ApplicationInstanceReference applicationInstanceReference); + /** Lock application instance with timeout. */ + MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly( + ApplicationInstanceReference applicationInstanceReference, + long timeoutSeconds); + /** * Returns all application instances that are allowed to be down. The intention is to use this * for visualization, informational and debugging purposes. 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 c84473a5199..18d6ed4b3f2 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 @@ -93,7 +93,8 @@ public class ZookeeperStatusService implements StatusService { } } - MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly( + @Override + public MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly( ApplicationInstanceReference applicationInstanceReference, long timeoutSeconds) { String lockPath = applicationInstanceLock2Path(applicationInstanceReference); diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java index 76d9398c44e..c3b1ceb66ec 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.orchestrator; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.jdisc.Timer; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference; @@ -77,7 +78,8 @@ public class OrchestratorImplTest { clustercontroller, new InMemoryStatusService(), new OrchestratorConfig(new OrchestratorConfig.Builder()), - new DummyInstanceLookupService()); + new DummyInstanceLookupService(), + mock(Timer.class)); clustercontroller.setAllDummyNodesAsUp(); } @@ -307,7 +309,8 @@ public class OrchestratorImplTest { clusterControllerClientFactory, statusService, new OrchestratorConfig(new OrchestratorConfig.Builder()), - lookupService); + lookupService, + mock(Timer.class)); HostName hostName = new HostName("host.yahoo.com"); TenantId tenantId = new TenantId("tenant"); diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java index 230e36469d3..5c5ee7d2260 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java @@ -5,6 +5,7 @@ import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.orchestrator.DummyInstanceLookupService; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.model.VespaModelUtil; import java.io.IOException; @@ -52,13 +53,13 @@ public class ClusterControllerClientFactoryMock implements ClusterControllerClie return new ClusterControllerClient() { @Override - public ClusterControllerStateResponse setNodeState(int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException { + public ClusterControllerStateResponse setNodeState(OrchestratorContext context, int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException { nodes.put(clusterName + storageNodeIndex, wantedState); return new ClusterControllerStateResponse(true, "Yes"); } @Override - public ClusterControllerStateResponse setApplicationState(ClusterControllerNodeState wantedState) throws IOException { + public ClusterControllerStateResponse setApplicationState(OrchestratorContext context, ClusterControllerNodeState wantedState) throws IOException { Set<String> keyCopy = new HashSet<>(nodes.keySet()); for (String s : keyCopy) { if (s.startsWith(clusterName)) { diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java index 09d52326eb8..b3f80b5129f 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java @@ -3,12 +3,16 @@ package com.yahoo.vespa.orchestrator.controller; import com.yahoo.vespa.jaxrs.client.JaxRsStrategy; import com.yahoo.vespa.jaxrs.client.LocalPassThroughJaxRsStrategy; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import org.junit.Test; +import java.util.Optional; + import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class ClusterControllerClientTest { private static final String CLUSTER_NAME = "clusterName"; @@ -24,7 +28,9 @@ public class ClusterControllerClientTest { final ClusterControllerNodeState wantedState = ClusterControllerNodeState.MAINTENANCE; - clusterControllerClient.setNodeState(STORAGE_NODE_INDEX, wantedState); + OrchestratorContext context = mock(OrchestratorContext.class); + when(context.getSuboperationTimeoutInSeconds()).thenReturn(Optional.of(1.0f)); + clusterControllerClient.setNodeState(context, STORAGE_NODE_INDEX, wantedState); final ClusterControllerStateRequest expectedNodeStateRequest = new ClusterControllerStateRequest( new ClusterControllerStateRequest.State(wantedState, ClusterControllerClientImpl.REQUEST_REASON), @@ -33,6 +39,7 @@ public class ClusterControllerClientTest { .setNodeState( eq(CLUSTER_NAME), eq(STORAGE_NODE_INDEX), + eq(1.0f), eq(expectedNodeStateRequest)); } } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java index b26d11a113f..c3dbe5c8a92 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/SingleInstanceClusterControllerClientFactoryTest.java @@ -4,11 +4,13 @@ package com.yahoo.vespa.orchestrator.controller; import com.yahoo.vespa.applicationmodel.ConfigId; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.jaxrs.client.JaxRsClientFactory; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import org.junit.Before; import org.junit.Test; import java.util.Arrays; import java.util.List; +import java.util.Optional; import static org.hamcrest.CoreMatchers.anyOf; import static org.hamcrest.CoreMatchers.equalTo; @@ -31,6 +33,7 @@ public class SingleInstanceClusterControllerClientFactoryTest { private static final HostName HOST_NAME_2 = new HostName("host2"); private static final HostName HOST_NAME_3 = new HostName("host3"); + OrchestratorContext context = mock(OrchestratorContext.class); private final ClusterControllerJaxRsApi mockApi = mock(ClusterControllerJaxRsApi.class); private final JaxRsClientFactory jaxRsClientFactory = mock(JaxRsClientFactory.class); private final ClusterControllerClientFactory clientFactory @@ -64,8 +67,9 @@ public class SingleInstanceClusterControllerClientFactoryTest { public void testCreateClientWithSingleClusterControllerInstance() throws Exception { final List<HostName> clusterControllers = Arrays.asList(HOST_NAME_1); + when(context.getSuboperationTimeoutInSeconds()).thenReturn(Optional.of(1.0f)); clientFactory.createClient(clusterControllers, "clusterName") - .setNodeState(0, ClusterControllerNodeState.MAINTENANCE); + .setNodeState(context, 0, ClusterControllerNodeState.MAINTENANCE); verify(jaxRsClientFactory).createClient( ClusterControllerJaxRsApi.class, @@ -91,8 +95,9 @@ public class SingleInstanceClusterControllerClientFactoryTest { public void testCreateClientWithThreeClusterControllerInstances() throws Exception { final List<HostName> clusterControllers = Arrays.asList(HOST_NAME_1, HOST_NAME_2, HOST_NAME_3); + when(context.getSuboperationTimeoutInSeconds()).thenReturn(Optional.of(1.0f)); clientFactory.createClient(clusterControllers, "clusterName") - .setNodeState(0, ClusterControllerNodeState.MAINTENANCE); + .setNodeState(context, 0, ClusterControllerNodeState.MAINTENANCE); verify(jaxRsClientFactory).createClient( eq(ClusterControllerJaxRsApi.class), 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 220371c4a17..329c9576f2c 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 @@ -5,6 +5,7 @@ package com.yahoo.vespa.orchestrator.policy; import com.yahoo.config.provision.ApplicationId; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.orchestrator.OrchestrationException; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; @@ -70,7 +71,8 @@ public class HostedVespaPolicyTest { InOrder order = inOrder(applicationApi, clusterPolicy, storageNode1, storageNode3); - policy.grantSuspensionRequest(applicationApi); + OrchestratorContext context = mock(OrchestratorContext.class); + policy.grantSuspensionRequest(context, applicationApi); order.verify(applicationApi).getClusters(); order.verify(clusterPolicy).verifyGroupGoingDownIsFine(clusterApi1); @@ -78,8 +80,8 @@ public class HostedVespaPolicyTest { order.verify(clusterPolicy).verifyGroupGoingDownIsFine(clusterApi3); order.verify(applicationApi).getUpStorageNodesInGroupInClusterOrder(); - order.verify(storageNode1).setNodeState(ClusterControllerNodeState.MAINTENANCE); - order.verify(storageNode3).setNodeState(ClusterControllerNodeState.MAINTENANCE); + order.verify(storageNode1).setNodeState(context, ClusterControllerNodeState.MAINTENANCE); + order.verify(storageNode3).setNodeState(context, ClusterControllerNodeState.MAINTENANCE); order.verify(applicationApi).getNodesInGroupWithStatus(HostStatus.NO_REMARKS); order.verify(applicationApi).setHostState(hostName1, HostStatus.ALLOWED_TO_BE_DOWN); @@ -120,7 +122,8 @@ public class HostedVespaPolicyTest { InOrder order = inOrder(applicationApi, clusterPolicy, storageNode1, storageNode3); - policy.acquirePermissionToRemove(applicationApi); + OrchestratorContext context = mock(OrchestratorContext.class); + policy.acquirePermissionToRemove(context, applicationApi); order.verify(applicationApi).getClusters(); order.verify(clusterPolicy).verifyGroupGoingDownPermanentlyIsFine(clusterApi1); @@ -128,8 +131,8 @@ public class HostedVespaPolicyTest { order.verify(clusterPolicy).verifyGroupGoingDownPermanentlyIsFine(clusterApi3); order.verify(applicationApi).getStorageNodesInGroupInClusterOrder(); - order.verify(storageNode1).setNodeState(ClusterControllerNodeState.DOWN); - order.verify(storageNode3).setNodeState(ClusterControllerNodeState.DOWN); + order.verify(storageNode1).setNodeState(context, ClusterControllerNodeState.DOWN); + order.verify(storageNode3).setNodeState(context, ClusterControllerNodeState.DOWN); order.verify(applicationApi).getNodesInGroupWithStatus(HostStatus.NO_REMARKS); order.verify(applicationApi).setHostState(hostName1, HostStatus.ALLOWED_TO_BE_DOWN); diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java index 2c7db25ae30..49f1a33febb 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.resources; +import com.yahoo.jdisc.Timer; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference; @@ -16,6 +17,7 @@ import com.yahoo.vespa.orchestrator.Host; import com.yahoo.vespa.orchestrator.InstanceLookupService; import com.yahoo.vespa.orchestrator.OrchestrationException; import com.yahoo.vespa.orchestrator.Orchestrator; +import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.OrchestratorImpl; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactoryMock; import com.yahoo.vespa.orchestrator.model.ApplicationApi; @@ -59,7 +61,7 @@ import static org.mockito.Mockito.when; * @author hakonhall */ public class HostResourceTest { - + private static final Timer timer = mock(Timer.class); private static final int SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS = 0; private static final TenantId TENANT_ID = new TenantId("tenantId"); private static final ApplicationInstanceId APPLICATION_INSTANCE_ID = new ApplicationInstanceId("applicationId"); @@ -110,28 +112,20 @@ public class HostResourceTest { private static class AlwaysAllowPolicy implements Policy { @Override - public void grantSuspensionRequest( - ApplicationInstance applicationInstance, - HostName hostName, - MutableStatusRegistry hostStatusService) { - - } - - @Override - public void grantSuspensionRequest(ApplicationApi applicationApi) { + public void grantSuspensionRequest(OrchestratorContext context, ApplicationApi applicationApi) { } @Override - public void releaseSuspensionGrant(ApplicationApi application) { + public void releaseSuspensionGrant(OrchestratorContext context, ApplicationApi application) { } @Override - public void acquirePermissionToRemove(ApplicationApi applicationApi) { + public void acquirePermissionToRemove(OrchestratorContext context, ApplicationApi applicationApi) { } @Override public void releaseSuspensionGrant( - ApplicationInstance applicationInstance, + OrchestratorContext context, ApplicationInstance applicationInstance, HostName hostName, MutableStatusRegistry hostStatusRegistry) { } @@ -141,14 +135,16 @@ public class HostResourceTest { new AlwaysAllowPolicy(), new ClusterControllerClientFactoryMock(), EVERY_HOST_IS_UP_HOST_STATUS_SERVICE, mockInstanceLookupService, - SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS + SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS, + timer ); private static final OrchestratorImpl hostNotFoundOrchestrator = new OrchestratorImpl( new AlwaysAllowPolicy(), new ClusterControllerClientFactoryMock(), EVERY_HOST_IS_UP_HOST_STATUS_SERVICE, alwaysEmptyInstanceLookUpService, - SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS + SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS, + timer ); private final UriInfo uriInfo = mock(UriInfo.class); @@ -209,31 +205,23 @@ public class HostResourceTest { private static class AlwaysFailPolicy implements Policy { @Override - public void grantSuspensionRequest( - ApplicationInstance applicationInstance, - HostName hostName, - MutableStatusRegistry hostStatusRegistry) throws HostStateChangeDeniedException { - doThrow(); - } - - @Override - public void grantSuspensionRequest(ApplicationApi applicationApi) throws HostStateChangeDeniedException { + public void grantSuspensionRequest(OrchestratorContext context, ApplicationApi applicationApi) throws HostStateChangeDeniedException { doThrow(); } @Override - public void releaseSuspensionGrant(ApplicationApi application) throws HostStateChangeDeniedException { + public void releaseSuspensionGrant(OrchestratorContext context, ApplicationApi application) throws HostStateChangeDeniedException { doThrow(); } @Override - public void acquirePermissionToRemove(ApplicationApi applicationApi) throws HostStateChangeDeniedException { + public void acquirePermissionToRemove(OrchestratorContext context, ApplicationApi applicationApi) throws HostStateChangeDeniedException { doThrow(); } @Override public void releaseSuspensionGrant( - ApplicationInstance applicationInstance, + OrchestratorContext context, ApplicationInstance applicationInstance, HostName hostName, MutableStatusRegistry hostStatusRegistry) throws HostStateChangeDeniedException { doThrow(); @@ -253,7 +241,8 @@ public class HostResourceTest { new AlwaysFailPolicy(), new ClusterControllerClientFactoryMock(), EVERY_HOST_IS_UP_HOST_STATUS_SERVICE,mockInstanceLookupService, - SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS); + SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS, + timer); try { HostResource hostResource = new HostResource(alwaysRejectResolver, uriInfo); @@ -271,7 +260,8 @@ public class HostResourceTest { new ClusterControllerClientFactoryMock(), EVERY_HOST_IS_UP_HOST_STATUS_SERVICE, mockInstanceLookupService, - SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS); + SERVICE_MONITOR_CONVERGENCE_LATENCY_SECONDS, + timer); try { HostSuspensionResource hostSuspensionResource = new HostSuspensionResource(alwaysRejectResolver); |