- 
                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.