Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing _list/shards API for closed indices #16606

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gargharsh3134
Copy link
Contributor

@gargharsh3134 gargharsh3134 commented Nov 11, 2024

Description

_list/shards API throws index_closed_exception whenever the cluster has closed indices. This change is to resolve and ignore the closed indices from getting explicitly passed to IndicesStatsAction.

Related Issues

Resolves #16626

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link
Contributor

❌ Gradle check result for c62e5f2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 691803c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@github-actions github-actions bot added bug Something isn't working Cluster Manager labels Nov 13, 2024
Copy link
Contributor

✅ Gradle check result for 4663f2b: SUCCESS

Copy link

codecov bot commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.

Project coverage is 72.14%. Comparing base (10873f1) to head (4663f2b).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...admin/cluster/shards/TransportCatShardsAction.java 0.00% 3 Missing ⚠️
...rch/action/pagination/ShardPaginationStrategy.java 92.85% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #16606      +/-   ##
============================================
- Coverage     72.15%   72.14%   -0.02%     
- Complexity    65145    65185      +40     
============================================
  Files          5315     5318       +3     
  Lines        303573   303832     +259     
  Branches      43925    43959      +34     
============================================
+ Hits         219039   219189     +150     
- Misses        66587    66656      +69     
- Partials      17947    17987      +40     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +182 to +198
CountDownLatch latch = new CountDownLatch(1);
client().execute(CatShardsAction.INSTANCE, shardsRequest, new ActionListener<CatShardsResponse>() {
@Override
public void onResponse(CatShardsResponse catShardsResponse) {
List<ShardRouting> shardRoutings = catShardsResponse.getResponseShards();
assertFalse(shardRoutings.stream().anyMatch(shard -> shard.getIndexName().equals("test-3")));
latch.countDown();
}

@Override
public void onFailure(Exception e) {
fail();
latch.countDown();
}
});
latch.await();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can skip the latch by using the variant the returns a future, something like:

ActionFuture<CatShardsResponse> response = client().execute(CatShardsAction.INSTANCE, shardsRequest);
List<ShardRouting> shardRoutings = response.get().getResponseShards();
assertFalse(shardRoutings.stream().anyMatch(shard -> shard.getIndexName().equals("test-3")));

}

private static Predicate<IndexMetadata> indexNameFilter(String lastIndexName) {
return metadata -> metadata.getIndex().getName().equals(lastIndexName);
}

private static Predicate<IndexMetadata> indexStateFilter(IndicesOptions indicesOptions) {
if (Objects.isNull(indicesOptions) || !indicesOptions.forbidClosedIndices()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Objects.isNull exists to use as a predicate. The canonical way to do this is just indicesOptions == null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Cluster Manager
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[BUG] _list/shards API failing with index_closed_exception
2 participants