Return 500 when a request is canceled due to an exceeded server deadline
Server side deadlines define the maximum time that a request is allowed
to take. If a request takes longer it's an error and hence we should
return 500 ISE. Returning 500 ISE, instead of 408 Request Timeout, means
that the cancelled requests are counted against the SLO budget now.
Release-Notes: Return 500 when a request is canceled due to an exceeded server deadline
Change-Id: Id68a288328e9d6752ae10c88fe0c9d22b36be8b0
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 9056732..0ace77e 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1869,8 +1869,9 @@
case CLIENT_CLOSED_REQUEST:
return SC_CLIENT_CLOSED_REQUEST;
case CLIENT_PROVIDED_DEADLINE_EXCEEDED:
- case SERVER_DEADLINE_EXCEEDED:
return SC_REQUEST_TIMEOUT;
+ case SERVER_DEADLINE_EXCEEDED:
+ return SC_INTERNAL_SERVER_ERROR;
}
logger.atSevere().log("Unexpected cancellation reason: %s", cancellationReason);
return SC_INTERNAL_SERVER_ERROR;
diff --git a/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java b/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
index c868d0b..2cc4857 100644
--- a/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.httpd.restapi.RestApiServlet.SC_CLIENT_CLOSED_REQUEST;
+import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
import static org.apache.http.HttpStatus.SC_REQUEST_TIMEOUT;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -100,7 +101,7 @@
try (Registration registration =
extensionRegistry.newRegistration().add(projectCreationListener)) {
RestResponse response = adminRestSession.put("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent()).isEqualTo("Server Deadline Exceeded");
}
}
@@ -119,7 +120,7 @@
try (Registration registration =
extensionRegistry.newRegistration().add(projectCreationListener)) {
RestResponse response = adminRestSession.put("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndeadline = 10m");
}
@@ -140,7 +141,7 @@
try (Registration registration =
extensionRegistry.newRegistration().add(projectCreationListener)) {
RestResponse response = adminRestSession.put("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndeadline = 10m");
}
@@ -197,7 +198,7 @@
public void abortIfServerDeadlineExceeded() throws Exception {
testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent()).isEqualTo("Server Deadline Exceeded\n\ntimeout=1ms");
}
@@ -207,7 +208,7 @@
public void stricterDeadlineTakesPrecedence() throws Exception {
testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\nfoo.timeout=1ms");
}
@@ -218,7 +219,7 @@
public void abortIfServerDeadlineExceeded_requestType() throws Exception {
testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndefault.timeout=1ms");
}
@@ -229,7 +230,7 @@
public void abortIfServerDeadlineExceeded_requestUriPattern() throws Exception {
testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndefault.timeout=1ms");
}
@@ -242,7 +243,7 @@
public void abortIfServerDeadlineExceeded_excludedRequestUriPattern() throws Exception {
testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndefault.timeout=1ms");
}
@@ -257,7 +258,7 @@
throws Exception {
testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndefault.timeout=1ms");
}
@@ -268,7 +269,7 @@
public void abortIfServerDeadlineExceeded_projectPattern() throws Exception {
testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndefault.timeout=1ms");
}
@@ -279,7 +280,7 @@
public void abortIfServerDeadlineExceeded_account() throws Exception {
testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(2));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndefault.timeout=1ms");
}
@@ -356,7 +357,7 @@
public void nonAdvisoryDeadlineIsAppliedIfStricterAdvisoryDeadlineExists() throws Exception {
testTicker.useFakeTicker().setAutoIncrementStep(Duration.ofMillis(4));
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndefault.timeout=2ms");
}
@@ -462,7 +463,7 @@
try (Registration registration =
extensionRegistry.newRegistration().add(projectCreationValidationListener)) {
RestResponse response = adminRestSession.putWithHeaders("/projects/" + name("new"));
- assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getEntityContent())
.isEqualTo("Server Deadline Exceeded\n\ndefault.timeout=500ms");
}
