You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2021/02/05 00:05:01 UTC
[GitHub] [helix] dasahcc commented on a change in pull request #1637: Skip cancellation for partition reset pending message
dasahcc commented on a change in pull request #1637:
URL: https://github.com/apache/helix/pull/1637#discussion_r570623421
##########
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())
Review comment:
NIT: HelixDefinedState.ERROR.name()
##########
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:
Question: This line logic is expected. But I did not find it from previous code?
----------------------------------------------------------------
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