jiajunwang commented on a change in pull request #1098:
URL: https://github.com/apache/helix/pull/1098#discussion_r441771981
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/AbstractRebalancer.java
##########
@@ -385,10 +389,19 @@ public static int getStateCount(String state,
StateModelDefinition stateModelDef
// If the desired state is the top state, but the instance cannot be
transited to the
// top state in one hop, try to keep the top state on current host
or a host with a closer
// state.
- for (String currentStatePrioritizedInstance : sortedPreferenceList) {
- if (!assigned.contains(currentStatePrioritizedInstance) &&
liveAndEnabled
- .contains(currentStatePrioritizedInstance)) {
+ while (currentStatePrioritizedInstanceIter.hasNext()) {
+ // Note that it is safe to check the prioritized instance items
only once here.
+ // Since the only possible condition when we don't use an instance
in this list is that
+ // it has been assigned with some state. And this is not
revertable in this method. So
+ // checking it one more time later will only waste time.
+ String currentStatePrioritizedInstance =
currentStatePrioritizedInstanceIter.next();
+ if (!assigned.contains(currentStatePrioritizedInstance)) {
Review comment:
I moved this logic above. Inactive nodes have been removed from the
currentStatePrioritizedInstanceList. This shortens the instance list and saves
some computing.
Note that we may don't want to do it for the preferencelist since it is not
deepcopy.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
