xyuanlu commented on code in PR #3043:
URL: https://github.com/apache/helix/pull/3043#discussion_r2129437591
##########
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 partitionInstanceCount = instanceStateMap.keySet().size();
+
+ // Get pending upward state transition messages to second top or top
state
+ List<Message> pendingUpwardStateTransitionMessagesToTopOrSecondTopStates
= getPendingTransitionsToTopOrSecondTopStates(
Review Comment:
nit:
the name is too long. pendingUpMsgToTopOrSecondTop
Also we are making the assumption that there is no more than 3 state in the
state machine definition.
##########
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 partitionInstanceCount = instanceStateMap.keySet().size();
+
+ // Get pending upward state transition messages to second top or top
state
+ List<Message> pendingUpwardStateTransitionMessagesToTopOrSecondTopStates
= getPendingTransitionsToTopOrSecondTopStates(
+ resourceName, partition, currentStateOutput, stateModelDef);
+
+ // Initialize replica counter for prioritization
+ int initialReplicaNumber = partitionInstanceCount -
pendingUpwardStateTransitionMessagesToTopOrSecondTopStates.size();
Review Comment:
The partitionInstanceCount contains to be dropped replicas. Then the actual
replica count after converge would be less than the value.
Also please update the comment to be more accurate. what is the "replica
counter for prioritization" here mean? What does replica count include? All non
offline replicas now? All replicas? All replicas after converge?
##########
helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java:
##########
@@ -250,17 +260,30 @@ private void generateMessage(final Resource resource,
final BaseControllerDataPr
pendingMessage, manager, resource, partition,
sessionIdMap, instanceName,
stateModelDef, cancellationMessage, isCancellationEnabled);
} else {
+ // Default currentReplicaNumber is -1 (provides metadata for
participant-side prioritization)
+ int currentReplicaNumber = -1;
Review Comment:
I think providing the currentReplicaNumber for both upward state transition
and downward state transition both help participant side. Why we want to set -1
for upward state transition messages?
##########
helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java:
##########
@@ -290,6 +313,62 @@ 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);
Review Comment:
Can we inline this func? It just one line and no need to separate the logic.
--
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]
