Skip to content

Commit 80b7879

Browse files
authored
Simplify logic around missing shards check in search phases (#122817)
This removes a couple of indirections: the error message for missing shards is always the same no matter the search phase. This was required to provide a slightly different error message for open PIT. The previous error was misleading when open PIT did not support setting allow_partial_search_results, but now that it does, it looks like we can unify the error message and simplify the code around it.
1 parent 29cbbd9 commit 80b7879

File tree

6 files changed

+7
-30
lines changed

6 files changed

+7
-30
lines changed

server/src/internalClusterTest/java/org/elasticsearch/search/retriever/RetrieverRewriteIT.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,7 @@ public void testRewriteCompoundRetrieverShouldThrowForPartialResults() throws Ex
127127
SearchPhaseExecutionException.class,
128128
client().prepareSearch(testIndex).setSource(source)::get
129129
);
130-
assertThat(
131-
ex.getDetailedMessage(),
132-
containsString("[open_point_in_time] action requires all shards to be available. Missing shards")
133-
);
130+
assertThat(ex.getDetailedMessage(), containsString("Search rejected due to missing shards"));
134131
} finally {
135132
internalCluster().restartNode(randomDataNode);
136133
}

server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ private synchronized void consumeResult(int shardIndex, boolean canMatch, MinAnd
201201

202202
private void checkNoMissingShards(List<SearchShardIterator> shards) {
203203
assert assertSearchCoordinationThread();
204-
SearchPhase.doCheckNoMissingShards("can_match", request, shards, SearchPhase::makeMissingShardsError);
204+
SearchPhase.doCheckNoMissingShards("can_match", request, shards);
205205
}
206206

207207
private Map<SendingTarget, List<SearchShardIterator>> groupByNode(List<SearchShardIterator> shards) {

server/src/main/java/org/elasticsearch/action/search/SearchPhase.java

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
import java.util.List;
1616
import java.util.Objects;
17-
import java.util.function.Function;
1817

1918
/**
2019
* Base class for all individual search phases like collecting distributed frequencies, fetching documents, querying shards.
@@ -35,26 +34,13 @@ public String getName() {
3534
return name;
3635
}
3736

38-
protected String missingShardsErrorMessage(StringBuilder missingShards) {
39-
return makeMissingShardsError(missingShards);
40-
}
41-
42-
protected static String makeMissingShardsError(StringBuilder missingShards) {
37+
private static String makeMissingShardsError(StringBuilder missingShards) {
4338
return "Search rejected due to missing shards ["
4439
+ missingShards
4540
+ "]. Consider using `allow_partial_search_results` setting to bypass this error.";
4641
}
4742

48-
protected void doCheckNoMissingShards(String phaseName, SearchRequest request, List<SearchShardIterator> shardsIts) {
49-
doCheckNoMissingShards(phaseName, request, shardsIts, this::missingShardsErrorMessage);
50-
}
51-
52-
protected static void doCheckNoMissingShards(
53-
String phaseName,
54-
SearchRequest request,
55-
List<SearchShardIterator> shardsIts,
56-
Function<StringBuilder, String> makeErrorMessage
57-
) {
43+
protected static void doCheckNoMissingShards(String phaseName, SearchRequest request, List<SearchShardIterator> shardsIts) {
5844
assert request.allowPartialSearchResults() != null : "SearchRequest missing setting for allowPartialSearchResults";
5945
if (request.allowPartialSearchResults() == false) {
6046
final StringBuilder missingShards = new StringBuilder();
@@ -70,7 +56,7 @@ protected static void doCheckNoMissingShards(
7056
}
7157
if (missingShards.isEmpty() == false) {
7258
// Status red - shard is missing all copies and would produce partial results for an index search
73-
final String msg = makeErrorMessage.apply(missingShards);
59+
final String msg = makeMissingShardsError(missingShards);
7460
throw new SearchPhaseExecutionException(phaseName, msg, null, ShardSearchFailure.EMPTY_ARRAY);
7561
}
7662
}

server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,6 @@ void runOpenPointInTimePhase(
241241
searchRequest.getMaxConcurrentShardRequests(),
242242
clusters
243243
) {
244-
protected String missingShardsErrorMessage(StringBuilder missingShards) {
245-
return "[open_point_in_time] action requires all shards to be available. Missing shards: ["
246-
+ missingShards
247-
+ "]. Consider using `allow_partial_search_results` setting to bypass this error.";
248-
}
249-
250244
@Override
251245
protected void executePhaseOnShard(
252246
SearchShardIterator shardIt,

x-pack/plugin/sql/qa/jdbc/single-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/jdbc/single_node/JdbcShardFailureIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public void createTestIndex() throws IOException {
7676
public void testPartialResponseHandling() throws SQLException {
7777
try (Connection c = esJdbc(); Statement s = c.createStatement()) {
7878
SQLException exception = expectThrows(SQLException.class, () -> s.executeQuery("SELECT * FROM test ORDER BY test_field ASC"));
79-
assertThat(exception.getMessage(), containsString("[open_point_in_time] action requires all shards to be available"));
79+
assertThat(exception.getMessage(), containsString("Search rejected due to missing shards"));
8080
}
8181
}
8282
}

x-pack/plugin/sql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/sql/qa/single_node/JdbcShardFailureIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ public void testPartialResponseHandling() throws Exception {
8989
createTestIndex();
9090
try (Connection c = esJdbc(); Statement s = c.createStatement()) {
9191
SQLException exception = expectThrows(SQLException.class, () -> s.executeQuery("SELECT * FROM test ORDER BY test_field ASC"));
92-
assertThat(exception.getMessage(), containsString("[open_point_in_time] action requires all shards to be available"));
92+
assertThat(exception.getMessage(), containsString("Search rejected due to missing shards"));
9393
}
9494
}
9595

0 commit comments

Comments
 (0)