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..125dd4c09847b 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);