From 40b264179ad57fe7320927d6fbd74a51e9d63caf Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Fri, 29 Nov 2024 09:55:54 +0530 Subject: [PATCH] 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); + } }