Clean up timeouts in reserved cluster state handlers (#112820)

DaveCTurner · web-flow · commit 46dd1a5a9bdc · 2024-09-13T03:40:54.000+10:00
In #109828 we introduced a `DUMMY_TIMEOUT` constant for these handlers in which timeouts are ignored, but we have not been very consistent with its usage and it has an overly-generic name making it harder for readers to understand its meaning. This commit renames the constant to clarify why it's being used, and fixes up several spots where we should have been using it already.
diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryAction.java
@@ -82,7 +82,7 @@ public TransformState transform(Object source, TransformState prevState) throws
         toDelete.removeAll(entities);
 
         for (var repositoryToDelete : toDelete) {
-            var task = new RepositoriesService.UnregisterRepositoryTask(DUMMY_TIMEOUT, repositoryToDelete);
+            var task = new RepositoriesService.UnregisterRepositoryTask(RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT, repositoryToDelete);
             state = task.execute(state);
         }
 
@@ -97,7 +97,11 @@ public List<PutRepositoryRequest> fromXContent(XContentParser parser) throws IOE
         Map<String, ?> source = parser.map();
 
         for (var entry : source.entrySet()) {
-            PutRepositoryRequest putRepositoryRequest = new PutRepositoryRequest(DUMMY_TIMEOUT, DUMMY_TIMEOUT, entry.getKey());
+            PutRepositoryRequest putRepositoryRequest = new PutRepositoryRequest(
+                RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                entry.getKey()
+            );
             @SuppressWarnings("unchecked")
             Map<String, ?> content = (Map<String, ?>) entry.getValue();
             try (XContentParser repoParser = mapToXContentParser(XContentParserConfiguration.EMPTY, content)) {
diff --git a/server/src/main/java/org/elasticsearch/reservedstate/ReservedClusterStateHandler.java b/server/src/main/java/org/elasticsearch/reservedstate/ReservedClusterStateHandler.java
@@ -126,5 +126,5 @@ default void validate(MasterNodeRequest<?> request) {
     /**
      * Reserved-state handlers create master-node requests but never actually send them to the master node so the timeouts are not relevant.
      */
-    TimeValue DUMMY_TIMEOUT = TimeValue.THIRTY_SECONDS;
+    TimeValue RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT = TimeValue.THIRTY_SECONDS;
 }
diff --git a/server/src/main/java/org/elasticsearch/reservedstate/action/ReservedClusterSettingsAction.java b/server/src/main/java/org/elasticsearch/reservedstate/action/ReservedClusterSettingsAction.java
@@ -14,7 +14,6 @@
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.settings.SettingsUpdater;
-import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.reservedstate.ReservedClusterStateHandler;
 import org.elasticsearch.reservedstate.TransformState;
 import org.elasticsearch.xcontent.XContentParser;
@@ -60,9 +59,8 @@ private static ClusterUpdateSettingsRequest prepare(Object input, Set<String> pr
         toDelete.forEach(k -> newSettings.put(k, (String) null));
 
         final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = new ClusterUpdateSettingsRequest(
-            // timeouts are unused, any value will do
-            TimeValue.THIRTY_SECONDS,
-            TimeValue.THIRTY_SECONDS
+            RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+            RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT
         );
         clusterUpdateSettingsRequest.persistentSettings(newSettings);
         return clusterUpdateSettingsRequest;
diff --git a/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/action/ReservedAutoscalingPolicyAction.java b/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/action/ReservedAutoscalingPolicyAction.java
@@ -8,7 +8,6 @@
 package org.elasticsearch.xpack.autoscaling.action;
 
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.reservedstate.ReservedClusterStateHandler;
 import org.elasticsearch.reservedstate.TransformState;
 import org.elasticsearch.xcontent.XContentParser;
@@ -96,8 +95,8 @@ public List<PutAutoscalingPolicyAction.Request> fromXContent(XContentParser pars
                     PutAutoscalingPolicyAction.Request.parse(
                         policyParser,
                         (roles, deciders) -> new PutAutoscalingPolicyAction.Request(
-                            TimeValue.MINUS_ONE,
-                            TimeValue.MINUS_ONE,
+                            RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                            RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
                             name,
                             roles,
                             deciders
diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/ReservedLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/ReservedLifecycleAction.java
@@ -10,7 +10,6 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.reservedstate.ReservedClusterStateHandler;
 import org.elasticsearch.reservedstate.TransformState;
@@ -64,8 +63,11 @@ public Collection<PutLifecycleRequest> prepare(Object input) throws IOException
         List<LifecyclePolicy> policies = (List<LifecyclePolicy>) input;
 
         for (var policy : policies) {
-            // timeouts don't matter here
-            PutLifecycleRequest request = new PutLifecycleRequest(TimeValue.THIRTY_SECONDS, TimeValue.THIRTY_SECONDS, policy);
+            PutLifecycleRequest request = new PutLifecycleRequest(
+                RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                policy
+            );
             validate(request);
             result.add(request);
         }
@@ -97,8 +99,11 @@ public TransformState transform(Object source, TransformState prevState) throws
 
         for (var policyToDelete : toDelete) {
             TransportDeleteLifecycleAction.DeleteLifecyclePolicyTask task = new TransportDeleteLifecycleAction.DeleteLifecyclePolicyTask(
-                // timeouts don't matter here
-                new DeleteLifecycleAction.Request(TimeValue.THIRTY_SECONDS, TimeValue.THIRTY_SECONDS, policyToDelete),
+                new DeleteLifecycleAction.Request(
+                    RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                    RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                    policyToDelete
+                ),
                 ActionListener.noop()
             );
             state = task.execute(state);
diff --git a/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/action/ReservedSnapshotAction.java b/x-pack/plugin/slm/src/main/java/org/elasticsearch/xpack/slm/action/ReservedSnapshotAction.java
@@ -9,7 +9,6 @@
 
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.features.FeatureService;
 import org.elasticsearch.reservedstate.ReservedClusterStateHandler;
 import org.elasticsearch.reservedstate.TransformState;
@@ -59,10 +58,9 @@ private Collection<PutSnapshotLifecycleAction.Request> prepare(List<SnapshotLife
         List<Exception> exceptions = new ArrayList<>();
 
         for (var policy : policies) {
-            // timeouts don't matter here
             PutSnapshotLifecycleAction.Request request = new PutSnapshotLifecycleAction.Request(
-                TimeValue.THIRTY_SECONDS,
-                TimeValue.THIRTY_SECONDS,
+                RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
                 policy.getId(),
                 policy
             );
@@ -106,9 +104,12 @@ public TransformState transform(Object source, TransformState prevState) throws
         toDelete.removeAll(entities);
 
         for (var policyToDelete : toDelete) {
-            // timeouts don't matter here
             var task = new TransportDeleteSnapshotLifecycleAction.DeleteSnapshotPolicyTask(
-                new DeleteSnapshotLifecycleAction.Request(TimeValue.THIRTY_SECONDS, TimeValue.THIRTY_SECONDS, policyToDelete),
+                new DeleteSnapshotLifecycleAction.Request(
+                    RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                    RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT,
+                    policyToDelete
+                ),
                 ActionListener.noop()
             );
             state = task.execute(state);

Original file line number	Diff line number	Diff line change
`@@ -126,5 +126,5 @@ default void validate(MasterNodeRequest<?> request) {`
`126`	`126`	`/**`
`127`	`127`	`* Reserved-state handlers create master-node requests but never actually send them to the master node so the timeouts are not relevant.`
`128`	`128`	`*/`
`129`		`- TimeValue DUMMY_TIMEOUT = TimeValue.THIRTY_SECONDS;`
	`129`	`+ TimeValue RESERVED_CLUSTER_STATE_HANDLER_IGNORED_TIMEOUT = TimeValue.THIRTY_SECONDS;`
`130`	`130`	`}`