From 13f21d33aa4ceaafee665d8cde1b99b17bd63eb1 Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Sun, 23 Feb 2020 17:01:47 +0100 Subject: Return 409 on Orchestrator timeout instead of 504 --- .../java/com/yahoo/vespa/orchestrator/resources/HostResource.java | 5 +++-- .../vespa/orchestrator/resources/HostSuspensionResource.java | 3 +-- .../com/yahoo/vespa/orchestrator/resources/HostResourceTest.java | 8 ++++---- 3 files changed, 8 insertions(+), 8 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 fc5c5eb5004..c93bb9e1d4a 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 @@ -157,8 +157,9 @@ public class HostResource implements HostApi { private static WebApplicationException webExceptionFromTimeout(String operationDescription, HostName hostName, UncheckedTimeoutException e) { - return createWebException(operationDescription, hostName, e, HostedVespaPolicy.DEADLINE_CONSTRAINT, e.getMessage(), - Response.Status.GATEWAY_TIMEOUT); + // Return timeouts as 409 Conflict instead of 504 Gateway Timeout to reduce noise in 5xx graphs. + return createWebException(operationDescription, hostName, e, + HostedVespaPolicy.DEADLINE_CONSTRAINT, e.getMessage(), Response.Status.CONFLICT); } private static WebApplicationException webExceptionWithDenialReason( 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 79e0fc0f3e9..6e857563f9b 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,7 +18,6 @@ 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; @@ -48,7 +47,7 @@ public class HostSuspensionResource implements HostSuspensionApi { 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); + throw createWebApplicationException(e.getMessage(), Response.Status.CONFLICT); } 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/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/resources/HostResourceTest.java index dc26c1a3770..ff7413cd3bb 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 @@ -368,7 +368,7 @@ public class HostResourceTest { } @Test - public void throws_504_on_timeout() throws HostNameNotFoundException, HostStateChangeDeniedException { + public void throws_409_on_timeout() throws HostNameNotFoundException, HostStateChangeDeniedException { Orchestrator orchestrator = mock(Orchestrator.class); doThrow(new UncheckedTimeoutException("Timeout Message")).when(orchestrator).resume(any(HostName.class)); @@ -377,13 +377,13 @@ public class HostResourceTest { hostResource.resume("hostname"); fail(); } catch (WebApplicationException w) { - assertThat(w.getResponse().getStatus()).isEqualTo(504); + assertThat(w.getResponse().getStatus()).isEqualTo(409); assertEquals("resume failed: Timeout Message [deadline]", w.getMessage()); } } @Test - public void throws_504_on_suspendAll_timeout() throws BatchHostStateChangeDeniedException, BatchHostNameNotFoundException, BatchInternalErrorException { + public void throws_409_on_suspendAll_timeout() throws BatchHostStateChangeDeniedException, BatchHostNameNotFoundException, BatchInternalErrorException { Orchestrator orchestrator = mock(Orchestrator.class); doThrow(new UncheckedTimeoutException("Timeout Message")).when(orchestrator).suspendAll(any(), any()); @@ -392,7 +392,7 @@ public class HostResourceTest { resource.suspendAll("parenthost", Arrays.asList("h1", "h2", "h3")); fail(); } catch (WebApplicationException w) { - assertThat(w.getResponse().getStatus()).isEqualTo(504); + assertThat(w.getResponse().getStatus()).isEqualTo(409); } } } -- cgit v1.2.3