Skip to content

Commit 79990fa

Browse files
joegallojoegallo
authored
Remove "Push back excessive requests for stats (#83832)" (#87054)
1 parent 32c90ab commit 79990fa

File tree

26 files changed

+53
-670
lines changed

26 files changed

+53
-670
lines changed

‎docs/changelog/83832.yaml‎

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎docs/reference/cluster/nodes-stats.asciidoc‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,6 @@ using metrics.
9191
`transport`::
9292
Transport statistics about sent and received bytes in cluster
9393
communication.
94-
95-
`stats_requests`::
96-
Statistics about stats requests such as indices stats, nodes stats,
97-
recovery stats etc.
9894
--
9995

10096
`<index_metric>`::
@@ -2647,37 +2643,6 @@ search requests on the keyed node.
26472643
The rank of this node; used for shard selection when routing search
26482644
requests.
26492645
======
2650-
======
2651-
2652-
[[cluster-nodes-stats-api-response-body-stats-requests]]
2653-
`stats_requests`::
2654-
(object)
2655-
Contains statistics about the stats requests the node has received.
2656-
+
2657-
.Properties of `stats_requests`
2658-
[%collapsible%open]
2659-
======
2660-
`<stats_requests_name>`::
2661-
(object)
2662-
Contains statistics about a specific type of a stats request the node has received.
2663-
+
2664-
.Properties of `<stats_requests_name>`
2665-
[%collapsible%open]
2666-
=======
2667-
`current`::
2668-
(integer)
2669-
Number of stats requests currently in progress.
2670-
2671-
`completed`::
2672-
(integer)
2673-
Number of stats requests that have been completed by the node (successfully or
2674-
not).
2675-
2676-
`rejected`::
2677-
(integer)
2678-
Number of stats requests that were rejected by the node because it had reached
2679-
the limit of concurrent stats requests (`node.stats.max_concurrent_requests`).
2680-
=======
26812646
=====
26822647
====
26832648

‎docs/reference/modules/cluster/misc.asciidoc‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -101,24 +101,6 @@ number of shards for each node, use the
101101
setting.
102102
--
103103

104-
[[stats-requests-limit]]
105-
===== Stats request limit
106-
107-
A stats request might require information from all nodes to be aggregated before it returns to the user.
108-
These requests can be heavy and they put extra pressure on the coordinating node (the node collecting the
109-
responses from all the nodes), for this reason there is a limit on the concurrent requests that a node can coordinate.
110-
111-
--
112-
113-
[[node-stats-max-concurrent-requests]]
114-
`node.stats.max_concurrent_requests`::
115-
+
116-
--
117-
(<<dynamic-cluster-setting,Dynamic>>)
118-
Limits the stats requests a coordinating node can concurrently handle. Defaults to `100`.
119-
120-
121-
122104
[[user-defined-data]]
123105
===== User-defined cluster metadata
124106

‎server/src/internalClusterTest/java/org/elasticsearch/indices/stats/IndexStatsIT.java‎

Lines changed: 37 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@
7979
import java.util.concurrent.atomic.AtomicInteger;
8080
import java.util.concurrent.atomic.AtomicReference;
8181

82-
import static org.elasticsearch.action.support.StatsRequestLimiter.MAX_CONCURRENT_STATS_REQUESTS_PER_NODE;
8382
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
8483
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
8584
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAllSuccessful;
@@ -1387,57 +1386,52 @@ public void testConcurrentIndexingAndStatsRequests() throws BrokenBarrierExcepti
13871386
}
13881387

13891388
// start threads that will get stats concurrently with indexing
1390-
try {
1391-
updateClusterSettings(Settings.builder().put(MAX_CONCURRENT_STATS_REQUESTS_PER_NODE.getKey(), numberOfStatsThreads + 1));
1392-
for (int i = 0; i < numberOfStatsThreads; i++) {
1393-
final Thread thread = new Thread(() -> {
1389+
for (int i = 0; i < numberOfStatsThreads; i++) {
1390+
final Thread thread = new Thread(() -> {
1391+
try {
1392+
barrier.await();
1393+
} catch (final BrokenBarrierException | InterruptedException e) {
1394+
failed.set(true);
1395+
executionFailures.get().add(e);
1396+
latch.countDown();
1397+
}
1398+
final IndicesStatsRequest request = new IndicesStatsRequest();
1399+
request.all();
1400+
request.indices(new String[0]);
1401+
while (stop.get() == false) {
13941402
try {
1395-
barrier.await();
1396-
} catch (final BrokenBarrierException | InterruptedException e) {
1397-
failed.set(true);
1398-
executionFailures.get().add(e);
1399-
latch.countDown();
1400-
}
1401-
final IndicesStatsRequest request = new IndicesStatsRequest();
1402-
request.all();
1403-
request.indices(new String[0]);
1404-
while (stop.get() == false) {
1405-
try {
1406-
final IndicesStatsResponse response = client().admin().indices().stats(request).get();
1407-
if (response.getFailedShards() > 0) {
1408-
failed.set(true);
1409-
shardFailures.get().addAll(Arrays.asList(response.getShardFailures()));
1410-
latch.countDown();
1411-
}
1412-
} catch (final ExecutionException | InterruptedException e) {
1403+
final IndicesStatsResponse response = client().admin().indices().stats(request).get();
1404+
if (response.getFailedShards() > 0) {
14131405
failed.set(true);
1414-
executionFailures.get().add(e);
1406+
shardFailures.get().addAll(Arrays.asList(response.getShardFailures()));
14151407
latch.countDown();
14161408
}
1409+
} catch (final ExecutionException | InterruptedException e) {
1410+
failed.set(true);
1411+
executionFailures.get().add(e);
1412+
latch.countDown();
14171413
}
1418-
});
1419-
thread.setName("stats-" + i);
1420-
threads.add(thread);
1421-
thread.start();
1422-
}
1423-
1424-
// release the hounds
1425-
barrier.await();
1414+
}
1415+
});
1416+
thread.setName("stats-" + i);
1417+
threads.add(thread);
1418+
thread.start();
1419+
}
14261420

1427-
// wait for a failure, or for fifteen seconds to elapse
1428-
latch.await(15, TimeUnit.SECONDS);
1421+
// release the hounds
1422+
barrier.await();
14291423

1430-
// stop all threads and wait for them to complete
1431-
stop.set(true);
1432-
for (final Thread thread : threads) {
1433-
thread.join();
1434-
}
1424+
// wait for a failure, or for fifteen seconds to elapse
1425+
latch.await(15, TimeUnit.SECONDS);
14351426

1436-
assertThat(shardFailures.get(), emptyCollectionOf(DefaultShardOperationFailedException.class));
1437-
assertThat(executionFailures.get(), emptyCollectionOf(Exception.class));
1438-
} finally {
1439-
updateClusterSettings(Settings.builder().putNull(MAX_CONCURRENT_STATS_REQUESTS_PER_NODE.getKey()));
1427+
// stop all threads and wait for them to complete
1428+
stop.set(true);
1429+
for (final Thread thread : threads) {
1430+
thread.join();
14401431
}
1432+
1433+
assertThat(shardFailures.get(), emptyCollectionOf(DefaultShardOperationFailedException.class));
1434+
assertThat(executionFailures.get(), emptyCollectionOf(Exception.class));
14411435
}
14421436

14431437
/**

‎server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/TransportNodesInfoAction.java‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@
88

99
package org.elasticsearch.action.admin.cluster.node.info;
1010

11-
import org.elasticsearch.action.ActionListener;
1211
import org.elasticsearch.action.FailedNodeException;
1312
import org.elasticsearch.action.support.ActionFilters;
14-
import org.elasticsearch.action.support.StatsRequestLimiter;
1513
import org.elasticsearch.action.support.nodes.TransportNodesAction;
1614
import org.elasticsearch.cluster.node.DiscoveryNode;
1715
import org.elasticsearch.cluster.service.ClusterService;
@@ -35,16 +33,14 @@ public class TransportNodesInfoAction extends TransportNodesAction<
3533
NodeInfo> {
3634

3735
private final NodeService nodeService;
38-
private final StatsRequestLimiter statsRequestLimiter;
3936

4037
@Inject
4138
public TransportNodesInfoAction(
4239
ThreadPool threadPool,
4340
ClusterService clusterService,
4441
TransportService transportService,
4542
NodeService nodeService,
46-
ActionFilters actionFilters,
47-
StatsRequestLimiter statsRequestLimiter
43+
ActionFilters actionFilters
4844
) {
4945
super(
5046
NodesInfoAction.NAME,
@@ -58,7 +54,6 @@ public TransportNodesInfoAction(
5854
NodeInfo.class
5955
);
6056
this.nodeService = nodeService;
61-
this.statsRequestLimiter = statsRequestLimiter;
6257
}
6358

6459
@Override
@@ -99,11 +94,6 @@ protected NodeInfo nodeOperation(NodeInfoRequest nodeRequest, Task task) {
9994
);
10095
}
10196

102-
@Override
103-
protected void doExecute(Task task, NodesInfoRequest request, ActionListener<NodesInfoResponse> listener) {
104-
statsRequestLimiter.tryToExecute(task, request, listener, super::doExecute);
105-
}
106-
10797
public static class NodeInfoRequest extends TransportRequest {
10898

10999
NodesInfoRequest request;

‎server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88

99
package org.elasticsearch.action.admin.cluster.node.stats;
1010

11-
import org.elasticsearch.Version;
12-
import org.elasticsearch.action.support.StatsRequestStats;
1311
import org.elasticsearch.action.support.nodes.BaseNodeResponse;
1412
import org.elasticsearch.cluster.node.DiscoveryNode;
1513
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
@@ -89,9 +87,6 @@ public class NodeStats extends BaseNodeResponse implements ToXContentFragment {
8987
@Nullable
9088
private IndexingPressureStats indexingPressureStats;
9189

92-
@Nullable
93-
private StatsRequestStats statsRequestStats;
94-
9590
public NodeStats(StreamInput in) throws IOException {
9691
super(in);
9792
timestamp = in.readVLong();
@@ -112,9 +107,6 @@ public NodeStats(StreamInput in) throws IOException {
112107
ingestStats = in.readOptionalWriteable(IngestStats::new);
113108
adaptiveSelectionStats = in.readOptionalWriteable(AdaptiveSelectionStats::new);
114109
indexingPressureStats = in.readOptionalWriteable(IndexingPressureStats::new);
115-
if (in.getVersion().onOrAfter(Version.V_8_3_0)) {
116-
statsRequestStats = in.readOptionalWriteable(StatsRequestStats::new);
117-
}
118110
}
119111

120112
public NodeStats(
@@ -134,8 +126,7 @@ public NodeStats(
134126
@Nullable IngestStats ingestStats,
135127
@Nullable AdaptiveSelectionStats adaptiveSelectionStats,
136128
@Nullable ScriptCacheStats scriptCacheStats,
137-
@Nullable IndexingPressureStats indexingPressureStats,
138-
@Nullable StatsRequestStats statsRequestStats
129+
@Nullable IndexingPressureStats indexingPressureStats
139130
) {
140131
super(node);
141132
this.timestamp = timestamp;
@@ -154,7 +145,6 @@ public NodeStats(
154145
this.adaptiveSelectionStats = adaptiveSelectionStats;
155146
this.scriptCacheStats = scriptCacheStats;
156147
this.indexingPressureStats = indexingPressureStats;
157-
this.statsRequestStats = statsRequestStats;
158148
}
159149

160150
public long getTimestamp() {
@@ -259,11 +249,6 @@ public IndexingPressureStats getIndexingPressureStats() {
259249
return indexingPressureStats;
260250
}
261251

262-
@Nullable
263-
public StatsRequestStats getStatsRequestStats() {
264-
return statsRequestStats;
265-
}
266-
267252
@Override
268253
public void writeTo(StreamOutput out) throws IOException {
269254
super.writeTo(out);
@@ -287,9 +272,6 @@ public void writeTo(StreamOutput out) throws IOException {
287272
out.writeOptionalWriteable(ingestStats);
288273
out.writeOptionalWriteable(adaptiveSelectionStats);
289274
out.writeOptionalWriteable(indexingPressureStats);
290-
if (out.getVersion().onOrAfter(Version.V_8_3_0)) {
291-
out.writeOptionalWriteable(statsRequestStats);
292-
}
293275
}
294276

295277
@Override
@@ -359,9 +341,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
359341
if (getIndexingPressureStats() != null) {
360342
getIndexingPressureStats().toXContent(builder, params);
361343
}
362-
if (getStatsRequestStats() != null) {
363-
getStatsRequestStats().toXContent(builder, params);
364-
}
365344
return builder;
366345
}
367346
}

‎server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodesStatsRequest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,7 @@ public enum Metric {
189189
INGEST("ingest"),
190190
ADAPTIVE_SELECTION("adaptive_selection"),
191191
SCRIPT_CACHE("script_cache"),
192-
INDEXING_PRESSURE("indexing_pressure"),
193-
STATS_REQUESTS("stats_requests"),;
192+
INDEXING_PRESSURE("indexing_pressure"),;
194193

195194
private String metricName;
196195

‎server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/TransportNodesStatsAction.java‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@
88

99
package org.elasticsearch.action.admin.cluster.node.stats;
1010

11-
import org.elasticsearch.action.ActionListener;
1211
import org.elasticsearch.action.FailedNodeException;
1312
import org.elasticsearch.action.support.ActionFilters;
14-
import org.elasticsearch.action.support.StatsRequestLimiter;
1513
import org.elasticsearch.action.support.nodes.TransportNodesAction;
1614
import org.elasticsearch.cluster.node.DiscoveryNode;
1715
import org.elasticsearch.cluster.service.ClusterService;
@@ -38,16 +36,14 @@ public class TransportNodesStatsAction extends TransportNodesAction<
3836
NodeStats> {
3937

4038
private final NodeService nodeService;
41-
private final StatsRequestLimiter statsRequestLimiter;
4239

4340
@Inject
4441
public TransportNodesStatsAction(
4542
ThreadPool threadPool,
4643
ClusterService clusterService,
4744
TransportService transportService,
4845
NodeService nodeService,
49-
ActionFilters actionFilters,
50-
StatsRequestLimiter statsRequestLimiter
46+
ActionFilters actionFilters
5147
) {
5248
super(
5349
NodesStatsAction.NAME,
@@ -61,7 +57,6 @@ public TransportNodesStatsAction(
6157
NodeStats.class
6258
);
6359
this.nodeService = nodeService;
64-
this.statsRequestLimiter = statsRequestLimiter;
6560
}
6661

6762
@Override
@@ -100,16 +95,10 @@ protected NodeStats nodeOperation(NodeStatsRequest nodeStatsRequest, Task task)
10095
NodesStatsRequest.Metric.INGEST.containedIn(metrics),
10196
NodesStatsRequest.Metric.ADAPTIVE_SELECTION.containedIn(metrics),
10297
NodesStatsRequest.Metric.SCRIPT_CACHE.containedIn(metrics),
103-
NodesStatsRequest.Metric.INDEXING_PRESSURE.containedIn(metrics),
104-
NodesStatsRequest.Metric.STATS_REQUESTS.containedIn(metrics)
98+
NodesStatsRequest.Metric.INDEXING_PRESSURE.containedIn(metrics)
10599
);
106100
}
107101

108-
@Override
109-
protected void doExecute(Task task, NodesStatsRequest request, ActionListener<NodesStatsResponse> listener) {
110-
statsRequestLimiter.tryToExecute(task, request, listener, super::doExecute);
111-
}
112-
113102
public static class NodeStatsRequest extends TransportRequest {
114103

115104
NodesStatsRequest request;

0 commit comments

Comments
 (0)