[GitHub] [helix] alirezazamani commented on a change in pull request #1040: Remove previousAssignment in processTaskWithPendingMessage method

[GitBox <gi...@apache.org>]

alirezazamani commented on a change in pull request #1040:
URL: https://github.com/apache/helix/pull/1040#discussion_r432771509



##########
File path: helix-core/src/main/java/org/apache/helix/task/AbstractTaskDispatcher.java
##########
@@ -449,43 +448,31 @@ private void updatePartitionInformationInJobContext(CurrentStateOutput currentSt
    * @param paMap
    * @param assignedPartitions
    */
-  private void processTaskWithPendingMessage(ResourceAssignment prevAssignment, Integer pId,
-      String pName, String instance, Message pendingMessage, TaskState jobState,
-      TaskPartitionState currState, Map<Integer, PartitionAssignment> paMap,
-      Map<String, Set<Integer>> assignedPartitions) {
-
-    // stateMap is a mapping of Instance -> TaskPartitionState (String)
-    Map<String, String> stateMap = prevAssignment.getReplicaMap(new Partition(pName));
-    if (stateMap != null) {
-      String prevState = stateMap.get(instance);
-      if (!pendingMessage.getToState().equals(prevState)) {
-        LOG.warn(String.format(
-            "Task pending to-state is %s while previous assigned state is %s. This should not"
-                + "happen.",
-            pendingMessage.getToState(), prevState));
+  private void processTaskWithPendingMessage(Integer pId, String pName, String instance,
+      Message pendingMessage, TaskState jobState, TaskPartitionState currState,
+      Map<Integer, PartitionAssignment> paMap, Map<String, Set<Integer>> assignedPartitions) {
+
+    if (jobState == TaskState.TIMING_OUT && currState == TaskPartitionState.INIT
+        && pendingMessage.getToState().equals(TaskPartitionState.RUNNING.name())) {

Review comment:
       This part is basically similar to the previous version of the code. 
   
   From the comments and the code: If there is a pending message for this task and this pending message is INIT->RUNNING, if the job goes to TIMING_OUT state, we need to cancel this state transition. Hence we would set the task back to INIT state in this case. Then if Helix can cancel the ST (if enabled) this ST will be cancelled by controller.
   
   Also without this change we are in the same situation: We decided to send INIT->RUNNING message for this task (hence in previous pipeline we set previousAssignment to RUNNING and send INIT-> RUNNING message), in next pipelines while the message has not been processed yet, job goes to TIMING_OUT state, hence this message is not needed we can cancel it (if enabled). In this case we set the paMap back to INIT.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org