Skip search shards with INDEX_REFRESH_BLOCK

server/src/internalClusterTest/java/org/elasticsearch/search/SearchWithIndexBlocksIT.java

@@ -0,0 +1,135 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.search;
+
+import org.elasticsearch.action.admin.indices.readonly.AddIndexBlockRequest;
+import org.elasticsearch.action.admin.indices.readonly.TransportAddIndexBlockAction;
+import org.elasticsearch.action.search.ClosePointInTimeRequest;
+import org.elasticsearch.action.search.OpenPointInTimeRequest;
+import org.elasticsearch.action.search.SearchRequest;
+import org.elasticsearch.action.search.TransportClosePointInTimeAction;
+import org.elasticsearch.action.search.TransportOpenPointInTimeAction;
+import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.core.TimeValue;
+import org.elasticsearch.index.query.QueryBuilders;
+import org.elasticsearch.search.builder.PointInTimeBuilder;
+import org.elasticsearch.search.builder.SearchSourceBuilder;
+import org.elasticsearch.test.ESIntegTestCase;
+
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
+
+public class SearchWithIndexBlocksIT extends ESIntegTestCase {
+
+    public void testSearchIndexWithIndexRefreshBlock() {
+        createIndex("test");
+
+        var addIndexBlockRequest = new AddIndexBlockRequest(IndexMetadata.APIBlock.REFRESH, "test");
+        client().execute(TransportAddIndexBlockAction.TYPE, addIndexBlockRequest).actionGet();
+
+        indexRandom(
+            true,
+            prepareIndex("test").setId("1").setSource("field", "value"),
+            prepareIndex("test").setId("2").setSource("field", "value"),
+            prepareIndex("test").setId("3").setSource("field", "value"),
+            prepareIndex("test").setId("4").setSource("field", "value"),
+            prepareIndex("test").setId("5").setSource("field", "value"),
+            prepareIndex("test").setId("6").setSource("field", "value")
+        );
+
+        assertHitCount(prepareSearch().setQuery(QueryBuilders.matchAllQuery()), 0);
+    }
+
+    public void testSearchMultipleIndicesEachWithAnIndexRefreshBlock() {
+        createIndex("test");
+        createIndex("test2");
+
+        var addIndexBlockRequest = new AddIndexBlockRequest(IndexMetadata.APIBlock.REFRESH, "test", "test2");
+        client().execute(TransportAddIndexBlockAction.TYPE, addIndexBlockRequest).actionGet();
+
+        indexRandom(
+            true,
+            prepareIndex("test").setId("1").setSource("field", "value"),
+            prepareIndex("test").setId("2").setSource("field", "value"),
+            prepareIndex("test").setId("3").setSource("field", "value"),
+            prepareIndex("test").setId("4").setSource("field", "value"),
+            prepareIndex("test").setId("5").setSource("field", "value"),
+            prepareIndex("test").setId("6").setSource("field", "value"),
+            prepareIndex("test2").setId("1").setSource("field", "value"),
+            prepareIndex("test2").setId("2").setSource("field", "value"),
+            prepareIndex("test2").setId("3").setSource("field", "value"),
+            prepareIndex("test2").setId("4").setSource("field", "value"),
+            prepareIndex("test2").setId("5").setSource("field", "value"),
+            prepareIndex("test2").setId("6").setSource("field", "value")
+        );
+
+        assertHitCount(prepareSearch().setQuery(QueryBuilders.matchAllQuery()), 0);
+    }
+
+    public void testSearchMultipleIndicesWithOneIndexRefreshBlock() {
+        createIndex("test");
+        createIndex("test2");
+
+        // Only block test
+        var addIndexBlockRequest = new AddIndexBlockRequest(IndexMetadata.APIBlock.REFRESH, "test");
+        client().execute(TransportAddIndexBlockAction.TYPE, addIndexBlockRequest).actionGet();
+
+        indexRandom(
+            true,
+            prepareIndex("test").setId("1").setSource("field", "value"),
+            prepareIndex("test").setId("2").setSource("field", "value"),
+            prepareIndex("test").setId("3").setSource("field", "value"),
+            prepareIndex("test").setId("4").setSource("field", "value"),
+            prepareIndex("test").setId("5").setSource("field", "value"),
+            prepareIndex("test").setId("6").setSource("field", "value"),
+            prepareIndex("test2").setId("1").setSource("field", "value"),
+            prepareIndex("test2").setId("2").setSource("field", "value"),
+            prepareIndex("test2").setId("3").setSource("field", "value"),
+            prepareIndex("test2").setId("4").setSource("field", "value"),
+            prepareIndex("test2").setId("5").setSource("field", "value"),
+            prepareIndex("test2").setId("6").setSource("field", "value")
+        );
+
+        // We should get test2 results (not blocked)
+        assertHitCount(prepareSearch().setQuery(QueryBuilders.matchAllQuery()), 6);
+    }
+
+    public void testOpenPITWithIndexRefreshBlock() {
+        createIndex("test");
+
+        var addIndexBlockRequest = new AddIndexBlockRequest(IndexMetadata.APIBlock.REFRESH, "test");
+        client().execute(TransportAddIndexBlockAction.TYPE, addIndexBlockRequest).actionGet();
+
+        indexRandom(
+            true,
+            prepareIndex("test").setId("1").setSource("field", "value"),
+            prepareIndex("test").setId("2").setSource("field", "value"),
+            prepareIndex("test").setId("3").setSource("field", "value"),
+            prepareIndex("test").setId("4").setSource("field", "value"),
+            prepareIndex("test").setId("5").setSource("field", "value"),
+            prepareIndex("test").setId("6").setSource("field", "value")
+        );
+
+        BytesReference pitId = null;
+        try {
+            OpenPointInTimeRequest openPITRequest = new OpenPointInTimeRequest("test").keepAlive(TimeValue.timeValueSeconds(10))
+                .allowPartialSearchResults(true);
+            pitId = client().execute(TransportOpenPointInTimeAction.TYPE, openPITRequest).actionGet().getPointInTimeId();
+            SearchRequest searchRequest = new SearchRequest().source(
+                new SearchSourceBuilder().pointInTimeBuilder(new PointInTimeBuilder(pitId).setKeepAlive(TimeValue.timeValueSeconds(10)))
+            );
+            assertHitCount(client().search(searchRequest), 0);
+        } finally {
+            if (pitId != null) {
+                client().execute(TransportClosePointInTimeAction.TYPE, new ClosePointInTimeRequest(pitId)).actionGet();
+            }
+        }
+    }
+}

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

@@ -186,6 +186,12 @@ private void runCoordinatorRewritePhase() {
         assert assertSearchCoordinationThread();
         final List<SearchShardIterator> matchedShardLevelRequests = new ArrayList<>();
         for (SearchShardIterator searchShardIterator : shardsIts) {
+            if (searchShardIterator.prefiltered() == false && searchShardIterator.skip()) {
+                // This implies the iterator was skipped due to an index level block,
+                // not a remote can-match run.
+                continue;
+            }
+
             final CanMatchNodeRequest canMatchNodeRequest = new CanMatchNodeRequest(
                 request,
                 searchShardIterator.getOriginalIndices().indicesOptions(),

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

@@ -41,7 +41,7 @@ public final class SearchShardIterator implements Comparable<SearchShardIterator
 
     /**
      * Creates a {@link SearchShardIterator} instance that iterates over a subset of the given shards
-     * this the a given <code>shardId</code>.
+     * for a given <code>shardId</code>.
      *
      * @param clusterAlias    the alias of the cluster where the shard is located
      * @param shardId         shard id of the group
@@ -54,6 +54,28 @@ public SearchShardIterator(@Nullable String clusterAlias, ShardId shardId, List<
 
     /**
      * Creates a {@link SearchShardIterator} instance that iterates over a subset of the given shards
+     * for a given <code>shardId</code>.
+     *
+     * @param clusterAlias    the alias of the cluster where the shard is located
+     * @param shardId         shard id of the group
+     * @param shards          shards to iterate
+     * @param originalIndices the indices that the search request originally related to (before any rewriting happened)
+     * @param skip            if true, then this group won't have matches (due to an index level block),
+     *                        and it can be safely skipped from the search
+     */
+    public SearchShardIterator(
+        @Nullable String clusterAlias,
+        ShardId shardId,
+        List<ShardRouting> shards,
+        OriginalIndices originalIndices,
+        boolean skip
+    ) {
+        this(clusterAlias, shardId, shards.stream().map(ShardRouting::currentNodeId).toList(), originalIndices, null, null, false, skip);
+    }
+
+    /**
+     * Creates a {@link SearchShardIterator} instance that iterates over a subset of the given shards
+     * for a given <code>shardId</code>.
      *
      * @param clusterAlias           the alias of the cluster where the shard is located
      * @param shardId                shard id of the group
@@ -62,7 +84,8 @@ public SearchShardIterator(@Nullable String clusterAlias, ShardId shardId, List<
      * @param searchContextId        the point-in-time specified for this group if exists
      * @param searchContextKeepAlive the time interval that data nodes should extend the keep alive of the point-in-time
      * @param prefiltered            if true, then this group already executed the can_match phase
-     * @param skip                   if true, then this group won't have matches, and it can be safely skipped from the search
+     * @param skip                   if true, then this group won't have matches (due to can match, or an index level block),
+     *                               and it can be safely skipped from the search
      */
     public SearchShardIterator(
         @Nullable String clusterAlias,
@@ -83,7 +106,6 @@ public SearchShardIterator(
         assert searchContextKeepAlive == null || searchContextId != null;
         this.prefiltered = prefiltered;
         this.skip = skip;
-        assert skip == false || prefiltered : "only prefiltered shards are skip-able";
     }
 
     /**

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

@@ -148,6 +148,9 @@ public class TransportSearchAction extends HandledTransportAction<SearchRequest,
         Property.NodeScope
     );
 
+    // Marker to indicate this index's shards should be skipped in a search
+    private static final OriginalIndices SKIPPED_INDICES = new OriginalIndices(Strings.EMPTY_ARRAY, IndicesOptions.strictExpandOpen());
+
     private final ThreadPool threadPool;
     private final ClusterService clusterService;
     private final TransportService transportService;
@@ -234,6 +237,10 @@ private Map<String, OriginalIndices> buildPerIndexOriginalIndices(
         for (String index : indices) {
             if (hasBlocks) {
                 blocks.indexBlockedRaiseException(projectId, ClusterBlockLevel.READ, index);
+                if (blocks.hasIndexBlock(projectState.projectId(), index, IndexMetadata.INDEX_REFRESH_BLOCK)) {
+                    res.put(index, SKIPPED_INDICES);
+                    continue;
+                }
             }
 
             String[] aliases = indexNameExpressionResolver.allIndexAliases(projectState.metadata(), index, indicesAndAliases);
@@ -589,7 +596,7 @@ public void onFailure(Exception e) {}
         );
     }
 
-    static void adjustSearchType(SearchRequest searchRequest, boolean singleShard) {
+    static void adjustSearchType(SearchRequest searchRequest, boolean oneOrZeroShardsToSearch) {
         // if there's a kNN search, always use DFS_QUERY_THEN_FETCH
         if (searchRequest.hasKnnSearch()) {
             searchRequest.searchType(DFS_QUERY_THEN_FETCH);
@@ -604,7 +611,7 @@ static void adjustSearchType(SearchRequest searchRequest, boolean singleShard) {
         }
 
         // optimize search type for cases where there is only one shard group to search on
-        if (singleShard) {
+        if (oneOrZeroShardsToSearch) {
             // if we only have one group, then we always want Q_T_F, no need for DFS, and no need to do THEN since we hit one shard
             searchRequest.searchType(QUERY_THEN_FETCH);
         }
@@ -1305,7 +1312,7 @@ private void executeSearch(
 
         Map<String, Float> concreteIndexBoosts = resolveIndexBoosts(searchRequest, projectState.cluster());
 
-        adjustSearchType(searchRequest, shardIterators.size() == 1);
+        adjustSearchType(searchRequest, oneOrZeroShardsToSearch(shardIterators));
 
         final DiscoveryNodes nodes = projectState.cluster().nodes();
         BiFunction<String, String, Transport.Connection> connectionLookup = buildConnectionLookup(
@@ -1338,6 +1345,33 @@ private void executeSearch(
         );
     }
 
+    /**
+     * Determines if only one (or zero) search shard iterators will be searched.
+     * (At this point, iterators may be marked as skipped due to index level blockers).
+     * We expect skipped iterators to be unlikely, so returning fast after we see more
+     * than one "not skipped" is an intended optimization.
+     *
+     * @param searchShardIterators all the shard iterators derived from indices being searched
+     * @return true if there are no more than one shard iterators, or if there are no more than
+     * one not marked to skip
+     */
+    private boolean oneOrZeroShardsToSearch(List<SearchShardIterator> searchShardIterators) {
+        if (searchShardIterators.size() <= 1) {
+            return true;
+        }
+
+        int notSkippedCount = 0;
+        for (SearchShardIterator searchShardIterator : searchShardIterators) {
+            if (searchShardIterator.skip() == false) {
+                notSkippedCount++;
+                if (notSkippedCount > 1) {
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+
     Executor asyncSearchExecutor(final String[] indices) {
         boolean seenSystem = false;
         boolean seenCritical = false;
@@ -1890,7 +1924,13 @@ List<SearchShardIterator> getLocalShardsIterator(
             final ShardId shardId = shardRouting.shardId();
             OriginalIndices finalIndices = originalIndices.get(shardId.getIndex().getName());
             assert finalIndices != null;
-            list[i++] = new SearchShardIterator(clusterAlias, shardId, shardRouting.getShardRoutings(), finalIndices);
+            list[i++] = new SearchShardIterator(
+                clusterAlias,
+                shardId,
+                shardRouting.getShardRoutings(),
+                finalIndices,
+                finalIndices == SKIPPED_INDICES
+            );
         }
         // the returned list must support in-place sorting, so this is the most memory efficient we can do here
         return Arrays.asList(list);

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java

@@ -283,7 +283,8 @@ public enum APIBlock implements Writeable {
         READ("read", INDEX_READ_BLOCK, Property.ServerlessPublic),
         WRITE("write", INDEX_WRITE_BLOCK, Property.ServerlessPublic),
         METADATA("metadata", INDEX_METADATA_BLOCK, Property.ServerlessPublic),
-        READ_ONLY_ALLOW_DELETE("read_only_allow_delete", INDEX_READ_ONLY_ALLOW_DELETE_BLOCK);
+        READ_ONLY_ALLOW_DELETE("read_only_allow_delete", INDEX_READ_ONLY_ALLOW_DELETE_BLOCK),
+        REFRESH("refresh", INDEX_REFRESH_BLOCK);
 
         final String name;
         final String settingName;