GrantPSpencer commented on code in PR #3043:
URL: https://github.com/apache/helix/pull/3043#discussion_r2121966378


##########
helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java:
##########
@@ -290,6 +315,113 @@ private void generateMessage(final Resource resource, 
final BaseControllerDataPr
     } // end of for-each-partition
   }
 
+  /**
+   * Check if a state is a top state
+   * @param state The state to check
+   * @param stateModelDef The state model definition
+   * @return True if it's a top state, false otherwise
+   */
+  private boolean isTopState(String state, StateModelDefinition stateModelDef) 
{
+    return stateModelDef.getTopState().contains(state);
+  }
+
+  /**
+   * Check if a state is a second top state
+   * @param state The state to check
+   * @param stateModelDef The state model definition
+   * @return True if it's a second top state, false otherwise
+   */
+  private boolean isSecondTopState(String state, StateModelDefinition 
stateModelDef) {
+    return stateModelDef.getSecondTopStates().contains(state);
+  }
+
+  /**
+   * Check if a state transition is already in the pending messages
+   * @param resourceName The resource name
+   * @param partition The partition
+   * @param instanceName The instance name
+   * @param fromState The from state
+   * @param toState The to state
+   * @param pendingMessages The list of pending messages
+   * @return True if the state transition is already in pending messages, 
false otherwise
+   */
+  private boolean isInPendingMessages(String resourceName, Partition 
partition, String instanceName,
+      String fromState, String toState, List<Message> pendingMessages) {
+
+    for (Message message : pendingMessages) {
+      if (message.getResourceName().equals(resourceName)
+          && message.getPartitionName().equals(partition.getPartitionName())
+          && message.getTgtName().equals(instanceName) && 
message.getFromState().equals(fromState)
+          && message.getToState().equals(toState)) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  /**
+   * Check if a state transition is upward
+   * @param fromState The from state
+   * @param toState The to state
+   * @param stateModelDef The state model definition
+   * @return True if it's an upward state transition, false otherwise
+   */
+  private boolean isUpwardStateTransition(String fromState, String toState,
+      StateModelDefinition stateModelDef) {
+
+    if (fromState == null || toState == null) {
+      return false;
+    }
+
+    Map<String, Integer> statePriorityMap = 
stateModelDef.getStatePriorityMap();
+
+    Integer fromStateWeight = statePriorityMap.get(fromState);
+    Integer toStateWeight = statePriorityMap.get(toState);
+
+    if (fromStateWeight == null || toStateWeight == null) {
+      return false;
+    }
+
+    return toStateWeight < fromStateWeight;
+  }
+
+  /**
+   * Get pending upward state transition messages from non-second top state to 
second top or top
+   * state
+   * @param resourceName The resource name
+   * @param partition The partition
+   * @param currentStateOutput The current state output
+   * @param stateModelDef The state model definition
+   * @return List of pending messages for upward state transitions
+   */
+  private List<Message> getPendingUpwardStateTransitionMessages(String 
resourceName,
+      Partition partition, CurrentStateOutput currentStateOutput,
+      StateModelDefinition stateModelDef) {
+    List<Message> pendingUpwardSTMessages = new ArrayList<>();
+
+    // Instance -> PendingMessage
+    Map<String, Message> pendingMessages =
+        currentStateOutput.getPendingMessageMap(resourceName, partition);
+
+    if (pendingMessages != null && !pendingMessages.isEmpty()) {
+      for (Message message : pendingMessages.values()) {
+        String fromState = message.getFromState();
+        String toState = message.getToState();
+
+        // Check if it's an upward state transition from non-second top state 
to second top or top
+        // state
+        if (isUpwardStateTransition(fromState, toState, stateModelDef)
+            && !isSecondTopState(fromState, stateModelDef)
+            && (isSecondTopState(toState, stateModelDef) || 
isTopState(toState, stateModelDef))) {

Review Comment:
   I'm not sure I completely follow, sorry. 
   
   1.)Can you confirm my understanding that `!isSecondTopState(fromState, 
stateModelDef)` is so we don't consider nodes going from FOLLOWER->LEADER when 
we subtract pending messages from the initialReplicaNumber? 
   Is this so that in this case:
   R1 FOLLOWER (pending FOLLOWER->LEADER)
   R2 FOLLOWER
   R3 OFFLINE
   
   then R3's transition replica number will be 3 rather than 2
   
   2.) Will pending message ever have a TO state that is more than 1 hop from 
the FROM state? These are built from existing messages in ZK and my assumption 
is they will always be 1 hop. Ultimately this a NIT so don't consider this 
comment a blocker, can have discussion offline later as well



-- 
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