Browse Source

`GetMlAutoscalingStats$Request` is not acknowledged (#108575)

This request is using `AcknowledgedRequest` simply as a
`MasterNodeRequest` which carries an extra timeout, but the timeout is
not used for tracking acks of a cluster state update so we shouldn't be
using `ackTimeout()` here. This commit changes it into a standard
`MasterNodeRequest` plus an extra timeout, without changing its
transport-protocol representation.
David Turner 1 year ago
parent
commit
437acfaed1

+ 30 - 5
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetMlAutoscalingStats.java

@@ -7,9 +7,10 @@
 
 package org.elasticsearch.xpack.core.ml.action;
 
+import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.action.ActionType;
-import org.elasticsearch.action.support.master.AcknowledgedRequest;
+import org.elasticsearch.action.support.master.MasterNodeRequest;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.core.TimeValue;
@@ -35,14 +36,33 @@ public class GetMlAutoscalingStats extends ActionType<Response> {
         super(NAME);
     }
 
-    public static class Request extends AcknowledgedRequest<Request> {
+    public static class Request extends MasterNodeRequest<Request> {
 
+        private final TimeValue requestTimeout;
+
+        public Request(TimeValue masterNodeTimeout, TimeValue requestTimeout) {
+            super(masterNodeTimeout);
+            this.requestTimeout = Objects.requireNonNull(requestTimeout);
+        }
+
+        @Deprecated(forRemoval = true) // temporary compatibility shi
         public Request(TimeValue timeout) {
-            super(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT, timeout);
+            this(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT, timeout);
         }
 
         public Request(StreamInput in) throws IOException {
             super(in);
+            this.requestTimeout = in.readTimeValue();
+        }
+
+        @Override
+        public void writeTo(StreamOutput out) throws IOException {
+            super.writeTo(out);
+            out.writeTimeValue(this.requestTimeout);
+        }
+
+        public TimeValue requestTimeout() {
+            return requestTimeout;
         }
 
         @Override
@@ -50,9 +70,14 @@ public class GetMlAutoscalingStats extends ActionType<Response> {
             return new CancellableTask(id, type, action, "get_ml_autoscaling_resources", parentTaskId, headers);
         }
 
+        @Override
+        public ActionRequestValidationException validate() {
+            return null;
+        }
+
         @Override
         public int hashCode() {
-            return Objects.hash(ackTimeout());
+            return Objects.hash(requestTimeout);
         }
 
         @Override
@@ -64,7 +89,7 @@ public class GetMlAutoscalingStats extends ActionType<Response> {
                 return false;
             }
             GetMlAutoscalingStats.Request other = (GetMlAutoscalingStats.Request) obj;
-            return Objects.equals(ackTimeout(), other.ackTimeout());
+            return Objects.equals(requestTimeout, other.requestTimeout);
         }
     }
 

+ 5 - 2
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetMlAutoscalingStatsRequestTests.java

@@ -23,11 +23,14 @@ public class GetMlAutoscalingStatsRequestTests extends AbstractWireSerializingTe
 
     @Override
     protected Request createTestInstance() {
-        return new Request(randomTimeValue(0, 10_000));
+        return new Request(TimeValue.THIRTY_SECONDS, randomTimeValue(0, 10_000));
     }
 
     @Override
     protected Request mutateInstance(Request instance) throws IOException {
-        return new Request(TimeValue.timeValueMillis(instance.ackTimeout().millis() + randomIntBetween(1, 1000)));
+        return new Request(
+            TimeValue.THIRTY_SECONDS,
+            TimeValue.timeValueMillis(instance.requestTimeout().millis() + randomIntBetween(1, 1000))
+        );
     }
 }