From 77c7c9fca2511622098d186f7781b3e0276bf9b8 Mon Sep 17 00:00:00 2001 From: Harsh Garg Date: Mon, 2 Dec 2024 10:56:27 +0530 Subject: [PATCH] Adding more ITs and ser/de for new parameter Signed-off-by: Harsh Garg --- .../shards/TransportCatShardsActionIT.java | 208 ++++++++++++++++-- .../shards/TransportCatShardsAction.java | 14 +- .../indices/stats/IndicesStatsRequest.java | 7 + .../stats/TransportIndicesStatsAction.java | 6 +- 4 files changed, 203 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..f5991bdaf8f21 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,6 +8,8 @@ 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.pagination.PageParams; import org.opensearch.client.Requests; @@ -31,9 +33,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 +133,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 +239,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 +274,123 @@ 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 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) { + CatShardsRequest listShardsRequest = new CatShardsRequest(); + listShardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT); + listShardsRequest.setIndices(indices); + listShardsRequest.setPageParams(new PageParams(null, 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); }