Skip to content

Commit 0a31ce6

Browse files
authored
Remove dangerous default executor from HandledTransportAction (#100162)
Today subclasses of `HandledTransportAction` can specify the executor on which they run, but the executor is optional and if omitted will use `DIRECT_EXECUTOR_SERVICE`, which means the action runs on a transport thread. This is a dangerous default behaviour because it makes it easy to add new transport actions which implicitly run on a network thread, which is very hard to pick up in reviews. This commit makes the executor explicit in all callers, and marks the dangerous methods for removal.
1 parent 9f08c33 commit 0a31ce6

File tree

182 files changed

+869
-197
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

182 files changed

+869
-197
lines changed

client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/bulk/TransportNoopBulkAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.elasticsearch.action.support.HandledTransportAction;
1818
import org.elasticsearch.action.update.UpdateResponse;
1919
import org.elasticsearch.common.inject.Inject;
20+
import org.elasticsearch.common.util.concurrent.EsExecutors;
2021
import org.elasticsearch.index.shard.ShardId;
2122
import org.elasticsearch.tasks.Task;
2223
import org.elasticsearch.transport.TransportService;
@@ -30,7 +31,7 @@ public class TransportNoopBulkAction extends HandledTransportAction<BulkRequest,
3031

3132
@Inject
3233
public TransportNoopBulkAction(TransportService transportService, ActionFilters actionFilters) {
33-
super(NoopBulkAction.NAME, transportService, actionFilters, BulkRequest::new);
34+
super(NoopBulkAction.NAME, transportService, actionFilters, BulkRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
3435
}
3536

3637
@Override

client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/search/TransportNoopSearchAction.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.action.support.HandledTransportAction;
1717
import org.elasticsearch.common.inject.Inject;
1818
import org.elasticsearch.common.io.stream.Writeable;
19+
import org.elasticsearch.common.util.concurrent.EsExecutors;
1920
import org.elasticsearch.search.SearchHit;
2021
import org.elasticsearch.search.SearchHits;
2122
import org.elasticsearch.search.aggregations.InternalAggregations;
@@ -30,7 +31,13 @@
3031
public class TransportNoopSearchAction extends HandledTransportAction<SearchRequest, SearchResponse> {
3132
@Inject
3233
public TransportNoopSearchAction(TransportService transportService, ActionFilters actionFilters) {
33-
super(NoopSearchAction.NAME, transportService, actionFilters, (Writeable.Reader<SearchRequest>) SearchRequest::new);
34+
super(
35+
NoopSearchAction.NAME,
36+
transportService,
37+
actionFilters,
38+
(Writeable.Reader<SearchRequest>) SearchRequest::new,
39+
EsExecutors.DIRECT_EXECUTOR_SERVICE
40+
);
3441
}
3542

3643
@Override

modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/GrokProcessorGetAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.elasticsearch.common.inject.Inject;
2020
import org.elasticsearch.common.io.stream.StreamInput;
2121
import org.elasticsearch.common.io.stream.StreamOutput;
22+
import org.elasticsearch.common.util.concurrent.EsExecutors;
2223
import org.elasticsearch.grok.GrokBuiltinPatterns;
2324
import org.elasticsearch.grok.PatternBank;
2425
import org.elasticsearch.rest.BaseRestHandler;
@@ -139,7 +140,7 @@ public TransportAction(TransportService transportService, ActionFilters actionFi
139140
PatternBank legacyGrokPatterns,
140141
PatternBank ecsV1GrokPatterns
141142
) {
142-
super(NAME, transportService, actionFilters, Request::new);
143+
super(NAME, transportService, actionFilters, Request::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
143144
this.legacyGrokPatterns = legacyGrokPatterns.bank();
144145
this.sortedLegacyGrokPatterns = new TreeMap<>(this.legacyGrokPatterns);
145146
this.ecsV1GrokPatterns = ecsV1GrokPatterns.bank();

modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportMultiSearchTemplateAction.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.action.support.HandledTransportAction;
1717
import org.elasticsearch.client.internal.node.NodeClient;
1818
import org.elasticsearch.common.inject.Inject;
19+
import org.elasticsearch.common.util.concurrent.EsExecutors;
1920
import org.elasticsearch.script.ScriptService;
2021
import org.elasticsearch.tasks.Task;
2122
import org.elasticsearch.transport.TransportService;
@@ -44,7 +45,13 @@ public TransportMultiSearchTemplateAction(
4445
NodeClient client,
4546
UsageService usageService
4647
) {
47-
super(MultiSearchTemplateAction.NAME, transportService, actionFilters, MultiSearchTemplateRequest::new);
48+
super(
49+
MultiSearchTemplateAction.NAME,
50+
transportService,
51+
actionFilters,
52+
MultiSearchTemplateRequest::new,
53+
EsExecutors.DIRECT_EXECUTOR_SERVICE
54+
);
4855
this.scriptService = scriptService;
4956
this.xContentRegistry = xContentRegistry;
5057
this.client = client;

modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/TransportSearchTemplateAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.client.internal.node.NodeClient;
1616
import org.elasticsearch.common.bytes.BytesArray;
1717
import org.elasticsearch.common.inject.Inject;
18+
import org.elasticsearch.common.util.concurrent.EsExecutors;
1819
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
1920
import org.elasticsearch.rest.action.search.RestSearchAction;
2021
import org.elasticsearch.script.Script;
@@ -54,7 +55,7 @@ public TransportSearchTemplateAction(
5455
NodeClient client,
5556
UsageService usageService
5657
) {
57-
super(SearchTemplateAction.NAME, transportService, actionFilters, SearchTemplateRequest::new);
58+
super(SearchTemplateAction.NAME, transportService, actionFilters, SearchTemplateRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
5859
this.scriptService = scriptService;
5960
this.xContentRegistry = xContentRegistry;
6061
this.client = client;

modules/lang-painless/src/main/java/org/elasticsearch/painless/action/PainlessContextAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.elasticsearch.common.io.stream.StreamInput;
2121
import org.elasticsearch.common.io.stream.StreamOutput;
2222
import org.elasticsearch.common.io.stream.Writeable;
23+
import org.elasticsearch.common.util.concurrent.EsExecutors;
2324
import org.elasticsearch.painless.PainlessScriptEngine;
2425
import org.elasticsearch.painless.lookup.PainlessLookup;
2526
import org.elasticsearch.rest.BaseRestHandler;
@@ -142,7 +143,7 @@ public static class TransportAction extends HandledTransportAction<Request, Resp
142143

143144
@Inject
144145
public TransportAction(TransportService transportService, ActionFilters actionFilters, PainlessScriptEngine painlessScriptEngine) {
145-
super(NAME, transportService, actionFilters, (Writeable.Reader<Request>) Request::new);
146+
super(NAME, transportService, actionFilters, (Writeable.Reader<Request>) Request::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
146147
this.painlessScriptEngine = painlessScriptEngine;
147148
}
148149

modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.elasticsearch.common.bytes.BytesArray;
2121
import org.elasticsearch.common.inject.Inject;
2222
import org.elasticsearch.common.util.Maps;
23+
import org.elasticsearch.common.util.concurrent.EsExecutors;
2324
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
2425
import org.elasticsearch.script.Script;
2526
import org.elasticsearch.script.ScriptService;
@@ -69,7 +70,7 @@ public TransportRankEvalAction(
6970
ScriptService scriptService,
7071
NamedXContentRegistry namedXContentRegistry
7172
) {
72-
super(RankEvalAction.NAME, transportService, actionFilters, RankEvalRequest::new);
73+
super(RankEvalAction.NAME, transportService, actionFilters, RankEvalRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
7374
this.scriptService = scriptService;
7475
this.namedXContentRegistry = namedXContentRegistry;
7576
this.client = client;

modules/reindex/src/main/java/org/elasticsearch/reindex/TransportDeleteByQueryAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.client.internal.ParentTaskAssigningClient;
1616
import org.elasticsearch.cluster.service.ClusterService;
1717
import org.elasticsearch.common.inject.Inject;
18+
import org.elasticsearch.common.util.concurrent.EsExecutors;
1819
import org.elasticsearch.index.reindex.BulkByScrollResponse;
1920
import org.elasticsearch.index.reindex.BulkByScrollTask;
2021
import org.elasticsearch.index.reindex.DeleteByQueryAction;
@@ -40,7 +41,7 @@ public TransportDeleteByQueryAction(
4041
ScriptService scriptService,
4142
ClusterService clusterService
4243
) {
43-
super(DeleteByQueryAction.NAME, transportService, actionFilters, DeleteByQueryRequest::new);
44+
super(DeleteByQueryAction.NAME, transportService, actionFilters, DeleteByQueryRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
4445
this.threadPool = threadPool;
4546
this.client = client;
4647
this.scriptService = scriptService;

modules/reindex/src/main/java/org/elasticsearch/reindex/TransportReindexAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.elasticsearch.common.settings.Setting;
2020
import org.elasticsearch.common.settings.Setting.Property;
2121
import org.elasticsearch.common.settings.Settings;
22+
import org.elasticsearch.common.util.concurrent.EsExecutors;
2223
import org.elasticsearch.index.reindex.BulkByScrollResponse;
2324
import org.elasticsearch.index.reindex.BulkByScrollTask;
2425
import org.elasticsearch.index.reindex.ReindexAction;
@@ -82,7 +83,7 @@ protected TransportReindexAction(
8283
TransportService transportService,
8384
ReindexSslConfig sslConfig
8485
) {
85-
super(name, transportService, actionFilters, ReindexRequest::new);
86+
super(name, transportService, actionFilters, ReindexRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
8687
this.client = client;
8788
this.reindexValidator = new ReindexValidator(settings, clusterService, indexNameExpressionResolver, autoCreateIndex);
8889
this.reindexer = new Reindexer(clusterService, client, threadPool, scriptService, sslConfig);

modules/reindex/src/main/java/org/elasticsearch/reindex/TransportUpdateByQueryAction.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.elasticsearch.cluster.ClusterState;
1919
import org.elasticsearch.cluster.service.ClusterService;
2020
import org.elasticsearch.common.inject.Inject;
21+
import org.elasticsearch.common.util.concurrent.EsExecutors;
2122
import org.elasticsearch.index.reindex.BulkByScrollResponse;
2223
import org.elasticsearch.index.reindex.BulkByScrollTask;
2324
import org.elasticsearch.index.reindex.ScrollableHitSource;
@@ -53,7 +54,7 @@ public TransportUpdateByQueryAction(
5354
ScriptService scriptService,
5455
ClusterService clusterService
5556
) {
56-
super(UpdateByQueryAction.NAME, transportService, actionFilters, UpdateByQueryRequest::new);
57+
super(UpdateByQueryAction.NAME, transportService, actionFilters, UpdateByQueryRequest::new, EsExecutors.DIRECT_EXECUTOR_SERVICE);
5758
this.threadPool = threadPool;
5859
this.client = client;
5960
this.scriptService = scriptService;

0 commit comments

Comments
 (0)