jiajunwang commented on a change in pull request #1098:
URL: https://github.com/apache/helix/pull/1098#discussion_r441773413
##########
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)) {
proposedInstance = currentStatePrioritizedInstance;
+ // If we find a different instance for the partition placement,
then we need to check
+ // the same instance again or it will not be assigned with any
partitions.
+ if (!proposedInstance.equals(instance)) {
+ i--;
Review comment:
This is a trade-off.
I agree queue would be much simpler. And with the queue, I can do more
optimization.
However, since this calculation will be run many times. Creating a queue
here might prolong the rebalance time a lot. Let me see if I can profile this.
----------------------------------------------------------------
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]
