Prevent field caps from failing due to can match failure

docs/changelog/134134.yaml

@@ -0,0 +1,6 @@
+pr: 134134
+summary: Prevent field caps from failing due to can match failure
+area: Search
+type: bug
+issues:
+ - 116106

server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/CCSFieldCapabilitiesIT.java

@@ -14,15 +14,12 @@
 import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.search.fieldcaps.FieldCapabilitiesIT.ExceptionOnRewriteQueryBuilder;
-import org.elasticsearch.search.fieldcaps.FieldCapabilitiesIT.ExceptionOnRewriteQueryPlugin;
+import org.elasticsearch.index.shard.IllegalIndexShardStateException;
 import org.elasticsearch.test.AbstractMultiClustersTestCase;
 import org.elasticsearch.transport.RemoteTransportException;
 
-import java.util.ArrayList;
+import java.io.IOException;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.List;
 
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
@@ -46,14 +43,7 @@ protected boolean reuseClusters() {
         return false;
     }
 
-    @Override
-    protected Collection<Class<? extends Plugin>> nodePlugins(String clusterAlias) {
-        final List<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins(clusterAlias));
-        plugins.add(ExceptionOnRewriteQueryPlugin.class);
-        return plugins;
-    }
-
-    public void testFailuresFromRemote() {
+    public void testFailuresFromRemote() throws IOException {
         Settings indexSettings = Settings.builder().put("index.number_of_replicas", 0).build();
         final Client localClient = client(LOCAL_CLUSTER);
         final Client remoteClient = client("remote_cluster");
@@ -71,11 +61,10 @@ public void testFailuresFromRemote() {
         FieldCapabilitiesResponse response = client().prepareFieldCaps("*", "remote_cluster:*").setFields("*").get();
         assertThat(Arrays.asList(response.getIndices()), containsInAnyOrder(localIndex, "remote_cluster:" + remoteErrorIndex));
 
-        // adding an index filter so remote call should fail
-        response = client().prepareFieldCaps("*", "remote_cluster:*")
-            .setFields("*")
-            .setIndexFilter(new ExceptionOnRewriteQueryBuilder())
-            .get();
+        // Closed shards will result to index error because shards must be in readable state
+        FieldCapabilitiesIT.closeShards(cluster("remote_cluster"), remoteErrorIndex);
+
+        response = client().prepareFieldCaps("*", "remote_cluster:*").setFields("*").get();
         assertThat(response.getIndices()[0], equalTo(localIndex));
         assertThat(response.getFailedIndicesCount(), equalTo(1));
         FieldCapabilitiesFailure failure = response.getFailures()
@@ -86,25 +75,22 @@ public void testFailuresFromRemote() {
         Exception ex = failure.getException();
         assertEquals(RemoteTransportException.class, ex.getClass());
         Throwable cause = ExceptionsHelper.unwrapCause(ex);
-        assertEquals(IllegalArgumentException.class, cause.getClass());
-        assertEquals("I throw because I choose to.", cause.getMessage());
+        assertEquals(IllegalIndexShardStateException.class, cause.getClass());
+        assertEquals(
+            "CurrentState[CLOSED] operations only allowed when shard state is one of [POST_RECOVERY, STARTED]",
+            cause.getMessage()
+        );
 
         // if we only query the remote we should get back an exception only
-        ex = expectThrows(
-            IllegalArgumentException.class,
-            client().prepareFieldCaps("remote_cluster:*").setFields("*").setIndexFilter(new ExceptionOnRewriteQueryBuilder())
-        );
-        assertEquals("I throw because I choose to.", ex.getMessage());
+        ex = expectThrows(IllegalIndexShardStateException.class, client().prepareFieldCaps("remote_cluster:*").setFields("*"));
+        assertEquals("CurrentState[CLOSED] operations only allowed when shard state is one of [POST_RECOVERY, STARTED]", ex.getMessage());
 
         // add an index that doesn't fail to the remote
         assertAcked(remoteClient.admin().indices().prepareCreate("okay_remote_index"));
         remoteClient.prepareIndex("okay_remote_index").setId("2").setSource("foo", "bar").get();
         remoteClient.admin().indices().prepareRefresh("okay_remote_index").get();
 
-        response = client().prepareFieldCaps("*", "remote_cluster:*")
-            .setFields("*")
-            .setIndexFilter(new ExceptionOnRewriteQueryBuilder())
-            .get();
+        response = client().prepareFieldCaps("*", "remote_cluster:*").setFields("*").get();
         assertThat(Arrays.asList(response.getIndices()), containsInAnyOrder(localIndex, "remote_cluster:okay_remote_index"));
         assertThat(response.getFailedIndicesCount(), equalTo(1));
         failure = response.getFailures()
@@ -113,8 +99,8 @@ public void testFailuresFromRemote() {
             .findFirst()
             .get();
         ex = failure.getException();
-        assertEquals(IllegalArgumentException.class, ex.getClass());
-        assertEquals("I throw because I choose to.", ex.getMessage());
+        assertEquals(IllegalIndexShardStateException.class, ex.getClass());
+        assertEquals("CurrentState[CLOSED] operations only allowed when shard state is one of [POST_RECOVERY, STARTED]", ex.getMessage());
     }
 
     public void testFailedToConnectToRemoteCluster() throws Exception {

server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java

@@ -47,7 +47,7 @@
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.index.query.QueryRewriteContext;
-import org.elasticsearch.index.query.SearchExecutionContext;
+import org.elasticsearch.index.shard.IllegalIndexShardStateException;
 import org.elasticsearch.index.shard.IndexShard;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.indices.IndexClosedException;
@@ -59,6 +59,7 @@
 import org.elasticsearch.search.DummyQueryBuilder;
 import org.elasticsearch.tasks.TaskInfo;
 import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.test.InternalTestCluster;
 import org.elasticsearch.test.MockLog;
 import org.elasticsearch.test.junit.annotations.TestLogging;
 import org.elasticsearch.test.transport.MockTransportService;
@@ -78,6 +79,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -87,7 +89,6 @@
 import java.util.function.Predicate;
 import java.util.stream.IntStream;
 
-import static java.util.Collections.singletonList;
 import static org.elasticsearch.action.support.ActionTestUtils.wrapAsRestResponseListener;
 import static org.elasticsearch.index.shard.IndexShardTestCase.closeShardNoCheck;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
@@ -193,7 +194,7 @@ public void setUp() throws Exception {
 
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
-        return List.of(TestMapperPlugin.class, ExceptionOnRewriteQueryPlugin.class, BlockingOnRewriteQueryPlugin.class);
+        return List.of(TestMapperPlugin.class, BlockingOnRewriteQueryPlugin.class);
     }
 
     @Override
@@ -445,30 +446,36 @@ public void testFieldMetricsAndDimensions() {
         assertThat(response.get().get("some_dimension").get("keyword").nonDimensionIndices(), array(equalTo("new_index")));
     }
 
-    public void testFailures() throws InterruptedException {
+    public void testFailures() throws IOException {
         // in addition to the existing "old_index" and "new_index", create two where the test query throws an error on rewrite
         assertAcked(prepareCreate("index1-error"), prepareCreate("index2-error"));
         ensureGreen("index1-error", "index2-error");
-        FieldCapabilitiesResponse response = client().prepareFieldCaps()
+
+        // Closed shards will result to index error because shards must be in readable state
+        closeShards(internalCluster(), "index1-error", "index2-error");
+
+        FieldCapabilitiesResponse response = client().prepareFieldCaps("old_index", "new_index", "index1-error", "index2-error")
             .setFields("*")
-            .setIndexFilter(new ExceptionOnRewriteQueryBuilder())
             .get();
         assertEquals(1, response.getFailures().size());
         assertEquals(2, response.getFailedIndicesCount());
         assertThat(response.getFailures().get(0).getIndices(), arrayContainingInAnyOrder("index1-error", "index2-error"));
         Exception failure = response.getFailures().get(0).getException();
-        assertEquals(IllegalArgumentException.class, failure.getClass());
-        assertEquals("I throw because I choose to.", failure.getMessage());
+        assertEquals(IllegalIndexShardStateException.class, failure.getClass());
+        assertEquals(
+            "CurrentState[CLOSED] operations only allowed when shard state is one of [POST_RECOVERY, STARTED]",
+            failure.getMessage()
+        );
 
         // the "indices" section should not include failed ones
         assertThat(Arrays.asList(response.getIndices()), containsInAnyOrder("old_index", "new_index"));
 
         // if all requested indices failed, we fail the request by throwing the exception
-        IllegalArgumentException ex = expectThrows(
-            IllegalArgumentException.class,
-            client().prepareFieldCaps("index1-error", "index2-error").setFields("*").setIndexFilter(new ExceptionOnRewriteQueryBuilder())
+        IllegalIndexShardStateException ex = expectThrows(
+            IllegalIndexShardStateException.class,
+            client().prepareFieldCaps("index1-error", "index2-error").setFields("*")
         );
-        assertEquals("I throw because I choose to.", ex.getMessage());
+        assertEquals("CurrentState[CLOSED] operations only allowed when shard state is one of [POST_RECOVERY, STARTED]", ex.getMessage());
     }
 
     private void populateTimeRangeIndices() throws Exception {
@@ -913,45 +920,17 @@ private void assertIndices(FieldCapabilitiesResponse response, String... indices
         assertArrayEquals(indices, response.getIndices());
     }
 
-    /**
-     * Adds an "exception" query that  throws on rewrite if the index name contains the string "error"
-     */
-    public static class ExceptionOnRewriteQueryPlugin extends Plugin implements SearchPlugin {
-
-        public ExceptionOnRewriteQueryPlugin() {}
-
-        @Override
-        public List<QuerySpec<?>> getQueries() {
-            return singletonList(
-                new QuerySpec<>("exception", ExceptionOnRewriteQueryBuilder::new, p -> new ExceptionOnRewriteQueryBuilder())
-            );
-        }
-    }
-
-    static class ExceptionOnRewriteQueryBuilder extends DummyQueryBuilder {
-
-        public static final String NAME = "exception";
-
-        ExceptionOnRewriteQueryBuilder() {}
-
-        ExceptionOnRewriteQueryBuilder(StreamInput in) throws IOException {
-            super(in);
-        }
-
-        @Override
-        protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException {
-            SearchExecutionContext searchExecutionContext = queryRewriteContext.convertToSearchExecutionContext();
-            if (searchExecutionContext != null) {
-                if (searchExecutionContext.indexMatches("*error*")) {
-                    throw new IllegalArgumentException("I throw because I choose to.");
+    static void closeShards(InternalTestCluster cluster, String... indices) throws IOException {
+        final Set<String> indicesToClose = Set.of(indices);
+        for (String node : cluster.getNodeNames()) {
+            final IndicesService indicesService = cluster.getInstance(IndicesService.class, node);
+            for (IndexService indexService : indicesService) {
+                if (indicesToClose.contains(indexService.getMetadata().getIndex().getName())) {
+                    for (IndexShard indexShard : indexService) {
+                        closeShardNoCheck(indexShard);
+                    }
                 }
             }
-            return this;
-        }
-
-        @Override
-        public String getWriteableName() {
-            return NAME;
         }
     }
 

server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilitiesFetcher.java

@@ -249,12 +249,17 @@ private static boolean canMatchShard(
         QueryBuilder indexFilter,
         long nowInMillis,
         SearchExecutionContext searchExecutionContext
-    ) throws IOException {
+    ) {
         assert alwaysMatches(indexFilter) == false : "should not be called for always matching [" + indexFilter + "]";
         assert nowInMillis != 0L;
         ShardSearchRequest searchRequest = new ShardSearchRequest(shardId, nowInMillis, AliasFilter.EMPTY);
         searchRequest.source(new SearchSourceBuilder().query(indexFilter));
-        return SearchService.queryStillMatchesAfterRewrite(searchRequest, searchExecutionContext);
+        try {
+            return SearchService.queryStillMatchesAfterRewrite(searchRequest, searchExecutionContext);
+        } catch (Exception e) {
+            // treat as if shard is still a potential match
+            return true;
+        }
     }
 
     private static boolean alwaysMatches(QueryBuilder indexFilter) {

x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceFeatures.java

@@ -84,7 +84,8 @@ public Set<NodeFeature> getTestFeatures() {
                 SEMANTIC_TEXT_HIGHLIGHTING_FLAT,
                 SEMANTIC_TEXT_SPARSE_VECTOR_INDEX_OPTIONS,
                 SEMANTIC_TEXT_FIELDS_CHUNKS_FORMAT,
-                SemanticQueryBuilder.SEMANTIC_QUERY_MULTIPLE_INFERENCE_IDS
+                SemanticQueryBuilder.SEMANTIC_QUERY_MULTIPLE_INFERENCE_IDS,
+                SemanticQueryBuilder.SEMANTIC_QUERY_FILTER_FIELD_CAPS_FIX
             )
         );
         if (RERANK_SNIPPETS.isEnabled()) {

x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/queries/SemanticQueryBuilder.java

@@ -56,6 +56,7 @@ public class SemanticQueryBuilder extends AbstractQueryBuilder<SemanticQueryBuil
     public static final String NAME = "semantic";
 
     public static final NodeFeature SEMANTIC_QUERY_MULTIPLE_INFERENCE_IDS = new NodeFeature("semantic_query.multiple_inference_ids");
+    public static final NodeFeature SEMANTIC_QUERY_FILTER_FIELD_CAPS_FIX = new NodeFeature("semantic_query.filter_field_caps_fix");
 
     private static final TransportVersion SEMANTIC_QUERY_MULTIPLE_INFERENCE_IDS_TV = TransportVersion.fromName(
         "semantic_query_multiple_inference_ids"

x-pack/plugin/inference/src/yamlRestTest/resources/rest-api-spec/test/inference/10_semantic_text_field_mapping.yml

@@ -548,6 +548,33 @@ setup:
   - not_exists: fields.dense_field.inference.chunks
   - not_exists: fields.dense_field.inference
 
+---
+"Field caps with semantic query does not fail":
+  - requires:
+      cluster_features: "semantic_query.filter_field_caps_fix"
+      reason: "fixed bug with semantic query filtering in field_caps (#116106)"
+  # We need at least one document present to exercise can-match phase
+  - do:
+      index:
+        index: test-index
+        id: doc_1
+        body:
+          sparse_field: "This is a story about a cat and a dog."
+        refresh: true
+
+  - do:
+      field_caps:
+        index: test-index
+        fields: "*"
+        body:
+          index_filter:
+            semantic:
+              field: "sparse_field"
+              query: "test"
+
+  - match: { indices: [ "test-index" ] }
+  - match: { fields.sparse_field.text.searchable: true }
+
 ---
 "Users can set dense vector index options and index documents using those options":
   - requires:

x-pack/plugin/inference/src/yamlRestTest/resources/rest-api-spec/test/inference/10_semantic_text_field_mapping_bwc.yml

@@ -447,6 +447,34 @@ setup:
   - not_exists: fields.dense_field.inference.chunks.text
   - not_exists: fields.dense_field.inference.chunks
   - not_exists: fields.dense_field.inference
+
+---
+"Field caps with semantic query does not fail":
+  - requires:
+      cluster_features: "semantic_query.filter_field_caps_fix"
+      reason: "fixed bug with semantic query filtering in field_caps (#116106)"
+  # We need at least one document present to exercise can-match phase
+  - do:
+      index:
+        index: test-index
+        id: doc_1
+        body:
+          sparse_field: "This is a story about a cat and a dog."
+        refresh: true
+
+  - do:
+      field_caps:
+        index: test-index
+        fields: "*"
+        body:
+          index_filter:
+            semantic:
+              field: "sparse_field"
+              query: "test"
+
+  - match: { indices: [ "test-index" ] }
+  - match: { fields.sparse_field.text.searchable: true }
+
 ---
 "Users can set dense vector index options and index documents using those options":
   - requires: