Skip to content

Commit 586405d

Browse files
authored
Remove trappy timeout from ClusterSearchShardsRequest (elastic#111442)
Exposes the `?master_timeout` parameter to the REST API and sets it appropriately on internal/test requests. Relates elastic#107984
1 parent 72b607b commit 586405d

File tree

18 files changed

+90
-131
lines changed

18 files changed

+90
-131
lines changed

docs/reference/search/search-shards.asciidoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ include::{es-ref-dir}/rest-api/common-parms.asciidoc[tag=preference]
6363

6464
include::{es-ref-dir}/rest-api/common-parms.asciidoc[tag=routing]
6565

66+
include::{es-ref-dir}/rest-api/common-parms.asciidoc[tag=master-timeout]
6667

6768
[[search-shards-api-example]]
6869
==== {api-examples-title}

modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,9 @@ public void testResolvabilityOfDataStreamsInAPIs() throws Exception {
585585
verifyResolvability(dataStreamName, indicesAdmin().prepareOpen(dataStreamName), false);
586586
verifyResolvability(dataStreamName, indicesAdmin().prepareClose(dataStreamName), true);
587587
verifyResolvability(aliasToDataStream, indicesAdmin().prepareClose(aliasToDataStream), true);
588-
verifyResolvability(client().execute(TransportClusterSearchShardsAction.TYPE, new ClusterSearchShardsRequest(dataStreamName)));
588+
verifyResolvability(
589+
client().execute(TransportClusterSearchShardsAction.TYPE, new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, dataStreamName))
590+
);
589591
verifyResolvability(client().execute(TransportIndicesShardStoresAction.TYPE, new IndicesShardStoresRequest(dataStreamName)));
590592

591593
request = new CreateDataStreamAction.Request("logs-barbaz");
@@ -629,7 +631,12 @@ public void testResolvabilityOfDataStreamsInAPIs() throws Exception {
629631
verifyResolvability(wildcardExpression, indicesAdmin().prepareGetIndex().addIndices(wildcardExpression), false);
630632
verifyResolvability(wildcardExpression, indicesAdmin().prepareOpen(wildcardExpression), false);
631633
verifyResolvability(wildcardExpression, indicesAdmin().prepareClose(wildcardExpression), false);
632-
verifyResolvability(client().execute(TransportClusterSearchShardsAction.TYPE, new ClusterSearchShardsRequest(wildcardExpression)));
634+
verifyResolvability(
635+
client().execute(
636+
TransportClusterSearchShardsAction.TYPE,
637+
new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, wildcardExpression)
638+
)
639+
);
633640
verifyResolvability(client().execute(TransportIndicesShardStoresAction.TYPE, new IndicesShardStoresRequest(wildcardExpression)));
634641
}
635642

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.action.ActionType;
1313
import org.elasticsearch.action.admin.cluster.shards.ClusterSearchShardsRequest;
1414
import org.elasticsearch.action.admin.cluster.shards.ClusterSearchShardsResponse;
15+
import org.elasticsearch.action.admin.cluster.shards.TransportClusterSearchShardsAction;
1516
import org.elasticsearch.action.search.SearchRequest;
1617
import org.elasticsearch.client.internal.Client;
1718
import org.elasticsearch.cluster.node.DiscoveryNode;
@@ -114,12 +115,14 @@ static <Request extends AbstractBulkByScrollRequest<Request>> void initTaskState
114115
) {
115116
int configuredSlices = request.getSlices();
116117
if (configuredSlices == AbstractBulkByScrollRequest.AUTO_SLICES) {
117-
ClusterSearchShardsRequest shardsRequest = new ClusterSearchShardsRequest();
118-
shardsRequest.indices(request.getSearchRequest().indices());
119-
client.admin().cluster().searchShards(shardsRequest, listener.safeMap(response -> {
120-
setWorkerCount(request, task, countSlicesBasedOnShards(response));
121-
return null;
122-
}));
118+
client.execute(
119+
TransportClusterSearchShardsAction.TYPE,
120+
new ClusterSearchShardsRequest(request.getTimeout(), request.getSearchRequest().indices()),
121+
listener.safeMap(response -> {
122+
setWorkerCount(request, task, countSlicesBasedOnShards(response));
123+
return null;
124+
})
125+
);
123126
} else {
124127
setWorkerCount(request, task, configuredSlices);
125128
listener.onResponse(null);

rest-api-spec/src/main/resources/rest-api-spec/api/search_shards.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@
6565
],
6666
"default":"open",
6767
"description":"Whether to expand wildcard expression to concrete indices that are open, closed or both."
68+
},
69+
"master_timeout":{
70+
"type":"time",
71+
"description":"Explicit operation timeout for connection to master node"
6872
}
6973
}
7074
}

server/src/internalClusterTest/java/org/elasticsearch/action/search/SearchProgressActionListenerIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ private static List<SearchShard> createRandomIndices(Client client) {
202202
ClusterSearchShardsResponse resp = safeExecute(
203203
client,
204204
TransportClusterSearchShardsAction.TYPE,
205-
new ClusterSearchShardsRequest("index-*")
205+
new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "index-*")
206206
);
207207
return Arrays.stream(resp.getGroups()).map(e -> new SearchShard(null, e.getShardId())).sorted().toList();
208208
}

server/src/internalClusterTest/java/org/elasticsearch/action/termvectors/GetTermVectorsIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,7 @@ public void testArtificialDocWithPreference() throws InterruptedException, IOExc
10171017
// Get search shards
10181018
ClusterSearchShardsResponse searchShardsResponse = safeExecute(
10191019
TransportClusterSearchShardsAction.TYPE,
1020-
new ClusterSearchShardsRequest("test")
1020+
new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "test")
10211021
);
10221022
List<Integer> shardIds = Arrays.stream(searchShardsResponse.getGroups()).map(s -> s.getShardId().id()).toList();
10231023

server/src/internalClusterTest/java/org/elasticsearch/cluster/routing/ShardRoutingRoleIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@ public void testSearchRouting() throws Exception {
550550
}
551551
// search-shards API
552552
for (int i = 0; i < 10; i++) {
553-
final var search = new ClusterSearchShardsRequest(INDEX_NAME);
553+
final var search = new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, INDEX_NAME);
554554
switch (randomIntBetween(0, 2)) {
555555
case 0 -> search.routing(randomAlphaOfLength(10));
556556
case 1 -> search.routing(randomSearchPreference(routingTableWatcher.numShards, internalCluster().getNodeNames()));

server/src/internalClusterTest/java/org/elasticsearch/cluster/shards/ClusterSearchShardsIT.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,15 @@ protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
4545
public void testSingleShardAllocation() {
4646
indicesAdmin().prepareCreate("test").setSettings(indexSettings(1, 0).put("index.routing.allocation.include.tag", "A")).get();
4747
ensureGreen();
48-
ClusterSearchShardsResponse response = safeExecute(new ClusterSearchShardsRequest("test"));
48+
ClusterSearchShardsResponse response = safeExecute(new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "test"));
4949
assertThat(response.getGroups().length, equalTo(1));
5050
assertThat(response.getGroups()[0].getShardId().getIndexName(), equalTo("test"));
5151
assertThat(response.getGroups()[0].getShardId().getId(), equalTo(0));
5252
assertThat(response.getGroups()[0].getShards().length, equalTo(1));
5353
assertThat(response.getNodes().length, equalTo(1));
5454
assertThat(response.getGroups()[0].getShards()[0].currentNodeId(), equalTo(response.getNodes()[0].getId()));
5555

56-
response = safeExecute(new ClusterSearchShardsRequest("test").routing("A"));
56+
response = safeExecute(new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "test").routing("A"));
5757
assertThat(response.getGroups().length, equalTo(1));
5858
assertThat(response.getGroups()[0].getShardId().getIndexName(), equalTo("test"));
5959
assertThat(response.getGroups()[0].getShardId().getId(), equalTo(0));
@@ -67,16 +67,16 @@ public void testMultipleShardsSingleNodeAllocation() {
6767
indicesAdmin().prepareCreate("test").setSettings(indexSettings(4, 0).put("index.routing.allocation.include.tag", "A")).get();
6868
ensureGreen();
6969

70-
ClusterSearchShardsResponse response = safeExecute(new ClusterSearchShardsRequest("test"));
70+
ClusterSearchShardsResponse response = safeExecute(new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "test"));
7171
assertThat(response.getGroups().length, equalTo(4));
7272
assertThat(response.getGroups()[0].getShardId().getIndexName(), equalTo("test"));
7373
assertThat(response.getNodes().length, equalTo(1));
7474
assertThat(response.getGroups()[0].getShards()[0].currentNodeId(), equalTo(response.getNodes()[0].getId()));
7575

76-
response = safeExecute(new ClusterSearchShardsRequest("test").routing("ABC"));
76+
response = safeExecute(new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "test").routing("ABC"));
7777
assertThat(response.getGroups().length, equalTo(1));
7878

79-
response = safeExecute(new ClusterSearchShardsRequest("test").preference("_shards:2"));
79+
response = safeExecute(new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "test").preference("_shards:2"));
8080
assertThat(response.getGroups().length, equalTo(1));
8181
assertThat(response.getGroups()[0].getShardId().getId(), equalTo(2));
8282
}
@@ -90,7 +90,7 @@ public void testMultipleIndicesAllocation() {
9090
.get();
9191
clusterAdmin().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForGreenStatus().get();
9292

93-
ClusterSearchShardsResponse response = safeExecute(new ClusterSearchShardsRequest("routing_alias"));
93+
ClusterSearchShardsResponse response = safeExecute(new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "routing_alias"));
9494
assertThat(response.getGroups().length, equalTo(2));
9595
assertThat(response.getGroups()[0].getShards().length, equalTo(2));
9696
assertThat(response.getGroups()[1].getShards().length, equalTo(2));
@@ -132,7 +132,7 @@ public void testClusterSearchShardsWithBlocks() {
132132
)) {
133133
try {
134134
enableIndexBlock("test-blocks", blockSetting);
135-
ClusterSearchShardsResponse response = safeExecute(new ClusterSearchShardsRequest("test-blocks"));
135+
ClusterSearchShardsResponse response = safeExecute(new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "test-blocks"));
136136
assertThat(response.getGroups().length, equalTo(numShards.numPrimaries));
137137
} finally {
138138
disableIndexBlock("test-blocks", blockSetting);
@@ -149,7 +149,11 @@ public void testClusterSearchShardsWithBlocks() {
149149
ExceptionsHelper.unwrapCause(
150150
safeAwaitFailure(
151151
ClusterSearchShardsResponse.class,
152-
l -> client().execute(TransportClusterSearchShardsAction.TYPE, new ClusterSearchShardsRequest("test-blocks"), l)
152+
l -> client().execute(
153+
TransportClusterSearchShardsAction.TYPE,
154+
new ClusterSearchShardsRequest(TEST_REQUEST_TIMEOUT, "test-blocks"),
155+
l
156+
)
153157
)
154158
)
155159
)

server/src/main/java/org/elasticsearch/action/admin/cluster/shards/ClusterSearchShardsRequest.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.common.io.stream.StreamInput;
1717
import org.elasticsearch.common.io.stream.StreamOutput;
1818
import org.elasticsearch.core.Nullable;
19+
import org.elasticsearch.core.TimeValue;
1920

2021
import java.io.IOException;
2122
import java.util.Objects;
@@ -31,22 +32,16 @@ public final class ClusterSearchShardsRequest extends MasterNodeReadRequest<Clus
3132
private String preference;
3233
private IndicesOptions indicesOptions = IndicesOptions.lenientExpandOpen();
3334

34-
public ClusterSearchShardsRequest() {
35-
super(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT);
36-
}
37-
38-
public ClusterSearchShardsRequest(String... indices) {
39-
super(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT);
35+
public ClusterSearchShardsRequest(TimeValue masterNodeTimeout, String... indices) {
36+
super(masterNodeTimeout);
4037
indices(indices);
4138
}
4239

4340
public ClusterSearchShardsRequest(StreamInput in) throws IOException {
4441
super(in);
4542
indices = in.readStringArray();
46-
4743
routing = in.readOptionalString();
4844
preference = in.readOptionalString();
49-
5045
indicesOptions = IndicesOptions.readIndicesOptions(in);
5146
}
5247

@@ -56,7 +51,6 @@ public void writeTo(StreamOutput out) throws IOException {
5651
out.writeStringArray(indices);
5752
out.writeOptionalString(routing);
5853
out.writeOptionalString(preference);
59-
6054
indicesOptions.writeIndicesOptions(out);
6155
}
6256

server/src/main/java/org/elasticsearch/action/admin/cluster/shards/ClusterSearchShardsRequestBuilder.java

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

0 commit comments

Comments
 (0)