From 46dd1a5a9bdc0c638aae198b00862d2618ecb334 Mon Sep 17 00:00:00 2001 From: David Turner Date: Thu, 12 Sep 2024 18:40:54 +0100 Subject: [PATCH] Clean up timeouts in reserved cluster state handlers (#112820) 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. --- .../reservedstate/ReservedRepositoryAction.java | 8 ++++++-- .../ReservedClusterStateHandler.java | 2 +- .../action/ReservedClusterSettingsAction.java | 6 ++---- .../action/ReservedAutoscalingPolicyAction.java | 5 ++--- .../xpack/ilm/action/ReservedLifecycleAction.java | 15 ++++++++++----- .../xpack/slm/action/ReservedSnapshotAction.java | 13 +++++++------ 6 files changed, 28 insertions(+), 21 deletions(-) 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 index d3dc7c916f066..2dae8c4d98685 100644 --- 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 fromXContent(XContentParser parser) throws IOE Map 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 content = (Map) 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 index 92cdf57102f42..0fe533dc9dc85 100644 --- 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 index ebc0d5cdf50ec..dff9b5283c67e 100644 --- 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 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 index 2a064afc591ce..ac09630f1bc9f 100644 --- 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 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 index ba41e29531b76..d68e83814915e 100644 --- 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 prepare(Object input) throws IOException List policies = (List) 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 index 192b03aa385d5..a98e110ed88de 100644 --- 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 prepare(List 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);