Skip to content

Commit

Permalink
Run TransportGetComponentTemplateAction on local node (#116868)
Browse files Browse the repository at this point in the history
This action solely needs the cluster state, it can run on any node.
Additionally, it needs to be cancellable to avoid doing unnecessary work
after a client failure or timeout.

The `?local` parameter becomes a no-op and is marked as deprecated.

Relates #101805
Relates #107984
  • Loading branch information
nielsbauman authored Dec 23, 2024
1 parent c16bcb6 commit 9641c76
Show file tree
Hide file tree
Showing 15 changed files with 170 additions and 99 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/116868.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 116868
summary: Run `TransportGetComponentTemplateAction` on local node
area: Indices APIs
type: enhancement
issues: []
2 changes: 1 addition & 1 deletion docs/reference/indices/get-component-template.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Wildcard (`*`) expressions are supported.

include::{docdir}/rest-api/common-parms.asciidoc[tag=flat-settings]

include::{docdir}/rest-api/common-parms.asciidoc[tag=local]
include::{docdir}/rest-api/common-parms.asciidoc[tag=local-deprecated-9.0.0]

include::{docdir}/rest-api/common-parms.asciidoc[tag=master-timeout]

Expand Down
10 changes: 10 additions & 0 deletions docs/reference/rest-api/common-parms.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,16 @@ node only. Defaults to `false`, which means information is retrieved from
the master node.
end::local[]

tag::local-deprecated-9.0.0[]
`local`::
(Optional, Boolean) If `true`, the request retrieves information from the local
node only. Defaults to `false`, which means information is retrieved from
the master node.
+
deprecated::[9.0.0, "The `?local` query parameter to this API has no effect, is now deprecated, and will be removed in a future version."]

end::local-deprecated-9.0.0[]

tag::mappings[]
`mappings`::
+
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.elasticsearch.action.admin.cluster.state.ClusterStateAction;
import org.elasticsearch.action.admin.indices.alias.get.GetAliasesAction;
import org.elasticsearch.action.admin.indices.recovery.RecoveryAction;
import org.elasticsearch.action.admin.indices.template.get.GetComponentTemplateAction;
import org.elasticsearch.action.support.CancellableActionTestPlugin;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.action.support.RefCountingListener;
Expand Down Expand Up @@ -66,6 +67,10 @@ public void testCatAliasesCancellation() {
runRestActionCancellationTest(new Request(HttpGet.METHOD_NAME, "/_cat/aliases"), GetAliasesAction.NAME);
}

public void testGetComponentTemplateCancellation() {
runRestActionCancellationTest(new Request(HttpGet.METHOD_NAME, "/_component_template"), GetComponentTemplateAction.NAME);
}

private void runRestActionCancellationTest(Request request, String actionName) {
final var node = usually() ? internalCluster().getRandomNodeName() : internalCluster().startCoordinatingOnlyNode(Settings.EMPTY);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@
"params":{
"master_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to master node"
"description":"Timeout for waiting for new cluster state in case it is blocked"
},
"local":{
"deprecated":true,
"type":"boolean",
"description":"Return local information, do not retrieve the state from master node (default: false)"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@
"params":{
"master_timeout":{
"type":"time",
"description":"Explicit operation timeout for connection to master node"
"description":"Timeout for waiting for new cluster state in case it is blocked"
},
"local":{
"deprecated":true,
"type":"boolean",
"description":"Return local information, do not retrieve the state from master node (default: false)"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,33 @@
- match: {component_templates.0.component_template.template.lifecycle.enabled: true}
- match: {component_templates.0.component_template.template.lifecycle.data_retention: "10d"}
- is_true: component_templates.0.component_template.template.lifecycle.rollover

---
"Deprecated local parameter":
- requires:
capabilities:
- method: GET
path: /_component_template
capabilities: ["local_param_deprecated"]
test_runner_features: ["capabilities", "warnings"]
reason: Deprecation was implemented with capability

- do:
cluster.get_component_template:
local: true
warnings:
- "the [?local] query parameter to this API has no effect, is now deprecated, and will be removed in a future version"

---
"Deprecated local parameter works in v8 compat mode":
- requires:
test_runner_features: ["headers"]

- do:
headers:
Content-Type: "application/vnd.elasticsearch+json;compatible-with=8"
Accept: "application/vnd.elasticsearch+json;compatible-with=8"
cluster.get_component_template:
local: true

- exists: component_templates
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ private void assertComponentAndIndexTemplateDelete(CountDownLatch savedClusterSt

final var componentResponse = client().execute(
GetComponentTemplateAction.INSTANCE,
new GetComponentTemplateAction.Request("other*")
new GetComponentTemplateAction.Request(TEST_REQUEST_TIMEOUT, "other*")
).get();

assertTrue(componentResponse.getComponentTemplates().isEmpty());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,17 @@
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.ActionType;
import org.elasticsearch.action.admin.indices.rollover.RolloverConfiguration;
import org.elasticsearch.action.support.master.MasterNodeReadRequest;
import org.elasticsearch.action.support.local.LocalClusterStateRequest;
import org.elasticsearch.cluster.metadata.ComponentTemplate;
import org.elasticsearch.cluster.metadata.DataStreamGlobalRetention;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.core.UpdateForV10;
import org.elasticsearch.tasks.CancellableTask;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.tasks.TaskId;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
Expand All @@ -43,22 +48,23 @@ private GetComponentTemplateAction() {
/**
* Request that to retrieve one or more component templates
*/
public static class Request extends MasterNodeReadRequest<Request> {
public static class Request extends LocalClusterStateRequest {

@Nullable
private String name;
private boolean includeDefaults;

public Request() {
super(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT);
}

public Request(String name) {
super(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT);
public Request(TimeValue masterTimeout, String name) {
super(masterTimeout);
this.name = name;
this.includeDefaults = false;
}

/**
* NB prior to 9.0 get-component was a TransportMasterNodeReadAction so for BwC we must remain able to read these requests until
* we no longer need to support calling this action remotely.
*/
@UpdateForV10(owner = UpdateForV10.Owner.DATA_MANAGEMENT)
public Request(StreamInput in) throws IOException {
super(in);
name = in.readOptionalString();
Expand All @@ -70,17 +76,13 @@ public Request(StreamInput in) throws IOException {
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeOptionalString(name);
if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_9_X)) {
out.writeBoolean(includeDefaults);
}
public ActionRequestValidationException validate() {
return null;
}

@Override
public ActionRequestValidationException validate() {
return null;
public Task createTask(long id, String type, String action, TaskId parentTaskId, Map<String, String> headers) {
return new CancellableTask(id, type, action, "", parentTaskId, headers);
}

/**
Expand Down Expand Up @@ -123,19 +125,6 @@ public static class Response extends ActionResponse implements ToXContentObject
@Nullable
private final RolloverConfiguration rolloverConfiguration;

public Response(StreamInput in) throws IOException {
super(in);
componentTemplates = in.readMap(ComponentTemplate::new);
if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_9_X)) {
rolloverConfiguration = in.readOptionalWriteable(RolloverConfiguration::new);
} else {
rolloverConfiguration = null;
}
if (in.getTransportVersion().between(TransportVersions.V_8_14_0, TransportVersions.V_8_16_0)) {
in.readOptionalWriteable(DataStreamGlobalRetention::read);
}
}

/**
* Please use {@link GetComponentTemplateAction.Response#Response(Map)}
*/
Expand Down Expand Up @@ -183,6 +172,11 @@ public DataStreamGlobalRetention getGlobalRetention() {
return null;
}

/**
* NB prior to 9.0 get-component was a TransportMasterNodeReadAction so for BwC we must remain able to write these responses until
* we no longer need to support calling this action remotely.
*/
@UpdateForV10(owner = UpdateForV10.Owner.DATA_MANAGEMENT)
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeMap(componentTemplates, StreamOutput::writeWriteable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,51 +12,61 @@
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.master.TransportMasterNodeReadAction;
import org.elasticsearch.action.support.ChannelActionListener;
import org.elasticsearch.action.support.local.TransportLocalClusterStateAction;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.block.ClusterBlockException;
import org.elasticsearch.cluster.block.ClusterBlockLevel;
import org.elasticsearch.cluster.metadata.ComponentTemplate;
import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.ClusterSettings;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.core.UpdateForV10;
import org.elasticsearch.injection.guice.Inject;
import org.elasticsearch.tasks.CancellableTask;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;

import java.util.HashMap;
import java.util.Map;

public class TransportGetComponentTemplateAction extends TransportMasterNodeReadAction<
public class TransportGetComponentTemplateAction extends TransportLocalClusterStateAction<
GetComponentTemplateAction.Request,
GetComponentTemplateAction.Response> {

private final ClusterSettings clusterSettings;

/**
* NB prior to 9.0 this was a TransportMasterNodeReadAction so for BwC it must be registered with the TransportService until
* we no longer need to support calling this action remotely.
*/
@UpdateForV10(owner = UpdateForV10.Owner.DATA_MANAGEMENT)
@SuppressWarnings("this-escape")
@Inject
public TransportGetComponentTemplateAction(
TransportService transportService,
ClusterService clusterService,
ThreadPool threadPool,
ActionFilters actionFilters,
IndexNameExpressionResolver indexNameExpressionResolver
ActionFilters actionFilters
) {
super(
GetComponentTemplateAction.NAME,
transportService,
clusterService,
threadPool,
actionFilters,
GetComponentTemplateAction.Request::new,
indexNameExpressionResolver,
GetComponentTemplateAction.Response::new,
transportService.getTaskManager(),
clusterService,
EsExecutors.DIRECT_EXECUTOR_SERVICE
);
clusterSettings = clusterService.getClusterSettings();

transportService.registerRequestHandler(
actionName,
executor,
false,
true,
GetComponentTemplateAction.Request::new,
(request, channel, task) -> executeDirect(task, request, new ChannelActionListener<>(channel))
);
}

@Override
Expand All @@ -65,12 +75,13 @@ protected ClusterBlockException checkBlock(GetComponentTemplateAction.Request re
}

@Override
protected void masterOperation(
protected void localClusterStateOperation(
Task task,
GetComponentTemplateAction.Request request,
ClusterState state,
ActionListener<GetComponentTemplateAction.Response> listener
) {
final var cancellableTask = (CancellableTask) task;
Map<String, ComponentTemplate> allTemplates = state.metadata().componentTemplates();
Map<String, ComponentTemplate> results;

Expand All @@ -93,6 +104,7 @@ protected void masterOperation(

}
}
cancellableTask.ensureNotCancelled();
if (request.includeDefaults()) {
listener.onResponse(
new GetComponentTemplateAction.Response(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@

package org.elasticsearch.action.support.local;

import org.elasticsearch.TransportVersions;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.support.TransportAction;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.core.UpdateForV10;

import java.io.IOException;
import java.util.Objects;
Expand All @@ -32,6 +35,20 @@ protected LocalClusterStateRequest(TimeValue masterTimeout) {
this.masterTimeout = Objects.requireNonNull(masterTimeout);
}

/**
* This constructor exists solely for BwC purposes. It should exclusively be used by requests that used to extend
* {@link org.elasticsearch.action.support.master.MasterNodeReadRequest} and still need to be able to serialize incoming request.
*/
@UpdateForV10(owner = UpdateForV10.Owner.DISTRIBUTED_COORDINATION)
protected LocalClusterStateRequest(StreamInput in) throws IOException {
super(in);
masterTimeout = in.readTimeValue();
if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_15_0)) {
in.readVLong();
}
in.readBoolean();
}

@Override
public final void writeTo(StreamOutput out) throws IOException {
TransportAction.localOnly();
Expand Down
24 changes: 24 additions & 0 deletions server/src/main/java/org/elasticsearch/rest/RestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@

package org.elasticsearch.rest;

import org.elasticsearch.action.support.local.TransportLocalClusterStateAction;
import org.elasticsearch.action.support.master.AcknowledgedRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.core.Booleans;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.RestApiVersion;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.core.UpdateForV10;

import java.net.URI;
import java.nio.charset.Charset;
Expand Down Expand Up @@ -323,4 +328,23 @@ public static TimeValue getTimeout(RestRequest restRequest) {
assert restRequest != null;
return restRequest.paramAsTime(REST_TIMEOUT_PARAM, null);
}

// Remove the BWC support for the deprecated ?local parameter.
// NOTE: ensure each usage of this method has been deprecated for long enough to remove it.
@UpdateForV10(owner = UpdateForV10.Owner.DISTRIBUTED_COORDINATION)
public static void consumeDeprecatedLocalParameter(RestRequest request) {
if (request.hasParam("local") == false) {
return;
}
// Consume this param just for validation when in BWC mode.
final var local = request.paramAsBoolean("local", false);
if (request.getRestApiVersion() != RestApiVersion.V_8) {
DeprecationLogger.getLogger(TransportLocalClusterStateAction.class)
.critical(
DeprecationCategory.API,
"TransportLocalClusterStateAction-local-parameter",
"the [?local] query parameter to this API has no effect, is now deprecated, and will be removed in a future version"
);
}
}
}
Loading

0 comments on commit 9641c76

Please sign in to comment.