Skip to content

Commit

Permalink
Clean up timeouts in reserved cluster state handlers (elastic#112820)
Browse files Browse the repository at this point in the history
In elastic#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.
  • Loading branch information
DaveCTurner authored Sep 12, 2024
1 parent 1beaed5 commit 46dd1a5
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 46dd1a5

Please sign in to comment.