Skip to content

Commit bc9de12

Browse files
dnhatndnhatn
authored
Limit concurrent shard requests in disk usage API (#84900) (#85367)
The disk_usage API fails on indices that have more than 16 shards. The reason is that we broadcast all shard requests simultaneously, and the handling thread pool - the analyzer thread pool - whose queue size is only 16. That means some shard requests will be rejected. This PR limits the number of concurrent shard requests sent to each node when we broadcast them. Closes #84779
1 parent 9576c09 commit bc9de12

File tree

5 files changed

+563
-19
lines changed

5 files changed

+563
-19
lines changed

‎docs/changelog/84900.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 84900
2+
summary: Limit concurrent shard requests in disk usage API
3+
area: Search
4+
type: bug
5+
issues:
6+
- 84779

‎server/src/internalClusterTest/java/org/elasticsearch/action/admin/indices/diskusage/IndexDiskUsageAnalyzerIT.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import org.apache.lucene.geo.GeoTestUtil;
1212
import org.apache.lucene.util.English;
13+
import org.elasticsearch.action.support.IndicesOptions;
1314
import org.elasticsearch.action.support.PlainActionFuture;
1415
import org.elasticsearch.cluster.metadata.IndexMetadata;
1516
import org.elasticsearch.common.settings.Settings;
@@ -28,12 +29,14 @@
2829
import org.elasticsearch.xcontent.XContentFactory;
2930
import org.junit.Before;
3031

32+
import java.util.Arrays;
3133
import java.util.Collection;
3234
import java.util.List;
3335
import java.util.Optional;
3436
import java.util.Set;
3537
import java.util.stream.IntStream;
3638

39+
import static org.hamcrest.Matchers.emptyArray;
3740
import static org.hamcrest.Matchers.equalTo;
3841
import static org.hamcrest.Matchers.greaterThan;
3942

@@ -239,6 +242,50 @@ public void testFailOnFlush() throws Exception {
239242
assertThat(resp.getFailedShards(), equalTo(failedShards.size()));
240243
}
241244

245+
public void testManyShards() throws Exception {
246+
List<String> indices = IntStream.range(0, between(1, 5)).mapToObj(n -> "index_" + n).toList();
247+
int totalShards = 0;
248+
for (String indexName : indices) {
249+
int numberOfShards = between(10, 30);
250+
client().admin()
251+
.indices()
252+
.prepareCreate(indexName)
253+
.setSettings(
254+
Settings.builder()
255+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numberOfShards)
256+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, between(0, 1))
257+
.put("index.shard.check_on_startup", false)
258+
.put("index.routing.rebalance.enable", "none")
259+
)
260+
.get();
261+
totalShards += numberOfShards;
262+
int numDocs = randomIntBetween(10, 100);
263+
for (int i = 0; i < numDocs; i++) {
264+
int value = randomIntBetween(5, 20);
265+
final XContentBuilder doc = XContentFactory.jsonBuilder()
266+
.startObject()
267+
.field("english_text", English.intToEnglish(value))
268+
.field("value", value)
269+
.endObject();
270+
client().prepareIndex(indexName).setId("id-" + i).setSource(doc).get();
271+
}
272+
}
273+
274+
AnalyzeIndexDiskUsageResponse resp = client().execute(
275+
AnalyzeIndexDiskUsageAction.INSTANCE,
276+
new AnalyzeIndexDiskUsageRequest(new String[] { "index_*" }, IndicesOptions.fromOptions(false, false, true, true), true)
277+
).actionGet();
278+
assertThat(Arrays.toString(resp.getShardFailures()), resp.getShardFailures(), emptyArray());
279+
assertThat(resp.getTotalShards(), equalTo(totalShards));
280+
assertThat(resp.getSuccessfulShards(), equalTo(totalShards));
281+
assertThat(resp.getFailedShards(), equalTo(0));
282+
for (String index : indices) {
283+
IndexDiskUsageStats stats = resp.getStats().get(index);
284+
assertThat(stats.getIndexSizeInBytes(), greaterThan(0L));
285+
assertThat(stats.total().totalBytes(), greaterThan(0L));
286+
}
287+
}
288+
242289
void assertMetadataFields(IndexDiskUsageStats stats) {
243290
final IndexDiskUsageStats.PerFieldDiskUsage sourceField = stats.getFields().get("_source");
244291
assertThat(sourceField.getInvertedIndexBytes(), equalTo(0L));

‎server/src/main/java/org/elasticsearch/action/admin/indices/diskusage/TransportAnalyzeIndexDiskUsageAction.java‎

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@
1818
import org.elasticsearch.cluster.block.ClusterBlockException;
1919
import org.elasticsearch.cluster.block.ClusterBlockLevel;
2020
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
21+
import org.elasticsearch.cluster.node.DiscoveryNode;
2122
import org.elasticsearch.cluster.routing.GroupShardsIterator;
2223
import org.elasticsearch.cluster.routing.ShardIterator;
2324
import org.elasticsearch.cluster.routing.ShardRouting;
2425
import org.elasticsearch.cluster.service.ClusterService;
2526
import org.elasticsearch.common.inject.Inject;
2627
import org.elasticsearch.common.io.stream.StreamInput;
28+
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
2729
import org.elasticsearch.index.engine.Engine;
2830
import org.elasticsearch.index.shard.IndexShard;
2931
import org.elasticsearch.index.shard.ShardId;
@@ -36,8 +38,12 @@
3638
import java.io.IOException;
3739
import java.util.ArrayList;
3840
import java.util.HashMap;
41+
import java.util.Iterator;
42+
import java.util.LinkedList;
3943
import java.util.List;
4044
import java.util.Map;
45+
import java.util.Queue;
46+
import java.util.concurrent.atomic.AtomicInteger;
4147
import java.util.concurrent.atomic.AtomicReferenceArray;
4248

4349
public class TransportAnalyzeIndexDiskUsageAction extends TransportBroadcastAction<
@@ -46,6 +52,7 @@ public class TransportAnalyzeIndexDiskUsageAction extends TransportBroadcastActi
4652
AnalyzeDiskUsageShardRequest,
4753
AnalyzeDiskUsageShardResponse> {
4854
private final IndicesService indicesService;
55+
private final ThreadPool threadPool;
4956

5057
@Inject
5158
public TransportAnalyzeIndexDiskUsageAction(
@@ -66,11 +73,92 @@ public TransportAnalyzeIndexDiskUsageAction(
6673
ThreadPool.Names.ANALYZE
6774
);
6875
this.indicesService = indexServices;
76+
this.threadPool = transportService.getThreadPool();
6977
}
7078

7179
@Override
7280
protected void doExecute(Task task, AnalyzeIndexDiskUsageRequest request, ActionListener<AnalyzeIndexDiskUsageResponse> listener) {
73-
super.doExecute(task, request, listener);
81+
new LimitingRequestPerNodeBroadcastAction(task, request, listener, 5).start();
82+
}
83+
84+
private record ShardRequest(
85+
DiscoveryNode node,
86+
AnalyzeDiskUsageShardRequest shardRequest,
87+
ActionListener<AnalyzeDiskUsageShardResponse> handler
88+
) {
89+
90+
}
91+
92+
final class LimitingRequestPerNodeBroadcastAction extends AsyncBroadcastAction {
93+
private final Queue<ShardRequest> queue = new LinkedList<>();
94+
private final Map<DiscoveryNode, AtomicInteger> sendingCounters = ConcurrentCollections.newConcurrentMap();
95+
private final int maxConcurrentRequestsPerNode;
96+
97+
LimitingRequestPerNodeBroadcastAction(
98+
Task task,
99+
AnalyzeIndexDiskUsageRequest request,
100+
ActionListener<AnalyzeIndexDiskUsageResponse> listener,
101+
int maxConcurrentRequestsPerNode
102+
) {
103+
super(task, request, listener);
104+
this.maxConcurrentRequestsPerNode = maxConcurrentRequestsPerNode;
105+
}
106+
107+
private void trySendRequests() {
108+
assert Thread.holdsLock(this) == false;
109+
final List<ShardRequest> readyRequests = new ArrayList<>();
110+
synchronized (this) {
111+
final Iterator<ShardRequest> it = queue.iterator();
112+
while (it.hasNext()) {
113+
final ShardRequest r = it.next();
114+
final AtomicInteger sending = sendingCounters.computeIfAbsent(r.node, k -> new AtomicInteger());
115+
assert 0 <= sending.get() && sending.get() <= maxConcurrentRequestsPerNode : sending;
116+
if (sending.get() < maxConcurrentRequestsPerNode) {
117+
sending.incrementAndGet();
118+
readyRequests.add(r);
119+
it.remove();
120+
}
121+
}
122+
}
123+
if (readyRequests.isEmpty()) {
124+
return;
125+
}
126+
final Thread sendingThread = Thread.currentThread();
127+
for (ShardRequest r : readyRequests) {
128+
super.sendShardRequest(
129+
r.node,
130+
r.shardRequest,
131+
ActionListener.runAfter(r.handler, () -> onRequestResponded(sendingThread, r.node))
132+
);
133+
}
134+
}
135+
136+
private void onRequestResponded(Thread sendingThread, DiscoveryNode node) {
137+
final AtomicInteger sending = sendingCounters.get(node);
138+
assert sending != null && 1 <= sending.get() && sending.get() <= maxConcurrentRequestsPerNode : sending;
139+
sending.decrementAndGet();
140+
// fork to avoid StackOverflow
141+
if (sendingThread == Thread.currentThread()) {
142+
threadPool.generic().execute(this::trySendRequests);
143+
} else {
144+
trySendRequests();
145+
}
146+
}
147+
148+
@Override
149+
protected synchronized void sendShardRequest(
150+
DiscoveryNode node,
151+
AnalyzeDiskUsageShardRequest shardRequest,
152+
ActionListener<AnalyzeDiskUsageShardResponse> listener
153+
) {
154+
queue.add(new ShardRequest(node, shardRequest, listener));
155+
}
156+
157+
@Override
158+
public void start() {
159+
super.start();
160+
trySendRequests();
161+
}
74162
}
75163

76164
@Override

‎server/src/main/java/org/elasticsearch/action/support/broadcast/TransportBroadcastAction.java‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -171,26 +171,10 @@ protected void performOperation(final ShardIterator shardIt, final ShardRouting
171171
// no node connected, act as failure
172172
onOperation(shard, shardIt, shardIndex, new NoShardAvailableActionException(shardIt.shardId()));
173173
} else {
174-
transportService.sendRequest(
174+
sendShardRequest(
175175
node,
176-
transportShardAction,
177176
shardRequest,
178-
new TransportResponseHandler<ShardResponse>() {
179-
@Override
180-
public ShardResponse read(StreamInput in) throws IOException {
181-
return readShardResponse(in);
182-
}
183-
184-
@Override
185-
public void handleResponse(ShardResponse response) {
186-
onOperation(shard, shardIndex, response);
187-
}
188-
189-
@Override
190-
public void handleException(TransportException e) {
191-
onOperation(shard, shardIt, shardIndex, e);
192-
}
193-
}
177+
ActionListener.wrap(r -> onOperation(shard, shardIndex, r), e -> onOperation(shard, shardIt, shardIndex, e))
194178
);
195179
}
196180
} catch (Exception e) {
@@ -199,6 +183,25 @@ public void handleException(TransportException e) {
199183
}
200184
}
201185

186+
protected void sendShardRequest(DiscoveryNode node, ShardRequest shardRequest, ActionListener<ShardResponse> listener) {
187+
transportService.sendRequest(node, transportShardAction, shardRequest, new TransportResponseHandler<ShardResponse>() {
188+
@Override
189+
public ShardResponse read(StreamInput in) throws IOException {
190+
return readShardResponse(in);
191+
}
192+
193+
@Override
194+
public void handleResponse(ShardResponse response) {
195+
listener.onResponse(response);
196+
}
197+
198+
@Override
199+
public void handleException(TransportException e) {
200+
listener.onFailure(e);
201+
}
202+
});
203+
}
204+
202205
protected void onOperation(ShardRouting shard, int shardIndex, ShardResponse response) {
203206
logger.trace("received response for {}", shard);
204207
shardsResponses.set(shardIndex, response);

0 commit comments

Comments
 (0)