Skip to content

Commit

Permalink
Refactoring to filter out hidden and closed indices
Browse files Browse the repository at this point in the history 
Signed-off-by: Harsh Garg <[email protected]>
  • Loading branch information
Harsh Garg committed Nov 26, 2024
1 parent 06a1118 commit 692eeb6
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 32 deletions.
54 changes: 42 additions & 12 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 @@ -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;
Expand Down Expand Up @@ -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<CatShardsResponse> response = client().execute(CatShardsAction.INSTANCE, shardsRequest);
assertTrue(response.get().getResponseShards().stream().anyMatch(shard -> shard.getIndexName().equals("test-3")));
ActionFuture<CatShardsResponse> 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<CatShardsResponse> 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
);
}

}
26 changes: 19 additions & 7 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,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;
Expand All @@ -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;

Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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<String> filterClosedAndHiddenIndices(ClusterStateResponse clusterStateResponse, List<String> 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());
}
}
13 changes: 0 additions & 13 deletions server/src/main/java/org/opensearch/action/support/IndicesOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 692eeb6

Please sign in to comment.