Skip to content

Commit

Permalink
Adding more ITs and ser/de for new parameter
Browse files Browse the repository at this point in the history 
Signed-off-by: Harsh Garg <[email protected]>
  • Loading branch information
Harsh Garg committed Dec 3, 2024
1 parent 40b2641 commit 77c7c9f
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 32 deletions.
208 changes: 187 additions & 21 deletions ...usterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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<CatShardsResponse> 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<CatShardsResponse> 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(
Expand Down Expand Up @@ -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<CatShardsResponse> 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
Expand All @@ -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<CatShardsResponse> 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<CatShardsResponse> 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;
}

}
14 changes: 4 additions & 10 deletions ...er/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());

Check warning on line 114 in server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java#L113-L114 

Added lines #L113 - L114 were not covered by tests
// For paginated queries, if strategy outputs no shards to be returned, avoid fetching IndicesStats.
if (shouldSkipIndicesStatsRequest(paginationStrategy, indices)) {
catShardsResponse.setIndicesStatsResponse(IndicesStatsResponse.getEmptyResponse());
Expand Down Expand Up @@ -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<String> filterPaginationResponse(ClusterState clusterState, List<String> strategyIndices) {
private String[] filterClosedIndices(ClusterState clusterState, List<String> strategyIndices) {
return strategyIndices.stream().filter(index -> {
IndexMetadata metadata = clusterState.metadata().indices().get(index);

Check warning on line 189 in server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java#L188-L189 

Added lines #L188 - L189 were not covered by tests
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);

Check warning on line 191 in server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsAction.java#L191 

Added line #L191 was not covered by tests
}
}
7 changes: 7 additions & 0 deletions server/src/main/java/org/opensearch/action/admin/indices/stats/IndicesStatsRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}

/**
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 77c7c9f

Please sign in to comment.