From 22fd223b1a40d7c6a1dfe56051824ae1846f4cc5 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Mon, 11 Nov 2024 10:24:59 +0530 Subject: [PATCH 1/8] Fixing _list/shards API for closed indices Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsActionIT.java | 72 +++++++++++++++++++ .../shards/TransportCatShardsAction.java | 5 +- .../pagination/ShardPaginationStrategy.java | 50 ++++++------- .../ShardPaginationStrategyTests.java | 69 ++++++++++++++++++ 4 files changed, 171 insertions(+), 25 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java index 32d5b3db85629..c94da0b8eaa35 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java @@ -8,7 +8,10 @@ package org.opensearch.action.admin.cluster.shards; +import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.action.pagination.PageParams; +import org.opensearch.client.Requests; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.common.settings.Settings; @@ -22,10 +25,12 @@ import java.io.IOException; import java.util.List; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import static org.opensearch.cluster.routing.UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING; import static org.opensearch.common.unit.TimeValue.timeValueMillis; import static org.opensearch.search.SearchService.NO_TIMEOUT; +import static org.hamcrest.Matchers.equalTo; @OpenSearchIntegTestCase.ClusterScope(numDataNodes = 0, scope = OpenSearchIntegTestCase.Scope.TEST) public class TransportCatShardsActionIT extends OpenSearchIntegTestCase { @@ -125,4 +130,71 @@ public void onFailure(Exception e) { latch.await(); } + public void testCatShardsSuccessWithPaginationWithClosedIndices() throws InterruptedException, ExecutionException { + internalCluster().startClusterManagerOnlyNodes(1); + List nodes = internalCluster().startDataOnlyNodes(3); + final int numIndices = 3; + final int numShards = 5; + final int numReplicas = 2; + final int pageSize = numIndices * numReplicas * (numShards + 1); + createIndex( + "test-1", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 5) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2) + .put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "60m") + .build() + ); + createIndex( + "test-2", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 5) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2) + .put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "60m") + .build() + ); + createIndex( + "test-3", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2) + .put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "60m") + .build() + ); + ensureGreen(); + + // close index test-3 + client().admin().indices().close(Requests.closeIndexRequest("test-3")).get(); + + ClusterStateResponse clusterStateResponse = client().admin() + .cluster() + .prepareState() + .clear() + .setMetadata(true) + .setIndices("test-3") + .get(); + assertThat(clusterStateResponse.getState().metadata().index("test-3").getState(), equalTo(IndexMetadata.State.CLOSE)); + + final CatShardsRequest shardsRequest = new CatShardsRequest(); + shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); + shardsRequest.setIndices(Strings.EMPTY_ARRAY); + shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); + CountDownLatch latch = new CountDownLatch(1); + client().execute(CatShardsAction.INSTANCE, shardsRequest, new ActionListener() { + @Override + public void onResponse(CatShardsResponse catShardsResponse) { + List shardRoutings = catShardsResponse.getResponseShards(); + assertFalse(shardRoutings.stream().anyMatch(shard -> shard.getIndexName().equals("test-3"))); + latch.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail(); + latch.countDown(); + } + }); + latch.await(); + } + } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index 7b36b7a10f4f2..17243a6d5cce2 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -16,6 +16,7 @@ import org.opensearch.action.pagination.ShardPaginationStrategy; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; +import org.opensearch.action.support.IndicesOptions; import org.opensearch.action.support.TimeoutTaskCancellationUtility; import org.opensearch.client.node.NodeClient; import org.opensearch.common.breaker.ResponseLimitBreachedException; @@ -148,7 +149,9 @@ public void onFailure(Exception e) { } private ShardPaginationStrategy getPaginationStrategy(PageParams pageParams, ClusterStateResponse clusterStateResponse) { - return Objects.isNull(pageParams) ? null : new ShardPaginationStrategy(pageParams, clusterStateResponse.getState()); + return Objects.isNull(pageParams) + ? null + : new ShardPaginationStrategy(pageParams, clusterStateResponse.getState(), IndicesOptions.strictExpandOpenAndForbidClosed()); } private void validateRequestLimit( diff --git a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java index 1eb364c883e60..3fb0821d39224 100644 --- a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java @@ -9,6 +9,7 @@ package org.opensearch.action.pagination; import org.opensearch.OpenSearchParseException; +import org.opensearch.action.support.IndicesOptions; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.routing.IndexRoutingTable; @@ -37,14 +38,23 @@ public class ShardPaginationStrategy implements PaginationStrategy private PageData pageData; public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState) { + this(pageParams, clusterState, null); + } + + public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState, IndicesOptions indicesOptions) { ShardStrategyToken shardStrategyToken = getShardStrategyToken(pageParams.getRequestedToken()); // Get list of indices metadata sorted by their creation time and filtered by the last sent index - List filteredIndices = getEligibleIndices( + List filteredIndices = PaginationStrategy.getSortedIndexMetadata( clusterState, - pageParams.getSort(), - Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexName, - Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexCreationTime + getMetadataFilter( + pageParams.getSort(), + Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexName, + Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexCreationTime, + indicesOptions + ), + PageParams.PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) ? ASC_COMPARATOR : DESC_COMPARATOR ); + // Get the list of shards and indices belonging to current page. this.pageData = getPageData( filteredIndices, @@ -54,39 +64,31 @@ public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState) ); } - private static List getEligibleIndices( - ClusterState clusterState, + private static Predicate getMetadataFilter( String sortOrder, String lastIndexName, - Long lastIndexCreationTime + Long lastIndexCreationTime, + IndicesOptions indicesOptions ) { if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { - return PaginationStrategy.getSortedIndexMetadata( - clusterState, - PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR - ); - } else { - return PaginationStrategy.getSortedIndexMetadata( - clusterState, - getMetadataFilter(sortOrder, lastIndexName, lastIndexCreationTime), - PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR - ); - } - } - - private static Predicate getMetadataFilter(String sortOrder, String lastIndexName, Long lastIndexCreationTime) { - if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { - return indexMetadata -> true; + return indexStateFilter(indicesOptions); } return indexNameFilter(lastIndexName).or( IndexPaginationStrategy.getIndexCreateTimeFilter(sortOrder, lastIndexName, lastIndexCreationTime) - ); + ).and(indexStateFilter(indicesOptions)); } private static Predicate indexNameFilter(String lastIndexName) { return metadata -> metadata.getIndex().getName().equals(lastIndexName); } + private static Predicate indexStateFilter(IndicesOptions indicesOptions) { + if (Objects.isNull(indicesOptions) || !indicesOptions.forbidClosedIndices()) { + return metadata -> true; + } + return metadata -> metadata.getState().equals(IndexMetadata.State.OPEN); + } + /** * Will be used to get the list of shards and respective indices to which they belong, * which are to be displayed in a page. diff --git a/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java b/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java index aed7315660378..4da13e26ba100 100644 --- a/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java +++ b/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java @@ -10,6 +10,7 @@ import org.opensearch.OpenSearchParseException; import org.opensearch.Version; +import org.opensearch.action.support.IndicesOptions; import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; @@ -393,6 +394,45 @@ public void testRetrieveShardsWhenLastIndexGetsDeletedAndReCreated() { assertNull(strategy.getResponseToken().getNextToken()); } + /** + * Validates strategy filters out CLOSED indices, if forbidClosed() indices options are provided. + */ + public void testNoClosedIndicesReturnedByStrategy() { + final int pageSize = DEFAULT_NUMBER_OF_SHARDS * (DEFAULT_NUMBER_OF_REPLICAS + 1); + ClusterState clusterState = getRandomClusterState(List.of(0, 1, 2, 3, 4, 5)); + // Add 2 closed indices to cluster state + clusterState = addIndexToClusterState( + clusterState, + 6, + DEFAULT_NUMBER_OF_SHARDS, + DEFAULT_NUMBER_OF_REPLICAS, + IndexMetadata.State.CLOSE + ); + clusterState = addIndexToClusterState( + clusterState, + 7, + DEFAULT_NUMBER_OF_SHARDS, + DEFAULT_NUMBER_OF_REPLICAS, + IndexMetadata.State.CLOSE + ); + List shardRoutings = new ArrayList<>(); + List indices = new ArrayList<>(); + String requestedToken = null; + ShardPaginationStrategy strategy; + do { + PageParams pageParams = new PageParams(requestedToken, PARAM_ASC_SORT_VALUE, pageSize); + strategy = new ShardPaginationStrategy(pageParams, clusterState, IndicesOptions.strictExpandOpenAndForbidClosed()); + requestedToken = strategy.getResponseToken().getNextToken(); + shardRoutings.addAll(strategy.getRequestedEntities()); + indices.addAll(strategy.getRequestedIndices()); + } while (requestedToken != null); + // assert that the closed indices do not appear in the response + assertFalse(indices.contains(TEST_INDEX_PREFIX + 6)); + assertFalse(shardRoutings.stream().anyMatch(shard -> shard.getIndexName().equals(TEST_INDEX_PREFIX + 6))); + assertFalse(indices.contains(TEST_INDEX_PREFIX + 7)); + assertFalse(shardRoutings.stream().anyMatch(shard -> shard.getIndexName().equals(TEST_INDEX_PREFIX + 7))); + } + public void testCreatingShardStrategyPageTokenWithRequestedTokenNull() { try { new ShardPaginationStrategy.ShardStrategyToken(null); @@ -478,11 +518,40 @@ private ClusterState addIndexToClusterState( final int numShards, final int numReplicas, final long creationTime + ) { + return addIndexToClusterState(clusterState, indexNumber, numShards, numReplicas, creationTime, IndexMetadata.State.OPEN); + } + + private ClusterState addIndexToClusterState( + ClusterState clusterState, + final int indexNumber, + final int numShards, + final int numReplicas, + final IndexMetadata.State state + ) { + return addIndexToClusterState( + clusterState, + indexNumber, + numShards, + numReplicas, + Instant.now().plus(indexNumber, ChronoUnit.SECONDS).toEpochMilli(), + state + ); + } + + private ClusterState addIndexToClusterState( + ClusterState clusterState, + final int indexNumber, + final int numShards, + final int numReplicas, + final long creationTime, + final IndexMetadata.State state ) { IndexMetadata indexMetadata = IndexMetadata.builder(TEST_INDEX_PREFIX + indexNumber) .settings(settings(Version.CURRENT).put(SETTING_CREATION_DATE, creationTime)) .numberOfShards(numShards) .numberOfReplicas(numReplicas) + .state(state) .build(); IndexRoutingTable.Builder indexRoutingTableBuilder = new IndexRoutingTable.Builder(indexMetadata.getIndex()).initializeAsNew( indexMetadata From 4663f2b8cd08f3b79508c7d81a9a77bab890f6ca Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Wed, 13 Nov 2024 11:29:10 +0530 Subject: [PATCH 2/8] Added changeLog Signed-off-by: Harsh Garg --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e46628249c91e..53ae14a2dcf28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Support retrieving doc values of unsigned long field ([#16543](https://github.com/opensearch-project/OpenSearch/pull/16543)) - Fix rollover alias supports restored searchable snapshot index([#16483](https://github.com/opensearch-project/OpenSearch/pull/16483)) - Fix permissions error on scripted query against remote snapshot ([#16544](https://github.com/opensearch-project/OpenSearch/pull/16544)) +- Fix _list/shards API failing when closed indices are present in a cluster ([#16606](https://github.com/opensearch-project/OpenSearch/pull/16606)) ### Security From b6a8c8bfebf33b52d432f24f15698ff8afec1a59 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Tue, 19 Nov 2024 12:10:57 +0530 Subject: [PATCH 3/8] Changing indicesOptions to lenient for _list/shards Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsActionIT.java | 54 ++------------- .../shards/TransportCatShardsAction.java | 9 ++- .../pagination/ShardPaginationStrategy.java | 50 +++++++------- .../action/support/IndicesOptions.java | 13 ++++ .../ShardPaginationStrategyTests.java | 69 ------------------- 5 files changed, 50 insertions(+), 145 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java index c94da0b8eaa35..460b6b8629e42 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java @@ -8,12 +8,11 @@ package org.opensearch.action.admin.cluster.shards; -import org.opensearch.action.admin.cluster.state.ClusterStateResponse; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; -import org.opensearch.action.pagination.PageParams; import org.opensearch.client.Requests; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.routing.ShardRouting; +import org.opensearch.common.action.ActionFuture; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; @@ -30,7 +29,6 @@ import static org.opensearch.cluster.routing.UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING; import static org.opensearch.common.unit.TimeValue.timeValueMillis; import static org.opensearch.search.SearchService.NO_TIMEOUT; -import static org.hamcrest.Matchers.equalTo; @OpenSearchIntegTestCase.ClusterScope(numDataNodes = 0, scope = OpenSearchIntegTestCase.Scope.TEST) public class TransportCatShardsActionIT extends OpenSearchIntegTestCase { @@ -132,26 +130,14 @@ public void onFailure(Exception e) { public void testCatShardsSuccessWithPaginationWithClosedIndices() throws InterruptedException, ExecutionException { internalCluster().startClusterManagerOnlyNodes(1); - List nodes = internalCluster().startDataOnlyNodes(3); - final int numIndices = 3; - final int numShards = 5; - final int numReplicas = 2; - final int pageSize = numIndices * numReplicas * (numShards + 1); + internalCluster().startDataOnlyNodes(3); createIndex( - "test-1", - Settings.builder() - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 5) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2) - .put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "60m") - .build() + "test", + Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2).build() ); createIndex( "test-2", - Settings.builder() - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 5) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2) - .put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "60m") - .build() + Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2).build() ); createIndex( "test-3", @@ -162,39 +148,13 @@ public void testCatShardsSuccessWithPaginationWithClosedIndices() throws Interru .build() ); ensureGreen(); - // close index test-3 client().admin().indices().close(Requests.closeIndexRequest("test-3")).get(); - - ClusterStateResponse clusterStateResponse = client().admin() - .cluster() - .prepareState() - .clear() - .setMetadata(true) - .setIndices("test-3") - .get(); - assertThat(clusterStateResponse.getState().metadata().index("test-3").getState(), equalTo(IndexMetadata.State.CLOSE)); - final CatShardsRequest shardsRequest = new CatShardsRequest(); shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); shardsRequest.setIndices(Strings.EMPTY_ARRAY); - shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); - CountDownLatch latch = new CountDownLatch(1); - client().execute(CatShardsAction.INSTANCE, shardsRequest, new ActionListener() { - @Override - public void onResponse(CatShardsResponse catShardsResponse) { - List shardRoutings = catShardsResponse.getResponseShards(); - assertFalse(shardRoutings.stream().anyMatch(shard -> shard.getIndexName().equals("test-3"))); - latch.countDown(); - } - - @Override - public void onFailure(Exception e) { - fail(); - latch.countDown(); - } - }); - latch.await(); + ActionFuture response = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertTrue(response.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-3"))); } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index 17243a6d5cce2..122b4ab3f3269 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -116,6 +116,11 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { return; } IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest(); + if (paginationStrategy != null) { + // Use lenient indices options for paginated queries, which would silently + // ignore closed concrete indices for fetching stats instead of throwing out an error. + indicesStatsRequest.indicesOptions(IndicesOptions.lenientExpandOpenAndForbidClosed()); + } indicesStatsRequest.setShouldCancelOnTimeout(true); indicesStatsRequest.all(); indicesStatsRequest.indices(indices); @@ -149,9 +154,7 @@ public void onFailure(Exception e) { } private ShardPaginationStrategy getPaginationStrategy(PageParams pageParams, ClusterStateResponse clusterStateResponse) { - return Objects.isNull(pageParams) - ? null - : new ShardPaginationStrategy(pageParams, clusterStateResponse.getState(), IndicesOptions.strictExpandOpenAndForbidClosed()); + return Objects.isNull(pageParams) ? null : new ShardPaginationStrategy(pageParams, clusterStateResponse.getState()); } private void validateRequestLimit( diff --git a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java index 3fb0821d39224..1eb364c883e60 100644 --- a/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java +++ b/server/src/main/java/org/opensearch/action/pagination/ShardPaginationStrategy.java @@ -9,7 +9,6 @@ package org.opensearch.action.pagination; import org.opensearch.OpenSearchParseException; -import org.opensearch.action.support.IndicesOptions; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.routing.IndexRoutingTable; @@ -38,23 +37,14 @@ public class ShardPaginationStrategy implements PaginationStrategy private PageData pageData; public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState) { - this(pageParams, clusterState, null); - } - - public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState, IndicesOptions indicesOptions) { ShardStrategyToken shardStrategyToken = getShardStrategyToken(pageParams.getRequestedToken()); // Get list of indices metadata sorted by their creation time and filtered by the last sent index - List filteredIndices = PaginationStrategy.getSortedIndexMetadata( + List filteredIndices = getEligibleIndices( clusterState, - getMetadataFilter( - pageParams.getSort(), - Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexName, - Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexCreationTime, - indicesOptions - ), - PageParams.PARAM_ASC_SORT_VALUE.equals(pageParams.getSort()) ? ASC_COMPARATOR : DESC_COMPARATOR + pageParams.getSort(), + Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexName, + Objects.isNull(shardStrategyToken) ? null : shardStrategyToken.lastIndexCreationTime ); - // Get the list of shards and indices belonging to current page. this.pageData = getPageData( filteredIndices, @@ -64,31 +54,39 @@ public ShardPaginationStrategy(PageParams pageParams, ClusterState clusterState, ); } - private static Predicate getMetadataFilter( + private static List getEligibleIndices( + ClusterState clusterState, String sortOrder, String lastIndexName, - Long lastIndexCreationTime, - IndicesOptions indicesOptions + Long lastIndexCreationTime ) { if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { - return indexStateFilter(indicesOptions); + return PaginationStrategy.getSortedIndexMetadata( + clusterState, + PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR + ); + } else { + return PaginationStrategy.getSortedIndexMetadata( + clusterState, + getMetadataFilter(sortOrder, lastIndexName, lastIndexCreationTime), + PageParams.PARAM_ASC_SORT_VALUE.equals(sortOrder) ? ASC_COMPARATOR : DESC_COMPARATOR + ); + } + } + + private static Predicate getMetadataFilter(String sortOrder, String lastIndexName, Long lastIndexCreationTime) { + if (Objects.isNull(lastIndexName) || Objects.isNull(lastIndexCreationTime)) { + return indexMetadata -> true; } return indexNameFilter(lastIndexName).or( IndexPaginationStrategy.getIndexCreateTimeFilter(sortOrder, lastIndexName, lastIndexCreationTime) - ).and(indexStateFilter(indicesOptions)); + ); } private static Predicate indexNameFilter(String lastIndexName) { return metadata -> metadata.getIndex().getName().equals(lastIndexName); } - private static Predicate indexStateFilter(IndicesOptions indicesOptions) { - if (Objects.isNull(indicesOptions) || !indicesOptions.forbidClosedIndices()) { - return metadata -> true; - } - return metadata -> metadata.getState().equals(IndexMetadata.State.OPEN); - } - /** * Will be used to get the list of shards and respective indices to which they belong, * which are to be displayed in a page. diff --git a/server/src/main/java/org/opensearch/action/support/IndicesOptions.java b/server/src/main/java/org/opensearch/action/support/IndicesOptions.java index 2d9fecddb6f7d..7f93b1cf5a15a 100644 --- a/server/src/main/java/org/opensearch/action/support/IndicesOptions.java +++ b/server/src/main/java/org/opensearch/action/support/IndicesOptions.java @@ -167,6 +167,10 @@ public enum Option { EnumSet.of(Option.ALLOW_NO_INDICES, Option.IGNORE_UNAVAILABLE), EnumSet.of(WildcardStates.OPEN, WildcardStates.CLOSED, WildcardStates.HIDDEN) ); + public static final IndicesOptions LENIENT_EXPAND_OPEN_FORBID_CLOSED = new IndicesOptions( + EnumSet.of(Option.ALLOW_NO_INDICES, Option.IGNORE_UNAVAILABLE, Option.FORBID_CLOSED_INDICES), + EnumSet.of(WildcardStates.OPEN) + ); public static final IndicesOptions STRICT_EXPAND_OPEN_CLOSED = new IndicesOptions( EnumSet.of(Option.ALLOW_NO_INDICES), EnumSet.of(WildcardStates.OPEN, WildcardStates.CLOSED) @@ -654,6 +658,15 @@ public static IndicesOptions lenientExpandHidden() { return LENIENT_EXPAND_OPEN_CLOSED_HIDDEN; } + /** + * @return indices options that ignores unavailable indices and forbids closed indices (not return error if explicitly queried), + * expands wildcards to all open and closed indices and allows that no indices are resolved + * from wildcard expressions (not returning an error). + */ + public static IndicesOptions lenientExpandOpenAndForbidClosed() { + return LENIENT_EXPAND_OPEN_FORBID_CLOSED; + } + @Override public boolean equals(Object obj) { if (obj == null) { diff --git a/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java b/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java index 4da13e26ba100..aed7315660378 100644 --- a/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java +++ b/server/src/test/java/org/opensearch/action/pagination/ShardPaginationStrategyTests.java @@ -10,7 +10,6 @@ import org.opensearch.OpenSearchParseException; import org.opensearch.Version; -import org.opensearch.action.support.IndicesOptions; import org.opensearch.cluster.ClusterName; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; @@ -394,45 +393,6 @@ public void testRetrieveShardsWhenLastIndexGetsDeletedAndReCreated() { assertNull(strategy.getResponseToken().getNextToken()); } - /** - * Validates strategy filters out CLOSED indices, if forbidClosed() indices options are provided. - */ - public void testNoClosedIndicesReturnedByStrategy() { - final int pageSize = DEFAULT_NUMBER_OF_SHARDS * (DEFAULT_NUMBER_OF_REPLICAS + 1); - ClusterState clusterState = getRandomClusterState(List.of(0, 1, 2, 3, 4, 5)); - // Add 2 closed indices to cluster state - clusterState = addIndexToClusterState( - clusterState, - 6, - DEFAULT_NUMBER_OF_SHARDS, - DEFAULT_NUMBER_OF_REPLICAS, - IndexMetadata.State.CLOSE - ); - clusterState = addIndexToClusterState( - clusterState, - 7, - DEFAULT_NUMBER_OF_SHARDS, - DEFAULT_NUMBER_OF_REPLICAS, - IndexMetadata.State.CLOSE - ); - List shardRoutings = new ArrayList<>(); - List indices = new ArrayList<>(); - String requestedToken = null; - ShardPaginationStrategy strategy; - do { - PageParams pageParams = new PageParams(requestedToken, PARAM_ASC_SORT_VALUE, pageSize); - strategy = new ShardPaginationStrategy(pageParams, clusterState, IndicesOptions.strictExpandOpenAndForbidClosed()); - requestedToken = strategy.getResponseToken().getNextToken(); - shardRoutings.addAll(strategy.getRequestedEntities()); - indices.addAll(strategy.getRequestedIndices()); - } while (requestedToken != null); - // assert that the closed indices do not appear in the response - assertFalse(indices.contains(TEST_INDEX_PREFIX + 6)); - assertFalse(shardRoutings.stream().anyMatch(shard -> shard.getIndexName().equals(TEST_INDEX_PREFIX + 6))); - assertFalse(indices.contains(TEST_INDEX_PREFIX + 7)); - assertFalse(shardRoutings.stream().anyMatch(shard -> shard.getIndexName().equals(TEST_INDEX_PREFIX + 7))); - } - public void testCreatingShardStrategyPageTokenWithRequestedTokenNull() { try { new ShardPaginationStrategy.ShardStrategyToken(null); @@ -518,40 +478,11 @@ private ClusterState addIndexToClusterState( final int numShards, final int numReplicas, final long creationTime - ) { - return addIndexToClusterState(clusterState, indexNumber, numShards, numReplicas, creationTime, IndexMetadata.State.OPEN); - } - - private ClusterState addIndexToClusterState( - ClusterState clusterState, - final int indexNumber, - final int numShards, - final int numReplicas, - final IndexMetadata.State state - ) { - return addIndexToClusterState( - clusterState, - indexNumber, - numShards, - numReplicas, - Instant.now().plus(indexNumber, ChronoUnit.SECONDS).toEpochMilli(), - state - ); - } - - private ClusterState addIndexToClusterState( - ClusterState clusterState, - final int indexNumber, - final int numShards, - final int numReplicas, - final long creationTime, - final IndexMetadata.State state ) { IndexMetadata indexMetadata = IndexMetadata.builder(TEST_INDEX_PREFIX + indexNumber) .settings(settings(Version.CURRENT).put(SETTING_CREATION_DATE, creationTime)) .numberOfShards(numShards) .numberOfReplicas(numReplicas) - .state(state) .build(); IndexRoutingTable.Builder indexRoutingTableBuilder = new IndexRoutingTable.Builder(indexMetadata.getIndex()).initializeAsNew( indexMetadata From 692eeb62a625baa253473ffd9b5af361e5309619 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Tue, 26 Nov 2024 09:38:32 +0530 Subject: [PATCH 4/8] Refactoring to filter out hidden and closed indices Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsActionIT.java | 54 ++++++++++++++----- .../shards/TransportCatShardsAction.java | 26 ++++++--- .../action/support/IndicesOptions.java | 13 ----- 3 files changed, 61 insertions(+), 32 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java index 460b6b8629e42..b6a92535d1791 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java @@ -9,6 +9,7 @@ package org.opensearch.action.admin.cluster.shards; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.action.pagination.PageParams; import org.opensearch.client.Requests; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.routing.ShardRouting; @@ -128,33 +129,62 @@ public void onFailure(Exception e) { latch.await(); } - public void testCatShardsSuccessWithPaginationWithClosedIndices() throws InterruptedException, ExecutionException { + public void testListShardsWithClosedAndHiddenIndices() throws InterruptedException, ExecutionException { + final int numIndices = 3; + final int numShards = 1; + final int numReplicas = 2; + final int pageSize = numIndices * numReplicas * (numShards + 1); internalCluster().startClusterManagerOnlyNodes(1); internalCluster().startDataOnlyNodes(3); createIndex( "test", - Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2).build() + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build() ); createIndex( "test-2", - Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2).build() + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build() ); createIndex( - "test-3", + "test-closed-idx", Settings.builder() - .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) - .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2) - .put(INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING.getKey(), "60m") + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build() + ); + createIndex( + "test-hidden-idx", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .put(IndexMetadata.SETTING_INDEX_HIDDEN, true) .build() ); ensureGreen(); - // close index test-3 - client().admin().indices().close(Requests.closeIndexRequest("test-3")).get(); - final CatShardsRequest shardsRequest = new CatShardsRequest(); + // close index "test-closed-idx" + client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get(); + + CatShardsRequest shardsRequest = new CatShardsRequest(); shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); shardsRequest.setIndices(Strings.EMPTY_ARRAY); - ActionFuture response = client().execute(CatShardsAction.INSTANCE, shardsRequest); - assertTrue(response.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-3"))); + ActionFuture catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertTrue(catShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); + assertTrue(catShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); + + shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); + ActionFuture listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertTrue(listShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); + assertTrue(listShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); + assertEquals(catShardsResponse.get().getResponseShards().size(), listShardsResponse.get().getResponseShards().size()); + assertEquals( + catShardsResponse.get().getIndicesStatsResponse().getShards().length, + listShardsResponse.get().getIndicesStatsResponse().getShards().length + ); } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index 122b4ab3f3269..ffb19f6a5a9be 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -16,9 +16,9 @@ import org.opensearch.action.pagination.ShardPaginationStrategy; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; -import org.opensearch.action.support.IndicesOptions; import org.opensearch.action.support.TimeoutTaskCancellationUtility; import org.opensearch.client.node.NodeClient; +import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.breaker.ResponseLimitBreachedException; import org.opensearch.common.breaker.ResponseLimitSettings; import org.opensearch.common.inject.Inject; @@ -28,7 +28,9 @@ import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; +import java.util.List; import java.util.Objects; +import java.util.stream.Collectors; import static org.opensearch.common.breaker.ResponseLimitSettings.LimitEntity.SHARDS; @@ -101,7 +103,9 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { ); String[] indices = Objects.isNull(paginationStrategy) ? shardsRequest.getIndices() - : paginationStrategy.getRequestedIndices().toArray(new String[0]); + : filterClosedAndHiddenIndices(clusterStateResponse, paginationStrategy.getRequestedIndices()).toArray( + new String[0] + ); catShardsResponse.setNodes(clusterStateResponse.getState().getNodes()); catShardsResponse.setResponseShards( Objects.isNull(paginationStrategy) @@ -116,11 +120,6 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { return; } IndicesStatsRequest indicesStatsRequest = new IndicesStatsRequest(); - if (paginationStrategy != null) { - // Use lenient indices options for paginated queries, which would silently - // ignore closed concrete indices for fetching stats instead of throwing out an error. - indicesStatsRequest.indicesOptions(IndicesOptions.lenientExpandOpenAndForbidClosed()); - } indicesStatsRequest.setShouldCancelOnTimeout(true); indicesStatsRequest.all(); indicesStatsRequest.indices(indices); @@ -175,4 +174,17 @@ private void validateRequestLimit( private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy paginationStrategy) { return Objects.nonNull(paginationStrategy) && paginationStrategy.getRequestedEntities().isEmpty(); } + + /** + * Will be used by paginated query (_list/shards) to filter out closed and hidden indices before fetching + * IndicesStats. Since pagination strategy always passes concrete indices to TransportIndicesStatsAction, + * the default behaviour of StrictExpandOpenAndForbidClosed leads to errors if closed indices are encountered and + * stats being fetched for hidden indices, making it deviate from default non-paginated queries. + */ + private List filterClosedAndHiddenIndices(ClusterStateResponse clusterStateResponse, List indices) { + return indices.stream().filter(index -> { + IndexMetadata metadata = clusterStateResponse.getState().getMetadata().indices().get(index); + return metadata.getState().equals(IndexMetadata.State.OPEN) && !IndexMetadata.INDEX_HIDDEN_SETTING.get(metadata.getSettings()); + }).collect(Collectors.toList()); + } } diff --git a/server/src/main/java/org/opensearch/action/support/IndicesOptions.java b/server/src/main/java/org/opensearch/action/support/IndicesOptions.java index 7f93b1cf5a15a..2d9fecddb6f7d 100644 --- a/server/src/main/java/org/opensearch/action/support/IndicesOptions.java +++ b/server/src/main/java/org/opensearch/action/support/IndicesOptions.java @@ -167,10 +167,6 @@ public enum Option { EnumSet.of(Option.ALLOW_NO_INDICES, Option.IGNORE_UNAVAILABLE), EnumSet.of(WildcardStates.OPEN, WildcardStates.CLOSED, WildcardStates.HIDDEN) ); - public static final IndicesOptions LENIENT_EXPAND_OPEN_FORBID_CLOSED = new IndicesOptions( - EnumSet.of(Option.ALLOW_NO_INDICES, Option.IGNORE_UNAVAILABLE, Option.FORBID_CLOSED_INDICES), - EnumSet.of(WildcardStates.OPEN) - ); public static final IndicesOptions STRICT_EXPAND_OPEN_CLOSED = new IndicesOptions( EnumSet.of(Option.ALLOW_NO_INDICES), EnumSet.of(WildcardStates.OPEN, WildcardStates.CLOSED) @@ -658,15 +654,6 @@ public static IndicesOptions lenientExpandHidden() { return LENIENT_EXPAND_OPEN_CLOSED_HIDDEN; } - /** - * @return indices options that ignores unavailable indices and forbids closed indices (not return error if explicitly queried), - * expands wildcards to all open and closed indices and allows that no indices are resolved - * from wildcard expressions (not returning an error). - */ - public static IndicesOptions lenientExpandOpenAndForbidClosed() { - return LENIENT_EXPAND_OPEN_FORBID_CLOSED; - } - @Override public boolean equals(Object obj) { if (obj == null) { From 537fb2dc3dad8f5f6200e623109d4e29bac8eb13 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Wed, 27 Nov 2024 10:14:08 +0530 Subject: [PATCH 5/8] Fixing hidden indices being explicitly queried in _list/shards Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsActionIT.java | 78 +++++++++++++++++++ .../shards/TransportCatShardsAction.java | 29 ++++--- 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java index b6a92535d1791..e7446620504b8 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java @@ -23,6 +23,7 @@ import org.opensearch.test.OpenSearchIntegTestCase; import java.io.IOException; +import java.util.Arrays; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; @@ -169,6 +170,7 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti // close index "test-closed-idx" client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get(); + // Verifying responses for default queries: /_cat/shards and /_list/shards CatShardsRequest shardsRequest = new CatShardsRequest(); shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); shardsRequest.setIndices(Strings.EMPTY_ARRAY); @@ -185,6 +187,82 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti catShardsResponse.get().getIndicesStatsResponse().getShards().length, listShardsResponse.get().getIndicesStatsResponse().getShards().length ); + + // Verifying responses when hidden indices are explicitly queried: /_cat/shards/test-hidden-idx and /_list/shards/test-hidden-idx + // Shards for hidden index should appear in response along with stats + shardsRequest = new CatShardsRequest(); + shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); + shardsRequest.setIndices(List.of("test-hidden-idx").toArray(new String[0])); + catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertTrue(catShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); + assertTrue( + Arrays.stream(catShardsResponse.get().getIndicesStatsResponse().getShards()) + .allMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-hidden-idx")) + ); + assertEquals( + catShardsResponse.get().getResponseShards().size(), + catShardsResponse.get().getIndicesStatsResponse().getShards().length + ); + + shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); + assertTrue( + Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards()) + .allMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-hidden-idx")) + ); + assertEquals( + listShardsResponse.get().getResponseShards().size(), + listShardsResponse.get().getIndicesStatsResponse().getShards().length + ); + + // Verifying responses when hidden indices are queried with wildcards: /_cat/shards/test-hidden-idx* and + // /_list/shards/test-hidden-idx* + // Shards for hidden index should appear in response without stats + shardsRequest = new CatShardsRequest(); + shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); + shardsRequest.setIndices(List.of("test-hidden-idx*").toArray(new String[0])); + catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertTrue(catShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); + assertEquals(0, catShardsResponse.get().getIndicesStatsResponse().getShards().length); + + shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); + assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length); + + // Explicitly querying for closed index: /_cat/shards/test-closed-idx and /_list/shards/test-closed-idx + // /_cat/shards/test-closed-idx should result in IndexClosedException + // while /_list/shards/test-closed-idx should output closed shards without stats. + shardsRequest = new CatShardsRequest(); + shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); + shardsRequest.setIndices(List.of("test-closed-idx").toArray(new String[0])); + try { + catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + catShardsResponse.get(); + fail("Expected IndexClosedException"); + } catch (Exception exception) { + assertTrue(exception.getMessage().contains("IndexClosedException")); + } + + shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); + assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length); + + // Querying for closed index with wildcards: /_cat/shards/test-closed-idx and /_list/shards/test-closed-idx + // Both the queries should return zero entries + shardsRequest = new CatShardsRequest(); + shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); + shardsRequest.setIndices(List.of("test-closed-idx*").toArray(new String[0])); + catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertEquals(0, catShardsResponse.get().getResponseShards().size()); + assertEquals(0, catShardsResponse.get().getIndicesStatsResponse().getShards().length); + + shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + assertEquals(0, listShardsResponse.get().getResponseShards().size()); + assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length); } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index ffb19f6a5a9be..bfd9c2f9b0de8 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -28,6 +28,7 @@ import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; +import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -101,11 +102,6 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { shardsRequest.getPageParams(), clusterStateResponse ); - String[] indices = Objects.isNull(paginationStrategy) - ? shardsRequest.getIndices() - : filterClosedAndHiddenIndices(clusterStateResponse, paginationStrategy.getRequestedIndices()).toArray( - new String[0] - ); catShardsResponse.setNodes(clusterStateResponse.getState().getNodes()); catShardsResponse.setResponseShards( Objects.isNull(paginationStrategy) @@ -113,8 +109,16 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { : paginationStrategy.getRequestedEntities() ); catShardsResponse.setPageToken(Objects.isNull(paginationStrategy) ? null : paginationStrategy.getResponseToken()); + + String[] indices = Objects.isNull(paginationStrategy) + ? shardsRequest.getIndices() + : filterClosedAndHiddenIndices( + clusterStateResponse, + paginationStrategy.getRequestedIndices(), + Arrays.asList(shardsRequest.getIndices()) + ).toArray(new String[0]); // For paginated queries, if strategy outputs no shards to be returned, avoid fetching IndicesStats. - if (shouldSkipIndicesStatsRequest(paginationStrategy)) { + if (shouldSkipIndicesStatsRequest(paginationStrategy, indices)) { catShardsResponse.setIndicesStatsResponse(IndicesStatsResponse.getEmptyResponse()); cancellableListener.onResponse(catShardsResponse); return; @@ -171,8 +175,8 @@ private void validateRequestLimit( } } - private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy paginationStrategy) { - return Objects.nonNull(paginationStrategy) && paginationStrategy.getRequestedEntities().isEmpty(); + private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy paginationStrategy, String[] indices) { + return Objects.nonNull(paginationStrategy) && Objects.nonNull(indices) && indices.length == 0; } /** @@ -181,10 +185,15 @@ private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy pagination * the default behaviour of StrictExpandOpenAndForbidClosed leads to errors if closed indices are encountered and * stats being fetched for hidden indices, making it deviate from default non-paginated queries. */ - private List filterClosedAndHiddenIndices(ClusterStateResponse clusterStateResponse, List indices) { + private List filterClosedAndHiddenIndices( + ClusterStateResponse clusterStateResponse, + List indices, + List requestedIndices + ) { return indices.stream().filter(index -> { IndexMetadata metadata = clusterStateResponse.getState().getMetadata().indices().get(index); - return metadata.getState().equals(IndexMetadata.State.OPEN) && !IndexMetadata.INDEX_HIDDEN_SETTING.get(metadata.getSettings()); + return metadata.getState().equals(IndexMetadata.State.OPEN) + && (requestedIndices.contains(index) || !IndexMetadata.INDEX_HIDDEN_SETTING.get(metadata.getSettings())); }).collect(Collectors.toList()); } } From 40b264179ad57fe7320927d6fbd74a51e9d63caf Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Fri, 29 Nov 2024 09:55:54 +0530 Subject: [PATCH 6/8] Filter out only the closed indices Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsActionIT.java | 98 ++++++------------- .../shards/TransportCatShardsAction.java | 38 +++---- .../indices/stats/IndicesStatsRequest.java | 10 ++ .../stats/TransportIndicesStatsAction.java | 8 ++ 4 files changed, 67 insertions(+), 87 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java index e7446620504b8..396c5f17705a2 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java @@ -170,42 +170,38 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti // close index "test-closed-idx" client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get(); - // Verifying responses for default queries: /_cat/shards and /_list/shards - CatShardsRequest shardsRequest = new CatShardsRequest(); - shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); - shardsRequest.setIndices(Strings.EMPTY_ARRAY); - ActionFuture catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); - assertTrue(catShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); - assertTrue(catShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); - - shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); - ActionFuture listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + // Verifying response for default queries: /_list/shards + // all the shards should be part of response, however stats should not be displayed for closed index + CatShardsRequest listShardsRequest = new CatShardsRequest(); + listShardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); + listShardsRequest.setIndices(Strings.EMPTY_ARRAY); + listShardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); + ActionFuture listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); assertTrue(listShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); assertTrue(listShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); - assertEquals(catShardsResponse.get().getResponseShards().size(), listShardsResponse.get().getResponseShards().size()); - assertEquals( - catShardsResponse.get().getIndicesStatsResponse().getShards().length, - listShardsResponse.get().getIndicesStatsResponse().getShards().length + assertFalse( + Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards()) + .anyMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-closed-idx")) ); - // Verifying responses when hidden indices are explicitly queried: /_cat/shards/test-hidden-idx and /_list/shards/test-hidden-idx + // Verifying responses when hidden indices are explicitly queried: /_list/shards/test-hidden-idx // Shards for hidden index should appear in response along with stats - shardsRequest = new CatShardsRequest(); - shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); - shardsRequest.setIndices(List.of("test-hidden-idx").toArray(new String[0])); - catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); - assertTrue(catShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); + listShardsRequest.setIndices(List.of("test-hidden-idx").toArray(new String[0])); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); assertTrue( - Arrays.stream(catShardsResponse.get().getIndicesStatsResponse().getShards()) + Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards()) .allMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-hidden-idx")) ); assertEquals( - catShardsResponse.get().getResponseShards().size(), - catShardsResponse.get().getIndicesStatsResponse().getShards().length + listShardsResponse.get().getResponseShards().size(), + listShardsResponse.get().getIndicesStatsResponse().getShards().length ); - shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); - listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + // Verifying responses when hidden indices are queried with wildcards: /_list/shards/test-hidden-idx* + // Shards for hidden index should appear in response without stats + listShardsRequest.setIndices(List.of("test-hidden-idx*").toArray(new String[0])); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); assertTrue( Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards()) @@ -216,52 +212,18 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti listShardsResponse.get().getIndicesStatsResponse().getShards().length ); - // Verifying responses when hidden indices are queried with wildcards: /_cat/shards/test-hidden-idx* and - // /_list/shards/test-hidden-idx* - // Shards for hidden index should appear in response without stats - shardsRequest = new CatShardsRequest(); - shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); - shardsRequest.setIndices(List.of("test-hidden-idx*").toArray(new String[0])); - catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); - assertTrue(catShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); - assertEquals(0, catShardsResponse.get().getIndicesStatsResponse().getShards().length); - - shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); - listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); - assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); - assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length); - - // Explicitly querying for closed index: /_cat/shards/test-closed-idx and /_list/shards/test-closed-idx - // /_cat/shards/test-closed-idx should result in IndexClosedException - // while /_list/shards/test-closed-idx should output closed shards without stats. - shardsRequest = new CatShardsRequest(); - shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); - shardsRequest.setIndices(List.of("test-closed-idx").toArray(new String[0])); - try { - catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); - catShardsResponse.get(); - fail("Expected IndexClosedException"); - } catch (Exception exception) { - assertTrue(exception.getMessage().contains("IndexClosedException")); - } - - shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); - listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); + // Explicitly querying for closed index: /_list/shards/test-closed-idx + // should output closed shards without stats. + listShardsRequest.setIndices(List.of("test-closed-idx").toArray(new String[0])); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length); - // Querying for closed index with wildcards: /_cat/shards/test-closed-idx and /_list/shards/test-closed-idx - // Both the queries should return zero entries - shardsRequest = new CatShardsRequest(); - shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); - shardsRequest.setIndices(List.of("test-closed-idx*").toArray(new String[0])); - catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); - assertEquals(0, catShardsResponse.get().getResponseShards().size()); - assertEquals(0, catShardsResponse.get().getIndicesStatsResponse().getShards().length); - - shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); - listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest); - assertEquals(0, listShardsResponse.get().getResponseShards().size()); + // Querying for closed index with wildcards: /_list/shards/test-closed-idx* + // should output closed shards without stats. + listShardsRequest.setIndices(List.of("test-closed-idx*").toArray(new String[0])); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length); } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index bfd9c2f9b0de8..88371400bf0e2 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -16,19 +16,21 @@ import org.opensearch.action.pagination.ShardPaginationStrategy; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; +import org.opensearch.action.support.IndicesOptions; import org.opensearch.action.support.TimeoutTaskCancellationUtility; import org.opensearch.client.node.NodeClient; +import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.common.breaker.ResponseLimitBreachedException; import org.opensearch.common.breaker.ResponseLimitSettings; import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; import org.opensearch.core.action.NotifyOnceListener; +import org.opensearch.core.common.Strings; import org.opensearch.tasks.CancellableTask; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; -import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -67,6 +69,7 @@ public void doExecute(Task parentTask, CatShardsRequest shardsRequest, ActionLis clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()); } else { clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()).metadata(true); + clusterStateRequest.indicesOptions(IndicesOptions.lenientExpandHidden()); } assert parentTask instanceof CancellableTask; clusterStateRequest.setParentTask(client.getLocalNodeId(), parentTask.getId()); @@ -112,11 +115,9 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { String[] indices = Objects.isNull(paginationStrategy) ? shardsRequest.getIndices() - : filterClosedAndHiddenIndices( - clusterStateResponse, - paginationStrategy.getRequestedIndices(), - Arrays.asList(shardsRequest.getIndices()) - ).toArray(new String[0]); + : filterPaginationResponse(clusterStateResponse.getState(), paginationStrategy.getRequestedIndices()).toArray( + Strings.EMPTY_ARRAY + ); // For paginated queries, if strategy outputs no shards to be returned, avoid fetching IndicesStats. if (shouldSkipIndicesStatsRequest(paginationStrategy, indices)) { catShardsResponse.setIndicesStatsResponse(IndicesStatsResponse.getEmptyResponse()); @@ -127,6 +128,11 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { indicesStatsRequest.setShouldCancelOnTimeout(true); indicesStatsRequest.all(); indicesStatsRequest.indices(indices); + // Since the indices for paginated query are already concrete and have been filtered to + // only consider OPEN indices, invoking IndexNameExpressionResolver should be avoided. + if (paginationStrategy != null) { + indicesStatsRequest.skipIndexNameResolver(true); + } indicesStatsRequest.setParentTask(client.getLocalNodeId(), parentTask.getId()); client.admin().indices().stats(indicesStatsRequest, new ActionListener() { @Override @@ -176,24 +182,18 @@ private void validateRequestLimit( } private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy paginationStrategy, String[] indices) { - return Objects.nonNull(paginationStrategy) && Objects.nonNull(indices) && indices.length == 0; + return Objects.nonNull(paginationStrategy) && (indices == null || indices.length == 0); } /** - * Will be used by paginated query (_list/shards) to filter out closed and hidden indices before fetching + * Will be used by paginated query (_list/shards) to filter out closed indices (only consider OPEN) before fetching * IndicesStats. Since pagination strategy always passes concrete indices to TransportIndicesStatsAction, - * the default behaviour of StrictExpandOpenAndForbidClosed leads to errors if closed indices are encountered and - * stats being fetched for hidden indices, making it deviate from default non-paginated queries. + * the default behaviour of StrictExpandOpenAndForbidClosed leads to errors if closed indices are encountered. */ - private List filterClosedAndHiddenIndices( - ClusterStateResponse clusterStateResponse, - List indices, - List requestedIndices - ) { - return indices.stream().filter(index -> { - IndexMetadata metadata = clusterStateResponse.getState().getMetadata().indices().get(index); - return metadata.getState().equals(IndexMetadata.State.OPEN) - && (requestedIndices.contains(index) || !IndexMetadata.INDEX_HIDDEN_SETTING.get(metadata.getSettings())); + private List filterPaginationResponse(ClusterState clusterState, List strategyIndices) { + return strategyIndices.stream().filter(index -> { + IndexMetadata metadata = clusterState.metadata().indices().get(index); + return metadata != null && metadata.getState().equals(IndexMetadata.State.OPEN); }).collect(Collectors.toList()); } } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java index c36e53098d166..fe70386a19022 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java @@ -57,6 +57,7 @@ public class IndicesStatsRequest extends BroadcastRequest { private CommonStatsFlags flags = new CommonStatsFlags(); + private boolean skipIndexNameResolver = false; public IndicesStatsRequest() { super((String[]) null); @@ -307,4 +308,13 @@ public void writeTo(StreamOutput out) throws IOException { public boolean includeDataStreams() { return true; } + + public boolean skipIndexNameResolver() { + return skipIndexNameResolver; + } + + public IndicesStatsRequest skipIndexNameResolver(boolean skipIndexNameResolver) { + this.skipIndexNameResolver = skipIndexNameResolver; + return this; + } } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java index 2b85b6d5d6b5b..114b9e0ffd11e 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java @@ -153,4 +153,12 @@ protected ShardStats shardOperation(IndicesStatsRequest request, ShardRouting sh } return new ShardStats(indexShard.routingEntry(), indexShard.shardPath(), commonStats, commitStats, seqNoStats, retentionLeaseStats); } + + @Override + protected String[] resolveConcreteIndexNames(ClusterState clusterState, IndicesStatsRequest request) { + if (request.skipIndexNameResolver()) { + return request.indices(); + } + return super.resolveConcreteIndexNames(clusterState, request); + } } From 9db595dc4e18c0e0a562df1d240621879b4517c3 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Mon, 2 Dec 2024 10:56:27 +0530 Subject: [PATCH 7/8] Adding more ITs and ser/de for new parameter Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsActionIT.java | 280 ++++++++++++++++-- .../shards/TransportCatShardsAction.java | 14 +- .../indices/stats/IndicesStatsRequest.java | 7 + .../stats/TransportIndicesStatsAction.java | 6 +- 4 files changed, 275 insertions(+), 32 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java index 396c5f17705a2..a7cb4847b45e5 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java @@ -8,7 +8,10 @@ package org.opensearch.action.admin.cluster.shards; +import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest; +import org.opensearch.action.admin.indices.datastream.DataStreamTestCase; import org.opensearch.action.admin.indices.stats.IndicesStatsResponse; +import org.opensearch.action.admin.indices.stats.ShardStats; import org.opensearch.action.pagination.PageParams; import org.opensearch.client.Requests; import org.opensearch.cluster.metadata.IndexMetadata; @@ -23,6 +26,7 @@ import org.opensearch.test.OpenSearchIntegTestCase; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -31,9 +35,10 @@ import static org.opensearch.cluster.routing.UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING; import static org.opensearch.common.unit.TimeValue.timeValueMillis; import static org.opensearch.search.SearchService.NO_TIMEOUT; +import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked; @OpenSearchIntegTestCase.ClusterScope(numDataNodes = 0, scope = OpenSearchIntegTestCase.Scope.TEST) -public class TransportCatShardsActionIT extends OpenSearchIntegTestCase { +public class TransportCatShardsActionIT extends DataStreamTestCase { public void testCatShardsWithSuccessResponse() throws InterruptedException { internalCluster().startClusterManagerOnlyNodes(1); @@ -130,11 +135,81 @@ public void onFailure(Exception e) { latch.await(); } + public void testListShardsWithHiddenIndex() throws Exception { + final int numShards = 1; + final int numReplicas = 1; + internalCluster().startClusterManagerOnlyNodes(1); + internalCluster().startDataOnlyNodes(2); + createIndex( + "test-hidden-idx", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .put(IndexMetadata.SETTING_INDEX_HIDDEN, true) + .build() + ); + ensureGreen(); + + // Verify result for a default query: "_list/shards" + CatShardsRequest listShardsRequest = getListShardsTransportRequest(Strings.EMPTY_ARRAY, 100); + ActionFuture listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertSingleIndexResponseShards(listShardsResponse.get(), "test-hidden-idx", 2, true); + + // Verify result when hidden index is explicitly queried: "_list/shards" + listShardsRequest = getListShardsTransportRequest(new String[] { "test-hidden-idx" }, 100); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertSingleIndexResponseShards(listShardsResponse.get(), "test-hidden-idx", 2, true); + + // Verify result when hidden index is queried with wildcard: "_list/shards*" + // Since the ClusterStateAction underneath is invoked with lenientExpandOpen IndicesOptions, + // Wildcards for hidden indices should not get resolved. + listShardsRequest = getListShardsTransportRequest(new String[] { "test-hidden-idx*" }, 100); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertEquals(0, listShardsResponse.get().getResponseShards().size()); + assertSingleIndexResponseShards(listShardsResponse.get(), "test-hidden-idx", 0, false); + } + + public void testListShardsWithClosedIndex() throws Exception { + final int numShards = 1; + final int numReplicas = 1; + internalCluster().startClusterManagerOnlyNodes(1); + internalCluster().startDataOnlyNodes(2); + createIndex( + "test-closed-idx", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build() + ); + ensureGreen(); + + // close index "test-closed-idx" + client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get(); + ensureGreen(); + + // Verify result for a default query: "_list/shards" + CatShardsRequest listShardsRequest = getListShardsTransportRequest(Strings.EMPTY_ARRAY, 100); + ActionFuture listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertSingleIndexResponseShards(listShardsResponse.get(), "test-closed-idx", 2, false); + + // Verify result when closed index is explicitly queried: "_list/shards" + listShardsRequest = getListShardsTransportRequest(new String[] { "test-closed-idx" }, 100); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertSingleIndexResponseShards(listShardsResponse.get(), "test-closed-idx", 2, false); + + // Verify result when closed index is queried with wildcard: "_list/shards*" + // Since the ClusterStateAction underneath is invoked with lenientExpandOpen IndicesOptions, + // Wildcards for closed indices should not get resolved. + listShardsRequest = getListShardsTransportRequest(new String[] { "test-closed-idx*" }, 100); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertSingleIndexResponseShards(listShardsResponse.get(), "test-closed-idx", 0, false); + } + public void testListShardsWithClosedAndHiddenIndices() throws InterruptedException, ExecutionException { - final int numIndices = 3; + final int numIndices = 4; final int numShards = 1; final int numReplicas = 2; - final int pageSize = numIndices * numReplicas * (numShards + 1); + final int pageSize = 100; internalCluster().startClusterManagerOnlyNodes(1); internalCluster().startDataOnlyNodes(3); createIndex( @@ -166,23 +241,25 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti .put(IndexMetadata.SETTING_INDEX_HIDDEN, true) .build() ); - ensureGreen(); // close index "test-closed-idx" client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get(); + ensureGreen(); // Verifying response for default queries: /_list/shards // all the shards should be part of response, however stats should not be displayed for closed index - CatShardsRequest listShardsRequest = new CatShardsRequest(); - listShardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); - listShardsRequest.setIndices(Strings.EMPTY_ARRAY); - listShardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); + CatShardsRequest listShardsRequest = getListShardsTransportRequest(Strings.EMPTY_ARRAY, pageSize); ActionFuture listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); assertTrue(listShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); assertTrue(listShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); + assertEquals(numIndices * numShards * (numReplicas + 1), listShardsResponse.get().getResponseShards().size()); assertFalse( Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards()) .anyMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-closed-idx")) ); + assertEquals( + (numIndices - 1) * numShards * (numReplicas + 1), + listShardsResponse.get().getIndicesStatsResponse().getShards().length + ); // Verifying responses when hidden indices are explicitly queried: /_list/shards/test-hidden-idx // Shards for hidden index should appear in response along with stats @@ -199,32 +276,193 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti ); // Verifying responses when hidden indices are queried with wildcards: /_list/shards/test-hidden-idx* - // Shards for hidden index should appear in response without stats + // Shards for hidden index should not appear in response with stats. listShardsRequest.setIndices(List.of("test-hidden-idx*").toArray(new String[0])); listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); - assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx"))); - assertTrue( - Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards()) - .allMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-hidden-idx")) - ); - assertEquals( - listShardsResponse.get().getResponseShards().size(), - listShardsResponse.get().getIndicesStatsResponse().getShards().length - ); + assertEquals(0, listShardsResponse.get().getResponseShards().size()); + assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length); // Explicitly querying for closed index: /_list/shards/test-closed-idx // should output closed shards without stats. listShardsRequest.setIndices(List.of("test-closed-idx").toArray(new String[0])); listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); - assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); + assertTrue(listShardsResponse.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length); // Querying for closed index with wildcards: /_list/shards/test-closed-idx* - // should output closed shards without stats. + // should not output any closed shards. listShardsRequest.setIndices(List.of("test-closed-idx*").toArray(new String[0])); listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); - assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-closed-idx"))); + assertEquals(0, listShardsResponse.get().getResponseShards().size()); assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length); } + public void testListShardsWithClosedIndicesAcrossPages() throws InterruptedException, ExecutionException { + final int numIndices = 4; + final int numShards = 1; + final int numReplicas = 2; + final int pageSize = numShards * (numReplicas + 1); + internalCluster().startClusterManagerOnlyNodes(1); + internalCluster().startDataOnlyNodes(3); + createIndex( + "test-open-idx-1", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build() + ); + createIndex( + "test-closed-idx-1", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build() + ); + createIndex( + "test-open-idx-2", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build() + ); + createIndex( + "test-closed-idx-2", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .put(IndexMetadata.SETTING_INDEX_HIDDEN, true) + .build() + ); + // close index "test-closed-idx-1" + client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx-1")).get(); + ensureGreen(); + // close index "test-closed-idx-2" + client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx-2")).get(); + ensureGreen(); + + // Verifying response for default queries: /_list/shards + List responseShardRouting = new ArrayList<>(); + List responseShardStats = new ArrayList<>(); + String nextToken = null; + CatShardsRequest listShardsRequest; + ActionFuture listShardsResponse; + do { + listShardsRequest = getListShardsTransportRequest(Strings.EMPTY_ARRAY, nextToken, pageSize); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + nextToken = listShardsResponse.get().getPageToken().getNextToken(); + responseShardRouting.addAll(listShardsResponse.get().getResponseShards()); + responseShardStats.addAll(List.of(listShardsResponse.get().getIndicesStatsResponse().getShards())); + } while (nextToken != null); + + assertTrue(responseShardRouting.stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx-1"))); + assertTrue(responseShardRouting.stream().anyMatch(shard -> shard.getIndexName().equals("test-closed-idx-2"))); + assertEquals(numIndices * numShards * (numReplicas + 1), responseShardRouting.size()); + // ShardsStats should only appear for 2 open indices + assertFalse( + responseShardStats.stream().anyMatch(shardStats -> shardStats.getShardRouting().getIndexName().contains("test-closed-idx")) + ); + assertEquals(2 * numShards * (numReplicas + 1), responseShardStats.size()); + } + + public void testListShardsWithDataStream() throws Exception { + final int numDataNodes = 3; + String dataStreamName = "logs-test"; + internalCluster().startClusterManagerOnlyNodes(1); + internalCluster().startDataOnlyNodes(numDataNodes); + // Create an index template for data streams. + createDataStreamIndexTemplate("data-stream-template", List.of("logs-*")); + // Create data streams matching the "logs-*" index pattern. + createDataStream(dataStreamName); + ensureGreen(); + // Verifying default query's result. Data stream should have created a hidden backing index in the + // background and all the corresponding shards should appear in the response along with stats. + CatShardsRequest listShardsRequest = getListShardsTransportRequest(Strings.EMPTY_ARRAY, numDataNodes * numDataNodes); + ActionFuture listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertSingleIndexResponseShards(listShardsResponse.get(), dataStreamName, numDataNodes + 1, true); + // Verifying result when data stream is directly queried. Again, all the shards with stats should appear + listShardsRequest = getListShardsTransportRequest(new String[] { dataStreamName }, numDataNodes * numDataNodes); + listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertSingleIndexResponseShards(listShardsResponse.get(), dataStreamName, numDataNodes + 1, true); + } + + public void testListShardsWithAliases() throws Exception { + final int numShards = 1; + final int numReplicas = 1; + final String aliasName = "test-alias"; + internalCluster().startClusterManagerOnlyNodes(1); + internalCluster().startDataOnlyNodes(3); + createIndex( + "test-closed-idx", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .build() + ); + createIndex( + "test-hidden-idx", + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numReplicas) + .put(IndexMetadata.SETTING_INDEX_HIDDEN, true) + .build() + ); + ensureGreen(); + + // Point test alias to both the indices (one being hidden while the other is closed) + final IndicesAliasesRequest request = new IndicesAliasesRequest().origin("allowed"); + request.addAliasAction(IndicesAliasesRequest.AliasActions.add().index("test-closed-idx").alias(aliasName)); + assertAcked(client().admin().indices().aliases(request).actionGet()); + + request.addAliasAction(IndicesAliasesRequest.AliasActions.add().index("test-hidden-idx").alias(aliasName)); + assertAcked(client().admin().indices().aliases(request).actionGet()); + + // close index "test-closed-idx" + client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get(); + ensureGreen(); + + // Verifying result when an alias is explicitly queried. + CatShardsRequest listShardsRequest = getListShardsTransportRequest(new String[] { aliasName }, 100); + ActionFuture listShardsResponse = client().execute(CatShardsAction.INSTANCE, listShardsRequest); + assertTrue( + listShardsResponse.get() + .getResponseShards() + .stream() + .allMatch(shard -> shard.getIndexName().equals("test-hidden-idx") || shard.getIndexName().equals("test-closed-idx")) + ); + assertTrue( + Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards()) + .allMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-hidden-idx")) + ); + assertEquals(4, listShardsResponse.get().getResponseShards().size()); + assertEquals(2, listShardsResponse.get().getIndicesStatsResponse().getShards().length); + } + + private void assertSingleIndexResponseShards( + CatShardsResponse catShardsResponse, + String indexNamePattern, + final int totalNumShards, + boolean shardStatsExist + ) { + assertTrue(catShardsResponse.getResponseShards().stream().allMatch(shard -> shard.getIndexName().contains(indexNamePattern))); + assertEquals(totalNumShards, catShardsResponse.getResponseShards().size()); + if (shardStatsExist) { + assertTrue( + Arrays.stream(catShardsResponse.getIndicesStatsResponse().getShards()) + .allMatch(shardStats -> shardStats.getShardRouting().getIndexName().contains(indexNamePattern)) + ); + } + assertEquals(shardStatsExist ? totalNumShards : 0, catShardsResponse.getIndicesStatsResponse().getShards().length); + } + + private CatShardsRequest getListShardsTransportRequest(String[] indices, final int pageSize) { + return getListShardsTransportRequest(indices, null, pageSize); + } + + private CatShardsRequest getListShardsTransportRequest(String[] indices, String nextToken, final int pageSize) { + CatShardsRequest listShardsRequest = new CatShardsRequest(); + listShardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); + listShardsRequest.setIndices(indices); + listShardsRequest.setPageParams(new PageParams(nextToken, PageParams.PARAM_ASC_SORT_VALUE, pageSize)); + return listShardsRequest; + } } diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index 88371400bf0e2..cae9472f323f1 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -16,7 +16,6 @@ import org.opensearch.action.pagination.ShardPaginationStrategy; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; -import org.opensearch.action.support.IndicesOptions; import org.opensearch.action.support.TimeoutTaskCancellationUtility; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.ClusterState; @@ -26,14 +25,12 @@ import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; import org.opensearch.core.action.NotifyOnceListener; -import org.opensearch.core.common.Strings; import org.opensearch.tasks.CancellableTask; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; import static org.opensearch.common.breaker.ResponseLimitSettings.LimitEntity.SHARDS; @@ -69,7 +66,6 @@ public void doExecute(Task parentTask, CatShardsRequest shardsRequest, ActionLis clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()); } else { clusterStateRequest.clear().nodes(true).routingTable(true).indices(shardsRequest.getIndices()).metadata(true); - clusterStateRequest.indicesOptions(IndicesOptions.lenientExpandHidden()); } assert parentTask instanceof CancellableTask; clusterStateRequest.setParentTask(client.getLocalNodeId(), parentTask.getId()); @@ -115,9 +111,7 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { String[] indices = Objects.isNull(paginationStrategy) ? shardsRequest.getIndices() - : filterPaginationResponse(clusterStateResponse.getState(), paginationStrategy.getRequestedIndices()).toArray( - Strings.EMPTY_ARRAY - ); + : filterClosedIndices(clusterStateResponse.getState(), paginationStrategy.getRequestedIndices()); // For paginated queries, if strategy outputs no shards to be returned, avoid fetching IndicesStats. if (shouldSkipIndicesStatsRequest(paginationStrategy, indices)) { catShardsResponse.setIndicesStatsResponse(IndicesStatsResponse.getEmptyResponse()); @@ -190,10 +184,10 @@ private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy pagination * IndicesStats. Since pagination strategy always passes concrete indices to TransportIndicesStatsAction, * the default behaviour of StrictExpandOpenAndForbidClosed leads to errors if closed indices are encountered. */ - private List filterPaginationResponse(ClusterState clusterState, List strategyIndices) { + private String[] filterClosedIndices(ClusterState clusterState, List strategyIndices) { return strategyIndices.stream().filter(index -> { IndexMetadata metadata = clusterState.metadata().indices().get(index); - return metadata != null && metadata.getState().equals(IndexMetadata.State.OPEN); - }).collect(Collectors.toList()); + return metadata != null && metadata.getState().equals(IndexMetadata.State.CLOSE) == false; + }).toArray(String[]::new); } } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java index fe70386a19022..b3ede1d5f606c 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java @@ -32,6 +32,7 @@ package org.opensearch.action.admin.indices.stats; +import org.opensearch.Version; import org.opensearch.action.support.broadcast.BroadcastRequest; import org.opensearch.common.annotation.PublicApi; import org.opensearch.core.common.io.stream.StreamInput; @@ -66,6 +67,9 @@ public IndicesStatsRequest() { public IndicesStatsRequest(StreamInput in) throws IOException { super(in); flags = new CommonStatsFlags(in); + if (in.getVersion().onOrAfter(Version.V_3_0_0)) { + skipIndexNameResolver = in.readBoolean(); + } } /** @@ -302,6 +306,9 @@ public IndicesStatsRequest includeUnloadedSegments(boolean includeUnloadedSegmen public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); flags.writeTo(out); + if (out.getVersion().onOrAfter(Version.V_3_0_0)) { + out.writeBoolean(skipIndexNameResolver); + } } @Override diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java index 114b9e0ffd11e..7ddec140b49a6 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java @@ -56,6 +56,7 @@ import org.opensearch.transport.TransportService; import java.io.IOException; +import java.util.Arrays; import java.util.List; /** @@ -157,7 +158,10 @@ protected ShardStats shardOperation(IndicesStatsRequest request, ShardRouting sh @Override protected String[] resolveConcreteIndexNames(ClusterState clusterState, IndicesStatsRequest request) { if (request.skipIndexNameResolver()) { - return request.indices(); + // filter out all the indices which might not be present in the local clusterState + return Arrays.stream(request.indices()) + .filter(index -> clusterState.metadata().indices().containsKey(index)) + .toArray(String[]::new); } return super.resolveConcreteIndexNames(clusterState, request); } From 790b8c048e146071641a272e1c2e61bbc9b7a499 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Wed, 11 Dec 2024 10:25:02 +0530 Subject: [PATCH 8/8] Removing skipIndexResolver param Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsAction.java | 5 ----- .../indices/stats/IndicesStatsRequest.java | 17 ----------------- .../stats/TransportIndicesStatsAction.java | 12 ------------ 3 files changed, 34 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java index cae9472f323f1..01efa96a7369e 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java @@ -122,11 +122,6 @@ public void onResponse(ClusterStateResponse clusterStateResponse) { indicesStatsRequest.setShouldCancelOnTimeout(true); indicesStatsRequest.all(); indicesStatsRequest.indices(indices); - // Since the indices for paginated query are already concrete and have been filtered to - // only consider OPEN indices, invoking IndexNameExpressionResolver should be avoided. - if (paginationStrategy != null) { - indicesStatsRequest.skipIndexNameResolver(true); - } indicesStatsRequest.setParentTask(client.getLocalNodeId(), parentTask.getId()); client.admin().indices().stats(indicesStatsRequest, new ActionListener() { @Override diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java index b3ede1d5f606c..c36e53098d166 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java @@ -32,7 +32,6 @@ package org.opensearch.action.admin.indices.stats; -import org.opensearch.Version; import org.opensearch.action.support.broadcast.BroadcastRequest; import org.opensearch.common.annotation.PublicApi; import org.opensearch.core.common.io.stream.StreamInput; @@ -58,7 +57,6 @@ public class IndicesStatsRequest extends BroadcastRequest { private CommonStatsFlags flags = new CommonStatsFlags(); - private boolean skipIndexNameResolver = false; public IndicesStatsRequest() { super((String[]) null); @@ -67,9 +65,6 @@ public IndicesStatsRequest() { public IndicesStatsRequest(StreamInput in) throws IOException { super(in); flags = new CommonStatsFlags(in); - if (in.getVersion().onOrAfter(Version.V_3_0_0)) { - skipIndexNameResolver = in.readBoolean(); - } } /** @@ -306,22 +301,10 @@ public IndicesStatsRequest includeUnloadedSegments(boolean includeUnloadedSegmen public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); flags.writeTo(out); - if (out.getVersion().onOrAfter(Version.V_3_0_0)) { - out.writeBoolean(skipIndexNameResolver); - } } @Override public boolean includeDataStreams() { return true; } - - public boolean skipIndexNameResolver() { - return skipIndexNameResolver; - } - - public IndicesStatsRequest skipIndexNameResolver(boolean skipIndexNameResolver) { - this.skipIndexNameResolver = skipIndexNameResolver; - return this; - } } diff --git a/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java b/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java index 7ddec140b49a6..2b85b6d5d6b5b 100644 --- a/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java +++ b/server/src/main/java/org/opensearch/action/admin/indices/stats/TransportIndicesStatsAction.java @@ -56,7 +56,6 @@ import org.opensearch.transport.TransportService; import java.io.IOException; -import java.util.Arrays; import java.util.List; /** @@ -154,15 +153,4 @@ protected ShardStats shardOperation(IndicesStatsRequest request, ShardRouting sh } return new ShardStats(indexShard.routingEntry(), indexShard.shardPath(), commonStats, commitStats, seqNoStats, retentionLeaseStats); } - - @Override - protected String[] resolveConcreteIndexNames(ClusterState clusterState, IndicesStatsRequest request) { - if (request.skipIndexNameResolver()) { - // filter out all the indices which might not be present in the local clusterState - return Arrays.stream(request.indices()) - .filter(index -> clusterState.metadata().indices().containsKey(index)) - .toArray(String[]::new); - } - return super.resolveConcreteIndexNames(clusterState, request); - } }