-
Notifications
You must be signed in to change notification settings - Fork 4.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fix-14303][Master] Workflow with sub_process task can't be stopped w… #14343
Conversation
@SbloodyS need a label to continue check |
Codecov Report
@@ Coverage Diff @@
## 3.1.8-prepare #14343 +/- ##
================================================
Coverage ? 38.20%
Complexity ? 3980
================================================
Files ? 999
Lines ? 36902
Branches ? 4265
================================================
Hits ? 14097
Misses ? 21191
Partials ? 1614 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
SonarCloud Quality Gate failed.
|
} else if (state.isStop()) { | ||
return TaskExecutionStatus.KILL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we workflow stop is kill?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
STOP is workflow state, and KILL is task state. AFAK, task has no STOP state (TaskExecutionStatus
has STOP state, but it has not been used anywhere). TaskExecutionStatus
has code:
public boolean isFinished() {
return isSuccess() || isKill() || isFailure() || isPause();
}
PAUSE workflow has PAUSE task, only KILL task state can match STOP workflow state, both of these indicate termination.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
The pr target should change to dev branch first? |
In dev branch, sub_process has been refactor #13948 , and there is no relevant code anymore. BTW, I don't know if dev branch have the same issue. |
I browsed sub_process code in dev branch, the new logic task framework only use three state: RUNNING, SUCCESS, FAILED.(Reference code: |
Purpose of the pull request
fix #14303
Brief change log
When the sub workflow is finished, check the workflow state and transition to task status.
Verify this pull request
This pull request is already covered by existing tests, such as org.apache.dolphinscheduler.server.master.SubProcessTaskTest#testFinish.