Преглед на файлове

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.
Luca Cavanna преди 8 месеца
родител
ревизия
80b7879eda

+ 1 - 4
server/src/internalClusterTest/java/org/elasticsearch/search/retriever/RetrieverRewriteIT.java

@@ -127,10 +127,7 @@ public class RetrieverRewriteIT extends ESIntegTestCase {
                 SearchPhaseExecutionException.class,
                 client().prepareSearch(testIndex).setSource(source)::get
             );
-            assertThat(
-                ex.getDetailedMessage(),
-                containsString("[open_point_in_time] action requires all shards to be available. Missing shards")
-            );
+            assertThat(ex.getDetailedMessage(), containsString("Search rejected due to missing shards"));
         } finally {
             internalCluster().restartNode(randomDataNode);
         }

+ 1 - 1
server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java

@@ -201,7 +201,7 @@ final class CanMatchPreFilterSearchPhase {
 
     private void checkNoMissingShards(List<SearchShardIterator> shards) {
         assert assertSearchCoordinationThread();
-        SearchPhase.doCheckNoMissingShards("can_match", request, shards, SearchPhase::makeMissingShardsError);
+        SearchPhase.doCheckNoMissingShards("can_match", request, shards);
     }
 
     private Map<SendingTarget, List<SearchShardIterator>> groupByNode(List<SearchShardIterator> shards) {

+ 3 - 17
server/src/main/java/org/elasticsearch/action/search/SearchPhase.java

@@ -14,7 +14,6 @@ import org.elasticsearch.transport.Transport;
 
 import java.util.List;
 import java.util.Objects;
-import java.util.function.Function;
 
 /**
  * Base class for all individual search phases like collecting distributed frequencies, fetching documents, querying shards.
@@ -35,26 +34,13 @@ abstract class SearchPhase {
         return name;
     }
 
-    protected String missingShardsErrorMessage(StringBuilder missingShards) {
-        return makeMissingShardsError(missingShards);
-    }
-
-    protected static String makeMissingShardsError(StringBuilder missingShards) {
+    private static String makeMissingShardsError(StringBuilder missingShards) {
         return "Search rejected due to missing shards ["
             + missingShards
             + "]. Consider using `allow_partial_search_results` setting to bypass this error.";
     }
 
-    protected void doCheckNoMissingShards(String phaseName, SearchRequest request, List<SearchShardIterator> shardsIts) {
-        doCheckNoMissingShards(phaseName, request, shardsIts, this::missingShardsErrorMessage);
-    }
-
-    protected static void doCheckNoMissingShards(
-        String phaseName,
-        SearchRequest request,
-        List<SearchShardIterator> shardsIts,
-        Function<StringBuilder, String> makeErrorMessage
-    ) {
+    protected static void doCheckNoMissingShards(String phaseName, SearchRequest request, List<SearchShardIterator> shardsIts) {
         assert request.allowPartialSearchResults() != null : "SearchRequest missing setting for allowPartialSearchResults";
         if (request.allowPartialSearchResults() == false) {
             final StringBuilder missingShards = new StringBuilder();
@@ -70,7 +56,7 @@ abstract class SearchPhase {
             }
             if (missingShards.isEmpty() == false) {
                 // Status red - shard is missing all copies and would produce partial results for an index search
-                final String msg = makeErrorMessage.apply(missingShards);
+                final String msg = makeMissingShardsError(missingShards);
                 throw new SearchPhaseExecutionException(phaseName, msg, null, ShardSearchFailure.EMPTY_ARRAY);
             }
         }

+ 0 - 6
server/src/main/java/org/elasticsearch/action/search/TransportOpenPointInTimeAction.java

@@ -241,12 +241,6 @@ public class TransportOpenPointInTimeAction extends HandledTransportAction<OpenP
                 searchRequest.getMaxConcurrentShardRequests(),
                 clusters
             ) {
-                protected String missingShardsErrorMessage(StringBuilder missingShards) {
-                    return "[open_point_in_time] action requires all shards to be available. Missing shards: ["
-                        + missingShards
-                        + "].  Consider using `allow_partial_search_results` setting to bypass this error.";
-                }
-
                 @Override
                 protected void executePhaseOnShard(
                     SearchShardIterator shardIt,

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

@@ -76,7 +76,7 @@ public class JdbcShardFailureIT extends JdbcIntegrationTestCase {
     public void testPartialResponseHandling() throws SQLException {
         try (Connection c = esJdbc(); Statement s = c.createStatement()) {
             SQLException exception = expectThrows(SQLException.class, () -> s.executeQuery("SELECT * FROM test ORDER BY test_field ASC"));
-            assertThat(exception.getMessage(), containsString("[open_point_in_time] action requires all shards to be available"));
+            assertThat(exception.getMessage(), containsString("Search rejected due to missing shards"));
         }
     }
 }

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

@@ -89,7 +89,7 @@ public class JdbcShardFailureIT extends JdbcIntegrationTestCase {
         createTestIndex();
         try (Connection c = esJdbc(); Statement s = c.createStatement()) {
             SQLException exception = expectThrows(SQLException.class, () -> s.executeQuery("SELECT * FROM test ORDER BY test_field ASC"));
-            assertThat(exception.getMessage(), containsString("[open_point_in_time] action requires all shards to be available"));
+            assertThat(exception.getMessage(), containsString("Search rejected due to missing shards"));
         }
     }