Skip to content

Commit 1ed776f

Browse files
authored
Simplify logic around missing shards check in search phases (elastic#122817) (elastic#122916)
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 becd0c0 commit 1ed776f

File tree

5 files changed

+6
-16
lines changed

5 files changed

+6
-16
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/SearchPhase.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,13 @@ public void start() {
4343
}
4444
}
4545

46-
protected String missingShardsErrorMessage(StringBuilder missingShards) {
46+
private static String makeMissingShardsError(StringBuilder missingShards) {
4747
return "Search rejected due to missing shards ["
4848
+ missingShards
4949
+ "]. Consider using `allow_partial_search_results` setting to bypass this error.";
5050
}
5151

52-
protected void doCheckNoMissingShards(String phaseName, SearchRequest request, List<SearchShardIterator> shardsIts) {
52+
protected static void doCheckNoMissingShards(String phaseName, SearchRequest request, List<SearchShardIterator> shardsIts) {
5353
assert request.allowPartialSearchResults() != null : "SearchRequest missing setting for allowPartialSearchResults";
5454
if (request.allowPartialSearchResults() == false) {
5555
final StringBuilder missingShards = new StringBuilder();
@@ -65,7 +65,7 @@ protected void doCheckNoMissingShards(String phaseName, SearchRequest request, L
6565
}
6666
if (missingShards.isEmpty() == false) {
6767
// Status red - shard is missing all copies and would produce partial results for an index search
68-
final String msg = missingShardsErrorMessage(missingShards);
68+
final String msg = makeMissingShardsError(missingShards);
6969
throw new SearchPhaseExecutionException(phaseName, msg, null, ShardSearchFailure.EMPTY_ARRAY);
7070
}
7171
}

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -241,13 +241,6 @@ SearchPhase openPointInTimePhase(
241241
searchRequest.getMaxConcurrentShardRequests(),
242242
clusters
243243
) {
244-
@Override
245-
protected String missingShardsErrorMessage(StringBuilder missingShards) {
246-
return "[open_point_in_time] action requires all shards to be available. Missing shards: ["
247-
+ missingShards
248-
+ "]. Consider using `allow_partial_search_results` setting to bypass this error.";
249-
}
250-
251244
@Override
252245
protected void executePhaseOnShard(
253246
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)