-
Notifications
You must be signed in to change notification settings - Fork 25.7k
allocation: add timing and iteration to logging #139150
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
base: main
Are you sure you want to change the base?
allocation: add timing and iteration to logging #139150
Conversation
The DesiredBalanceComputer now has three groups of timing and iteration counts: the current round iteration and duration, the since-recompute iterations, round count, and duration, and the time since convergence. This logging change includes all available information in the logs, and groups them together by kind.
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
DaveCTurner
left a comment
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, much easier to review this separately. I left a couple of small suggestions for improvement that also apply to the same changes further down. Overall tho it looks good to add these extra details to these messages.
| """ | ||
| Desired balance computation for [%d] converged after [%s] and [%d] iterations, \ | ||
| resumed computation [%d] times with [%d] iterations since the last resumption [%s] ago""", | ||
| Desired balance computation for [%d] converged after [%s] and [%d] iterations this round, \ |
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.
I think the total time & number of iterations is more important than the number since it was last interrupted - could we reverse the order so that the total numbers come first?
| () -> Strings.format( | ||
| "Desired balance computation for [%d] converged after [%s] and [%d] iterations", | ||
| """ | ||
| Desired balance computation for [%d] converged after [%s] and [%d] iterations this round, \ |
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.
"this round" implies there have been other rounds, but that's not the case here. Maybe "in a single round" would be clearer?
The DesiredBalanceComputer now has three groups of timing and iteration counts: the current round iteration and duration, the since-recompute iterations, round count, and duration, and the time since convergence. This logging change includes all available information in the logs, and groups them together by kind.