zpinto commented on code in PR #2772:
URL: https://github.com/apache/helix/pull/2772#discussion_r1575656244


##########
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:
   I changed it to only bootstrap replicas on swap in node based on the 
corresponding instances in the state map. This means that offline instances 
will not have its assigned partitions on the swap in node because we don't know 
what the right state would be to copy over.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to