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

Reply via email to