dasahcc commented on a change in pull request #986:
URL: https://github.com/apache/helix/pull/986#discussion_r418776796
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/AbstractRebalancer.java
##########
@@ -305,45 +305,60 @@ public static int getStateCount(String state,
StateModelDefinition stateModelDef
Set<String> liveAndEnabled = new HashSet<>(liveInstances);
liveAndEnabled.removeAll(disabledInstancesForPartition);
- for (String state : statesPriorityList) {
- // Use the the specially ordered preferenceList for choosing instance
for top state.
- if (state.equals(statesPriorityList.get(0))) {
- List<String> preferenceListForTopState = new
ArrayList<>(preferenceList);
- Collections.sort(preferenceListForTopState,
- new TopStatePreferenceListComparator(currentStateMap,
stateModelDef));
- preferenceList = preferenceListForTopState;
- }
+ // Sort the instances based on replicas' state priority in the current
state
+ List<String> sortedPreferenceList = new ArrayList<>(preferenceList);
+ sortedPreferenceList.sort(new StatePriorityComparator(currentStateMap,
stateModelDef));
+ // Assign the state to the instances that appear in the preference list.
+ for (String state : statesPriorityList) {
int stateCount =
getStateCount(state, stateModelDef, liveAndEnabled.size(),
preferenceList.size());
for (String instance : preferenceList) {
if (stateCount <= 0) {
- break;
+ break; // continue assigning for the next state
+ }
+ if (assigned.contains(instance) || !liveAndEnabled.contains(instance))
{
+ continue; // continue checking for the next available instance
}
- if (!assigned.contains(instance) && liveAndEnabled.contains(instance))
{
- if
(HelixDefinedState.ERROR.toString().equals(currentStateMap.get(instance))) {
- bestPossibleStateMap.put(instance,
HelixDefinedState.ERROR.toString());
- } else {
- bestPossibleStateMap.put(instance, state);
- stateCount--;
+ String proposedInstance = instance;
+ // Additional check and alternate the assignment for reducing top
state handoff.
+ if (state.equals(stateModelDef.getTopState()) &&
!stateModelDef.getSecondTopStates()
Review comment:
I think you already sorted the preference list based on the state. Why
not just replace the looping of preference list with the sorted list but add
this logic?
This logic makes two order of preference list intersected each other that
logic is not very clear.
----------------------------------------------------------------
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]
