From 692eeb62a625baa253473ffd9b5af361e5309619 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Tue, 26 Nov 2024 09:38:32 +0530 Subject: [PATCH] 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) {