Optimize complicated if-else in routing table. #56870
Conversation
howardhuanghua
changed the title
original-brownbear
added
:Distributed Coordination/Allocation
|
Pinging @elastic/es-distributed (:Distributed/Allocation) |
elasticmachine
added
the
Team:Distributed (Obsolete)
| } | ||
| } else if (currentNumberOfReplicas > numberOfReplicas) { | ||
| int delta = currentNumberOfReplicas - numberOfReplicas; | ||
| if (delta <= 0) { |
There was a problem hiding this comment.
Maybe just remove this condition as it's obviously always false and keep the rest the same (maybe also inline delta like we have in the "add" case). Seems to me that makes the logic easiest to digest if we want to change it?
There was a problem hiding this comment.
@original-brownbear Thanks for checking, I think we could pre-compute the delta and seperate the add and remove operation. Please help to review the update.
There was a problem hiding this comment.
I'm with @original-brownbear here, using Math.abs(expr) and then branching on the sign of expr looks weird to me. Let's avoid the use of Math#abs (and therefore the super-broad @SuppressForbidden annotation).
There was a problem hiding this comment.
OK. I removed abs method. Thank you @DaveCTurner .
|
Jenkins test this |
|
Jenkins run elasticsearch-ci/bwc (unrelated) |
|
@elasticmachine update branch |
|
user doesn't have permission to update head repository |
|
@howardhuanghua could you please merge latest |
howardhuanghua
force-pushed
the
optimize_condition
branch
from
46f44bd to
6389d4b
Compare
howardhuanghua
force-pushed
the
optimize_condition
branch
from
6389d4b to
2bb15b2
Compare
|
@original-brownbear , I have merged the lastest master and updated this branch. Please help to re-trigger the test again. Thank you. |
|
Jenkins test this |
`delta` is always positive here.
`delta` is always positive here. Co-authored-by: Howard <[email protected]>
Optimize the complicated if-else logic in RoutingTable.