diff options
author | Håkon Hallingstad <hakon@verizonmedia.com> | 2019-10-21 11:49:42 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@verizonmedia.com> | 2019-10-21 11:49:42 +0200 |
commit | cab9d5745bcfbae60d99fe438c0ed3c28c49204d (patch) | |
tree | 8d616e4e3853fcdc9d321fd1e26c1a45796f1465 /orchestrator | |
parent | 2d045b479b68ce10db396e66de10823e867a9825 (diff) |
Return 504 Gateway Timeout on lock timeout from Orchestrator
Diffstat (limited to 'orchestrator')
5 files changed, 23 insertions, 12 deletions
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); } } } |