diff options
7 files changed, 28 insertions, 13 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java index dea0b8c19d0..acf742842c8 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/RetiredExpirer.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.hosted.provision.maintenance; +import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Deployer; import com.yahoo.config.provision.Deployment; @@ -117,6 +118,9 @@ public class RetiredExpirer extends Maintainer { orchestrator.acquirePermissionToRemove(new HostName(node.hostname())); log.info("Node " + node + " has been granted permission to be removed"); return true; + } catch (UncheckedTimeoutException e) { + log.info("Timed out trying to aquire permission to remove " + node.hostname() + ": " + e.getMessage()); + return false; } catch (OrchestrationException e) { log.info("Did not get permission to remove retired " + node + ": " + e.getMessage()); return false; diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java index cc8a401ed78..4bb93ffa3cb 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostResource.java @@ -152,29 +152,34 @@ public class HostResource implements HostApi { return new UpdateHostResponse(hostName.s(), null); } - private static WebApplicationException webExceptionFromTimeout(String operationDescription, HostName hostName, UncheckedTimeoutException e) { - return createWebException(operationDescription, hostName, e, HostedVespaPolicy.DEADLINE_CONSTRAINT, e.getMessage()); + private static WebApplicationException webExceptionFromTimeout(String operationDescription, + HostName hostName, + UncheckedTimeoutException e) { + return createWebException(operationDescription, hostName, e, HostedVespaPolicy.DEADLINE_CONSTRAINT, e.getMessage(), + Response.Status.GATEWAY_TIMEOUT); } private static WebApplicationException webExceptionWithDenialReason( String operationDescription, HostName hostName, HostStateChangeDeniedException e) { - return createWebException(operationDescription, hostName, e, e.getConstraintName(), e.getMessage()); + return createWebException(operationDescription, hostName, e, e.getConstraintName(), e.getMessage(), + Response.Status.CONFLICT); } private static WebApplicationException createWebException(String operationDescription, HostName hostname, Exception e, String constraint, - String message) { + String message, + Response.Status status) { HostStateChangeDenialReason hostStateChangeDenialReason = new HostStateChangeDenialReason( constraint, operationDescription + " failed: " + message); UpdateHostResponse response = new UpdateHostResponse(hostname.s(), hostStateChangeDenialReason); return new WebApplicationException( hostStateChangeDenialReason.toString(), e, - Response.status(Response.Status.CONFLICT) + Response.status(status) .entity(response) .type(MediaType.APPLICATION_JSON_TYPE) .build()); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java index 24d42264a51..79e0fc0f3e9 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/resources/HostSuspensionResource.java @@ -18,6 +18,7 @@ import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import java.util.List; +import java.util.concurrent.TimeoutException; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -42,9 +43,12 @@ public class HostSuspensionResource implements HostSuspensionApi { List<HostName> hostnames = hostnamesAsStrings.stream().map(HostName::new).collect(Collectors.toList()); try { orchestrator.suspendAll(parentHostname, hostnames); - } catch (BatchHostStateChangeDeniedException | UncheckedTimeoutException e) { + } catch (BatchHostStateChangeDeniedException e) { log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e); throw createWebApplicationException(e.getMessage(), Response.Status.CONFLICT); + } catch (UncheckedTimeoutException e) { + log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e); + throw createWebApplicationException(e.getMessage(), Response.Status.GATEWAY_TIMEOUT); } catch (BatchHostNameNotFoundException e) { log.log(LogLevel.DEBUG, "Failed to suspend nodes " + hostnames + " with parent host " + parentHostname, e); // Note that we're returning BAD_REQUEST instead of NOT_FOUND because the resource identified 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 9f91e08d344..993cddae2b3 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 @@ -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.status; +import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference; import com.yahoo.vespa.applicationmodel.HostName; import com.yahoo.vespa.orchestrator.OrchestratorContext; @@ -45,7 +46,7 @@ public interface StatusService { */ MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly( OrchestratorContext context, - ApplicationInstanceReference applicationInstanceReference); + ApplicationInstanceReference applicationInstanceReference) throws UncheckedTimeoutException; /** * Returns all application instances that are allowed to be down. The intention is to use this 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 f6e01a49ce8..e3d2a0827ed 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 @@ -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.status; +import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.container.jaxrs.annotation.Component; import com.yahoo.log.LogLevel; import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference; @@ -102,7 +103,7 @@ public class ZookeeperStatusService implements StatusService { @Override public MutableStatusRegistry lockApplicationInstance_forCurrentThreadOnly( OrchestratorContext context, - ApplicationInstanceReference applicationInstanceReference) { + ApplicationInstanceReference applicationInstanceReference) throws UncheckedTimeoutException { Duration duration = context.getTimeLeft(); String lockPath = applicationInstanceLock2Path(applicationInstanceReference); Lock lock = new Lock(lockPath, curator); 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 55ac65c036c..2e97b3b7242 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 @@ -352,7 +352,7 @@ public class HostResourceTest { } @Test - public void throws_409_on_timeout() throws HostNameNotFoundException, HostStateChangeDeniedException { + public void throws_504_on_timeout() throws HostNameNotFoundException, HostStateChangeDeniedException { Orchestrator orchestrator = mock(Orchestrator.class); doThrow(new UncheckedTimeoutException("Timeout Message")).when(orchestrator).resume(any(HostName.class)); @@ -361,13 +361,13 @@ public class HostResourceTest { hostResource.resume("hostname"); fail(); } catch (WebApplicationException w) { - assertThat(w.getResponse().getStatus()).isEqualTo(409); + assertThat(w.getResponse().getStatus()).isEqualTo(504); assertEquals("resume failed: Timeout Message [deadline]", w.getMessage()); } } @Test - public void throws_409_on_suspendAll_timeout() throws BatchHostStateChangeDeniedException, BatchHostNameNotFoundException, BatchInternalErrorException { + public void throws_504_on_suspendAll_timeout() throws BatchHostStateChangeDeniedException, BatchHostNameNotFoundException, BatchInternalErrorException { Orchestrator orchestrator = mock(Orchestrator.class); doThrow(new UncheckedTimeoutException("Timeout Message")).when(orchestrator).suspendAll(any(), any()); @@ -376,7 +376,7 @@ public class HostResourceTest { resource.suspendAll("parenthost", Arrays.asList("h1", "h2", "h3")); fail(); } catch (WebApplicationException w) { - assertThat(w.getResponse().getStatus()).isEqualTo(409); + assertThat(w.getResponse().getStatus()).isEqualTo(504); } } } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java index 23fa3cccad2..30af89d0ea8 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java @@ -24,7 +24,7 @@ public class Lock implements Mutex { } /** Take the lock with the given timeout. This may be called multiple times from the same thread - each matched by a close */ - public void acquire(Duration timeout) { + public void acquire(Duration timeout) throws UncheckedTimeoutException { boolean acquired; try { acquired = mutex.acquire(timeout.toMillis(), TimeUnit.MILLISECONDS); |