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


##########
helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java:
##########
@@ -250,17 +260,32 @@ private void generateMessage(final Resource resource, 
final BaseControllerDataPr
                     pendingMessage, manager, resource, partition, 
sessionIdMap, instanceName,
                     stateModelDef, cancellationMessage, isCancellationEnabled);
           } else {
+            // Default currentReplicaNumber is -1 (no prioritization)

Review Comment:
   Maybe better wording? We are not prioritize anything, just add more info for 
user to prioritize if they want.



##########
helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java:
##########
@@ -163,6 +163,16 @@ private void generateMessage(final Resource resource, 
final BaseControllerDataPr
       // desired-state->list of generated-messages
       Map<String, List<Message>> messageMap = new HashMap<>();
 
+      // Calculate metadata for message prioritization
+      int totalInstanceCount = instanceStateMap.keySet().size();
+
+      // Get pending upward state transition messages to second top or top 
state
+      List<Message> pendingUpwardStateTransitionMessages = 
getPendingUpwardStateTransitionMessages(
+          resourceName, partition, currentStateOutput, stateModelDef);
+
+      // Initialize replica counter for prioritization
+      int initialReplicaNumber = totalInstanceCount - 
pendingUpwardStateTransitionMessages.size();

Review Comment:
   does it include the to be dropped one?



##########
helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java:
##########
@@ -163,6 +163,16 @@ private void generateMessage(final Resource resource, 
final BaseControllerDataPr
       // desired-state->list of generated-messages
       Map<String, List<Message>> messageMap = new HashMap<>();
 
+      // Calculate metadata for message prioritization
+      int totalInstanceCount = instanceStateMap.keySet().size();

Review Comment:
   I guess the totalInstanceCount is the instance containing the partition. The 
naming is confusing. The name suggesting all instances in the cluster.



##########
helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java:
##########
@@ -250,17 +260,32 @@ private void generateMessage(final Resource resource, 
final BaseControllerDataPr
                     pendingMessage, manager, resource, partition, 
sessionIdMap, instanceName,
                     stateModelDef, cancellationMessage, isCancellationEnabled);
           } else {
+            // Default currentReplicaNumber is -1 (no prioritization)
+            int currentReplicaNumber = -1;
+            // Check if this is an upward state transition from non-second top 
state to second top
+            // or top state
+            if (isUpwardStateTransition(currentState, nextState, stateModelDef)
+                && !stateModelDef.getSecondTopStates().contains(currentState)
+                && (isSecondTopState(nextState, stateModelDef)
+                    || isTopState(nextState, stateModelDef))
+                && !isInPendingMessages(resourceName, partition, instanceName, 
currentState,
+                    nextState, pendingUpwardStateTransitionMessages)) {
+
+              // Assign the replica number for prioritization
+              currentReplicaNumber = initialReplicaNumber--;
+            }
+
             // Create new state transition message
             message = MessageUtil
                 .createStateTransitionMessage(manager.getInstanceName(), 
manager.getSessionId(),
                     resource, partition.getPartitionName(), instanceName, 
currentState, nextState,
-                    sessionIdMap.get(instanceName), stateModelDef.getId());
+                    sessionIdMap.get(instanceName), stateModelDef.getId(), 
currentReplicaNumber);

Review Comment:
   Does it mean we will pass -1 for some case? This is confusing for user



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