zpinto commented on code in PR #2772: URL: https://github.com/apache/helix/pull/2772#discussion_r1552146253
########## helix-core/src/main/java/org/apache/helix/controller/stages/BestPossibleStateCalcStage.java: ########## @@ -131,82 +135,102 @@ public void process(ClusterEvent event) throws Exception { }); } + private String selectSwapInState(StateModelDefinition stateModelDef, Map<String, String> stateMap, + String swapOutInstance) { + // If the swap-in node is live, select state with the following logic: + // 1. If the swap-out instance's replica is in the stateMap: + // - if the swap-out instance's replica is a topState, select the swap-in instance's replica to the topState. + // if another is allowed to be added, otherwise select the swap-in instance's replica to a secondTopState. + // - if the swap-out instance's replica is not a topState or ERROR, select the swap-in instance's replica to the same state. + // - if the swap-out instance's replica is ERROR, select the swap-in instance's replica to the initialState. + // 2. If the swap-out instance's replica is not in the stateMap, select the swap-in instance's replica to the initialState. + // This happens when the swap-out node is offline. + if (stateMap.containsKey(swapOutInstance)) { + if (stateMap.get(swapOutInstance).equals(stateModelDef.getTopState()) || stateMap.get( + swapOutInstance).equals(HelixDefinedState.ERROR.name())) { + // If the swap-out instance's replica is a topState, select the swap-in instance's replica + // to be the topState if the StateModel allows another to be added. If not, select the swap-in + // to be the secondTopState. + String topStateCount = stateModelDef.getNumInstancesPerState(stateModelDef.getTopState()); + if (topStateCount.equals(StateModelDefinition.STATE_REPLICA_COUNT_ALL_CANDIDATE_NODES) + || topStateCount.equals(StateModelDefinition.STATE_REPLICA_COUNT_ALL_REPLICAS)) { + // If the StateModel allows for another replica with the topState to be added, + // select the swap-in instance's replica to the topState. + return stateModelDef.getTopState(); + } else { + // If StateModel does not allow another topState replica to be + // added, select the swap-in instance's replica to be the secondTopState. + return stateModelDef.getSecondTopStates().iterator().next(); + } + } else { + // If the swap-out instance's replica is not a topState or ERROR, select the swap-in instance's replica + // to be the same state + return stateMap.get(swapOutInstance); + } + } else { + // If the swap-out instance's replica is not in the stateMap, select the swap-in instance's replica + // to be the initialState. This happens when the swap-out node is offline. + return stateModelDef.getInitialState(); Review Comment: It is possible for the instance to be in the preference list but for it not to be in the stateMap. This is when the swap out instance is offline. It will disappear from idealState. The assignment is still to the offline node. -- 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. To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org