[GitHub] [helix] jiajunwang commented on a change in pull request #1637: Skip cancellation for partition reset pending message

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

jiajunwang commented on a change in pull request #1637:
URL: https://github.com/apache/helix/pull/1637#discussion_r570664736



##########
File path: helix-core/src/main/java/org/apache/helix/controller/stages/MessageGenerationPhase.java
##########
@@ -271,6 +271,23 @@ private void generateMessage(final Resource resource, final BaseControllerDataPr
     } // end of for-each-partition
   }
 
+  private boolean shouldCreateSTCancellation(Message pendingMessage, String currentState,
+      String desiredState, String initialState) {
+    if (pendingMessage == null) {
+      return false;
+    }
+    if (NO_DESIRED_STATE.equals(desiredState)) {
+      return true;
+    }
+
+    // Don't cancel the ST for below scenarios:
+    // 1. current state has arrived at pending message's to state
+    // 2. pending message is an ERROR reset: ERROR -> initState (eg. OFFLINE)
+    return !currentState.equalsIgnoreCase(pendingMessage.getToState())
+        && !HelixDefinedState.ERROR.toString().equals(pendingMessage.getFromState())
+        && !initialState.equals(pendingMessage.getToState());

Review comment:
       Minor thing, as we discussed offline, the comment is about why we don't cancel, but the code returns true if the message should be canceled. Please try to make them align, or this part tends to be confusing.




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