-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Add new Shards Capacity Health Indicator #94552
Conversation
Pinging @elastic/es-data-management (Team:Data Management) |
Hi @HiDAl, I've created a changelog YAML for you. |
server/src/main/java/org/elasticsearch/health/node/ShardLimitsHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardLimitsHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrei Dan <andrei.dan@elastic.co>
fix bad definition name
@elasticsearchmachine run elasticsearch-ci/part-3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this Pablo
This generally looks great, I left a few rather minor comments
@tylerperk can you please go through the copy (as most of the output of the API is presented in the UI)
@shubhaat would you like to have a go through the copy?
...ernalClusterTest/java/org/elasticsearch/health/node/ShardLimitsHealthIndicatorServiceIT.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardLimitsHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardLimitsHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardLimitsHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardLimitsHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardLimitsHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardLimitsHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/indices/ShardLimitValidator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/indices/ShardLimitValidator.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/indices/ShardLimitValidator.java
Outdated
Show resolved
Hide resolved
@HiDAl the docs needs an update too https://github.com/elastic/elasticsearch/blob/main/docs/reference/health/health.asciidoc |
add tests to public methods remove method which could lead to confusions
this makes the method generic enough, so can easily test the internal logic
@andreidan I did rename the indicator to
|
@elasticsearchmachine run elasticsearch-ci/part-3 |
@HiDAl the Can you please update the PR description to reflect the latest state? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for iterating on this Pablo. This LGTM 🚀 - left a few very minor suggestions
...r/src/test/java/org/elasticsearch/health/node/ShardsCapacityHealthIndicatorServiceTests.java
Outdated
Show resolved
Hide resolved
...r/src/test/java/org/elasticsearch/health/node/ShardsCapacityHealthIndicatorServiceTests.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardsCapacityHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardsCapacityHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/health/node/ShardsCapacityHealthIndicatorService.java
Outdated
Show resolved
Hide resolved
@andreidan I've applied all the recommended changes :) |
@elasticmachine update branch |
@elasticsearchmachine run elasticsearch-ci/part-1 |
Introduces a new Health Indicator to check the cluster's health from the shards' capacity perspective. It calculates the amount of available room for data and frozen groups, according to the following rules: ``` if data or frozen nodes have less than 5 shards -> RED if data or frozen nodes have less than 10 shards -> YELLOW otherwise -> GREEN ```
In elastic#94552 was introduced a new Health Service to check the shards capacity of the cluster which will replace this Deprecation Check.
In elastic#94552 was introduced a new Health Service which checks the shards capacity of the cluster. This method is replacing the Old `ClusterDeprecationChecks#checkShard` used to validate the feasibility of upgrading a cluster.
Introduces a new Health Indicator to check the cluster's health from the shards' capacity perspective.
It calculates the amount of available room for
data
andfrozen
groups, according to the following rules:This is the output in case the cluster is unhealthy:
relates #94079 and #91119