Accumulate compute() calls and iterations between convergences in DesiredBalanceComputer#126008
Conversation
elasticsearchmachine
added
needs:triage
JeremyDahlgren
added
Team:Distributed Coordination (obsolete)
elasticsearchmachine
removed
the
needs:triage
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
JeremyDahlgren
added
>enhancement
needs:triage
elasticsearchmachine
removed
the
needs:triage
|
Hi @JeremyDahlgren, I've created a changelog YAML for you. |
JeremyDahlgren
force-pushed
the
fix/100850
branch
from
881da5b to
aeb8946
Compare
DaveCTurner
left a comment
DaveCTurner
left a comment
There was a problem hiding this comment.
Good stuff, I left some comments
...main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java
Outdated
Show resolved
Hide resolved
| "Desired balance computation for [{}] converged after [{}] and [{}] iterations, " | ||
| + "[{}] compute() calls with [{}] total iterations since last convergence [{}] ago", |
There was a problem hiding this comment.
Optional nit, I'd use a multi-line string here, they're slightly easier to read and less hassle to modify in future:
| "Desired balance computation for [{}] converged after [{}] and [{}] iterations, " | |
| + "[{}] compute() calls with [{}] total iterations since last convergence [{}] ago", | |
| """ | |
| Desired balance computation for [{}] converged after [{}] and [{}] iterations, \ | |
| [{}] compute() calls with [{}] total iterations since last convergence [{}] ago""", |
Although I note that we use regular string concatenation in other places nearby already. Up to you.
...main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/cluster/routing/allocation/allocator/DesiredBalanceComputer.java
Outdated
Show resolved
Hide resolved
| "Desired balance computation for [{}] is still not converged after [{}] and [{}] iterations, " | ||
| + "[{}] compute() calls with [{}] total iterations since last convergence [{}] ago", |
There was a problem hiding this comment.
This message is user-visible and I think as worded here it might raise some support questions - what is compute() for instance? Could we instead rephrase it so that the numbers in still not converged after [{}] and [{}] iterations reflect the activity since the last convergence rather than the last interrupt? Then maybe say something like resumed computation [{}] times with [{}] iterations since the last resumption [{}] ago to expose the numbers about the current computation. And maybe only do that if numComputeCallsSinceLastConverged > 1 so the message can be simpler when possible?
There was a problem hiding this comment.
Refactored per your recommendation. I have an if/else here testing the number of computations, and also for the convergence message. There is some duplication going on in these two areas, but decided to leave as is. Please let me know what you think.
Add tracking of the number of compute() calls and total iterations between convergences in the DesiredBalanceComputer, along with the time since the last convergence. These are included in the log message when the computer doesn't converge. Closes elastic#100850.
JeremyDahlgren
force-pushed
the
fix/100850
branch
from
dd5f442 to
ffdea84
Compare
…iredBalanceComputer (elastic#126008) Add tracking of the number of compute() calls and total iterations between convergences in the DesiredBalanceComputer, along with the time since the last convergence. These are included in the log message when the computer doesn't converge. Closes elastic#100850.
3 participants
Add tracking of the number of compute() calls and total iterations between convergences in the DesiredBalanceComputer, along with the time since the last convergence. These are included in the log message when the computer doesn't converge.
Closes #100850.