Skip to content

Commit 537fb2d

Browse files
author
Harsh Garg
committed
Fixing hidden indices being explicitly queried in _list/shards
Signed-off-by: Harsh Garg <gkharsh@amazon.com>
1 parent 503683f commit 537fb2d

2 files changed

Lines changed: 97 additions & 10 deletions

File tree

‎server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/shards/TransportCatShardsActionIT.java‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.opensearch.test.OpenSearchIntegTestCase;
2424

2525
import java.io.IOException;
26+
import java.util.Arrays;
2627
import java.util.List;
2728
import java.util.concurrent.CountDownLatch;
2829
import java.util.concurrent.ExecutionException;
@@ -169,6 +170,7 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti
169170
// close index "test-closed-idx"
170171
client().admin().indices().close(Requests.closeIndexRequest("test-closed-idx")).get();
171172

173+
// Verifying responses for default queries: /_cat/shards and /_list/shards
172174
CatShardsRequest shardsRequest = new CatShardsRequest();
173175
shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
174176
shardsRequest.setIndices(Strings.EMPTY_ARRAY);
@@ -185,6 +187,82 @@ public void testListShardsWithClosedAndHiddenIndices() throws InterruptedExcepti
185187
catShardsResponse.get().getIndicesStatsResponse().getShards().length,
186188
listShardsResponse.get().getIndicesStatsResponse().getShards().length
187189
);
190+
191+
// Verifying responses when hidden indices are explicitly queried: /_cat/shards/test-hidden-idx and /_list/shards/test-hidden-idx
192+
// Shards for hidden index should appear in response along with stats
193+
shardsRequest = new CatShardsRequest();
194+
shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
195+
shardsRequest.setIndices(List.of("test-hidden-idx").toArray(new String[0]));
196+
catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
197+
assertTrue(catShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
198+
assertTrue(
199+
Arrays.stream(catShardsResponse.get().getIndicesStatsResponse().getShards())
200+
.allMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-hidden-idx"))
201+
);
202+
assertEquals(
203+
catShardsResponse.get().getResponseShards().size(),
204+
catShardsResponse.get().getIndicesStatsResponse().getShards().length
205+
);
206+
207+
shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
208+
listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
209+
assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
210+
assertTrue(
211+
Arrays.stream(listShardsResponse.get().getIndicesStatsResponse().getShards())
212+
.allMatch(shardStats -> shardStats.getShardRouting().getIndexName().equals("test-hidden-idx"))
213+
);
214+
assertEquals(
215+
listShardsResponse.get().getResponseShards().size(),
216+
listShardsResponse.get().getIndicesStatsResponse().getShards().length
217+
);
218+
219+
// Verifying responses when hidden indices are queried with wildcards: /_cat/shards/test-hidden-idx* and
220+
// /_list/shards/test-hidden-idx*
221+
// Shards for hidden index should appear in response without stats
222+
shardsRequest = new CatShardsRequest();
223+
shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
224+
shardsRequest.setIndices(List.of("test-hidden-idx*").toArray(new String[0]));
225+
catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
226+
assertTrue(catShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
227+
assertEquals(0, catShardsResponse.get().getIndicesStatsResponse().getShards().length);
228+
229+
shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
230+
listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
231+
assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-hidden-idx")));
232+
assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length);
233+
234+
// Explicitly querying for closed index: /_cat/shards/test-closed-idx and /_list/shards/test-closed-idx
235+
// /_cat/shards/test-closed-idx should result in IndexClosedException
236+
// while /_list/shards/test-closed-idx should output closed shards without stats.
237+
shardsRequest = new CatShardsRequest();
238+
shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
239+
shardsRequest.setIndices(List.of("test-closed-idx").toArray(new String[0]));
240+
try {
241+
catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
242+
catShardsResponse.get();
243+
fail("Expected IndexClosedException");
244+
} catch (Exception exception) {
245+
assertTrue(exception.getMessage().contains("IndexClosedException"));
246+
}
247+
248+
shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
249+
listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
250+
assertTrue(listShardsResponse.get().getResponseShards().stream().allMatch(shard -> shard.getIndexName().equals("test-closed-idx")));
251+
assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length);
252+
253+
// Querying for closed index with wildcards: /_cat/shards/test-closed-idx and /_list/shards/test-closed-idx
254+
// Both the queries should return zero entries
255+
shardsRequest = new CatShardsRequest();
256+
shardsRequest.setCancelAfterTimeInterval(NO_TIMEOUT);
257+
shardsRequest.setIndices(List.of("test-closed-idx*").toArray(new String[0]));
258+
catShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
259+
assertEquals(0, catShardsResponse.get().getResponseShards().size());
260+
assertEquals(0, catShardsResponse.get().getIndicesStatsResponse().getShards().length);
261+
262+
shardsRequest.setPageParams(new PageParams(null, PageParams.PARAM_ASC_SORT_VALUE, pageSize));
263+
listShardsResponse = client().execute(CatShardsAction.INSTANCE, shardsRequest);
264+
assertEquals(0, listShardsResponse.get().getResponseShards().size());
265+
assertEquals(0, listShardsResponse.get().getIndicesStatsResponse().getShards().length);
188266
}
189267

190268
}

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.opensearch.tasks.Task;
2929
import org.opensearch.transport.TransportService;
3030

31+
import java.util.Arrays;
3132
import java.util.List;
3233
import java.util.Objects;
3334
import java.util.stream.Collectors;
@@ -101,20 +102,23 @@ public void onResponse(ClusterStateResponse clusterStateResponse) {
101102
shardsRequest.getPageParams(),
102103
clusterStateResponse
103104
);
104-
String[] indices = Objects.isNull(paginationStrategy)
105-
? shardsRequest.getIndices()
106-
: filterClosedAndHiddenIndices(clusterStateResponse, paginationStrategy.getRequestedIndices()).toArray(
107-
new String[0]
108-
);
109105
catShardsResponse.setNodes(clusterStateResponse.getState().getNodes());
110106
catShardsResponse.setResponseShards(
111107
Objects.isNull(paginationStrategy)
112108
? clusterStateResponse.getState().routingTable().allShards()
113109
: paginationStrategy.getRequestedEntities()
114110
);
115111
catShardsResponse.setPageToken(Objects.isNull(paginationStrategy) ? null : paginationStrategy.getResponseToken());
112+
113+
String[] indices = Objects.isNull(paginationStrategy)
114+
? shardsRequest.getIndices()
115+
: filterClosedAndHiddenIndices(
116+
clusterStateResponse,
117+
paginationStrategy.getRequestedIndices(),
118+
Arrays.asList(shardsRequest.getIndices())
119+
).toArray(new String[0]);
116120
// For paginated queries, if strategy outputs no shards to be returned, avoid fetching IndicesStats.
117-
if (shouldSkipIndicesStatsRequest(paginationStrategy)) {
121+
if (shouldSkipIndicesStatsRequest(paginationStrategy, indices)) {
118122
catShardsResponse.setIndicesStatsResponse(IndicesStatsResponse.getEmptyResponse());
119123
cancellableListener.onResponse(catShardsResponse);
120124
return;
@@ -171,8 +175,8 @@ private void validateRequestLimit(
171175
}
172176
}
173177

174-
private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy paginationStrategy) {
175-
return Objects.nonNull(paginationStrategy) && paginationStrategy.getRequestedEntities().isEmpty();
178+
private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy paginationStrategy, String[] indices) {
179+
return Objects.nonNull(paginationStrategy) && Objects.nonNull(indices) && indices.length == 0;
176180
}
177181

178182
/**
@@ -181,10 +185,15 @@ private boolean shouldSkipIndicesStatsRequest(ShardPaginationStrategy pagination
181185
* the default behaviour of StrictExpandOpenAndForbidClosed leads to errors if closed indices are encountered and
182186
* stats being fetched for hidden indices, making it deviate from default non-paginated queries.
183187
*/
184-
private List<String> filterClosedAndHiddenIndices(ClusterStateResponse clusterStateResponse, List<String> indices) {
188+
private List<String> filterClosedAndHiddenIndices(
189+
ClusterStateResponse clusterStateResponse,
190+
List<String> indices,
191+
List<String> requestedIndices
192+
) {
185193
return indices.stream().filter(index -> {
186194
IndexMetadata metadata = clusterStateResponse.getState().getMetadata().indices().get(index);
187-
return metadata.getState().equals(IndexMetadata.State.OPEN) && !IndexMetadata.INDEX_HIDDEN_SETTING.get(metadata.getSettings());
195+
return metadata.getState().equals(IndexMetadata.State.OPEN)
196+
&& (requestedIndices.contains(index) || !IndexMetadata.INDEX_HIDDEN_SETTING.get(metadata.getSettings()));
188197
}).collect(Collectors.toList());
189198
}
190199
}

0 commit comments

Comments
 (0)