-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ES-10037 Track the peak indexing load for each shard #125521
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
ES-10037 Track the peak indexing load for each shard #125521
Conversation
b96b1c3 to
36cafdc
Compare
bf752ef to
d2ff0cd
Compare
This tracks the highest value seen for the recent write load metric any time the stats for a shard was computed, exposes this value alongside the recent value, and persists it in index metadata alongside it too. The new test in `IndexShardTests` is designed to more thoroughly test the recent write load metric previously added, as well as to test the peak metric being added here. ES-10037
d2ff0cd to
2b2cf6d
Compare
|
Pinging @elastic/es-data-management (Team:Data Management) |
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.
LGTM, I left one super-minor comment
| if (shardsPeakWriteLoad != null && shardsPeakWriteLoad.size() != shardsUptimeInMillis.size()) { | ||
| assert false : "IndexWriteLoad.create() was called with non-matched lengths for shardsPeakWriteLoad and shardUptimeInMillis"; | ||
| throw new IllegalArgumentException( | ||
| "The same number of shard write loads and shard uptimes should be provided, but " |
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.
super minor, but should this say "peak write loads" instead of just "write loads"?
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, done (and I did the same to the 'recent' one above, which had the same issue).
This tracks the highest value seen for the recent write load metric any time the stats for a shard was computed, exposes this value alongside the recent value, and persists it in index metadata alongside it too. The new test in `IndexShardTests` is designed to more thoroughly test the recent write load metric previously added, as well as to test the peak metric being added here. ES-10037 #comment Added peak load metric in elastic#125521
This adds the fields added in elastic/elasticsearch#125521 and elastic/elasticsearch#124652 to the spec.
This adds the fields added in elastic/elasticsearch#125521 and elastic/elasticsearch#124652 to the spec.
This tracks the highest value seen for the recent write load metric any time the stats for a shard was computed, exposes this value alongside the recent value, and persists it in index metadata alongside it too.
The new test in
IndexShardTestsis designed to more thoroughly test the recent write load metric previously added, as well as to test the peak metric being added here.