Merge pull request #12313 from vespa-engine/hakonhall/return-409-on-orchestrator-timeout-instead-of-504

Return 409 on Orchestrator timeout instead of 504

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);
         }
     }
 }