qqu0127 commented on code in PR #2102:
URL: https://github.com/apache/helix/pull/2102#discussion_r875111176
##########
helix-core/src/main/java/org/apache/helix/task/WorkflowDispatcher.java:
##########
@@ -150,7 +149,10 @@ && isTimeout(workflowCtx.getStartTime(),
workflowCfg.getTimeout())) {
// For workflows that have already reached final states, STOP should not
take into effect.
if (!TaskConstants.FINAL_STATES.contains(workflowCtx.getWorkflowState())
&& TargetState.STOP.equals(targetState)) {
- LOG.info("Workflow {} is marked as stopped. Workflow state is {}",
workflow,
+ if (workflowCtx.getWorkflowState() == TaskState.STOPPED) {
+ return;
+ }
+ LOG.debug("Workflow {} is marked as stopped. Workflow state is {}",
workflow,
workflowCtx.getWorkflowState());
if (isWorkflowStopped(workflowCtx, workflowCfg)) {
workflowCtx.setWorkflowState(TaskState.STOPPED);
Review Comment:
Justification:
Ideally the message `Workflow {} is marked as stopped` should only be logged
once, same for `workflowCtx.setWorkflowState`, but it's triggered for each task
in the workflow. Verified there are tons of messages saying `Workflow ABC is
marked as stopped. Workflow state is STOPPED`, meaning the workflow has been
stopped already.
The only other place that set state STOPPED is in `JobDispatcher` where the
rest of the update logic is handled. As a result, early termination (if state
== TaskState.STOPPED) makes sense here.
--
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]
