[ML] Return 408 when start deployment api times out (elastic#89612)

This commit changes the status code returned when the start trained model deployment api times out from `500` to `408`. In addition, we add validation that the timeout must be positive. Relates elastic#89585
AbeleMM · Aug 25, 2022 · 3fca120 · 3fca120
1 parent 6616746
commit 3fca120
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 2 deletions.
diff --git a/docs/changelog/89612.yaml b/docs/changelog/89612.yaml
@@ -0,0 +1,5 @@
+pr: 89612
+summary: Return 408 when start deployment api times out
+area: Machine Learning
+type: enhancement
+issues: []
diff --git a/...c/main/java/org/elasticsearch/xpack/core/ml/action/StartTrainedModelDeploymentAction.java b/...c/main/java/org/elasticsearch/xpack/core/ml/action/StartTrainedModelDeploymentAction.java
@@ -255,6 +255,9 @@ public ActionRequestValidationException validate() {
             if (queueCapacity > MAX_QUEUE_CAPACITY) {
                 validationException.addValidationError("[" + QUEUE_CAPACITY + "] must be less than " + MAX_QUEUE_CAPACITY);
             }
+            if (timeout.nanos() < 1L) {
+                validationException.addValidationError("[" + TIMEOUT + "] must be positive");
+            }
             return validationException.validationErrors().isEmpty() ? null : validationException;
         }
 

diff --git a/.../java/org/elasticsearch/xpack/core/ml/action/StartTrainedModelDeploymentRequestTests.java b/.../java/org/elasticsearch/xpack/core/ml/action/StartTrainedModelDeploymentRequestTests.java
@@ -47,7 +47,7 @@ protected Request createTestInstance() {
     public static Request createRandom() {
         Request request = new Request(randomAlphaOfLength(10));
         if (randomBoolean()) {
-            request.setTimeout(TimeValue.parseTimeValue(randomTimeValue(), Request.TIMEOUT.getPreferredName()));
+            request.setTimeout(TimeValue.parseTimeValue(randomPositiveTimeValue(), Request.TIMEOUT.getPreferredName()));
         }
         if (randomBoolean()) {
             request.setWaitForState(randomFrom(AllocationStatus.State.values()));
@@ -169,6 +169,26 @@ public void testValidate_GivenQueueCapacityIsOverLimit() {
         assertThat(e.getMessage(), containsString("[queue_capacity] must be less than 1000000"));
     }
 
+    public void testValidate_GivenTimeoutIsNegative() {
+        Request request = createRandom();
+        request.setTimeout(TimeValue.parseTimeValue("-1s", "timeout"));
+
+        ActionRequestValidationException e = request.validate();
+
+        assertThat(e, is(not(nullValue())));
+        assertThat(e.getMessage(), containsString("[timeout] must be positive"));
+    }
+
+    public void testValidate_GivenTimeoutIsZero() {
+        Request request = createRandom();
+        request.setTimeout(TimeValue.parseTimeValue("0s", "timeout"));
+
+        ActionRequestValidationException e = request.validate();
+
+        assertThat(e, is(not(nullValue())));
+        assertThat(e.getMessage(), containsString("[timeout] must be positive"));
+    }
+
     public void testDefaults() {
         Request request = new Request(randomAlphaOfLength(10));
         assertThat(request.getTimeout(), equalTo(TimeValue.timeValueSeconds(20)));

diff --git a/...n/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentService.java b/...n/java/org/elasticsearch/xpack/ml/inference/assignment/TrainedModelAssignmentService.java
@@ -9,6 +9,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.elasticsearch.ElasticsearchStatusException;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionType;
@@ -25,6 +26,7 @@
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.node.NodeClosedException;
+import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.ConnectTransportException;
 import org.elasticsearch.xpack.core.ml.action.CreateTrainedModelAssignmentAction;
@@ -133,7 +135,7 @@ public void onTimeout(TimeValue timeout) {
 
     public interface WaitForAssignmentListener extends ActionListener<TrainedModelAssignment> {
         default void onTimeout(TimeValue timeout) {
-            onFailure(new IllegalStateException("Timed out when waiting for trained model assignment after " + timeout));
+            onFailure(new ElasticsearchStatusException("Starting deployment timed out after [{}]", RestStatus.REQUEST_TIMEOUT, timeout));
         }
     }