[Fix-17817][Master]Add workflow timeout event and handle#18063
Open
njnu-seafish wants to merge 9 commits intoapache:devfrom
Open
[Fix-17817][Master]Add workflow timeout event and handle#18063njnu-seafish wants to merge 9 commits intoapache:devfrom
njnu-seafish wants to merge 9 commits intoapache:devfrom
Conversation
njnu-seafish
requested review from
SbloodyS,
caishunfeng and
ruanwenjun
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @AllArgsConstructor(access = AccessLevel.PRIVATE) | ||
| public class WorkflowTimeoutLifecycleEvent extends AbstractWorkflowLifecycleLifecycleEvent { | ||
|
|
||
| private IWorkflowExecutionRunnable workflowExecutionRunnable; |
Check notice
Code scanning / CodeQL
Missing Override annotation Note
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #17817 by introducing a workflow-level timeout lifecycle event/handler so that configured workflow timeout alerts are actually emitted by the master engine.
Changes:
- Add a new
WorkflowTimeoutLifecycleEvent+WorkflowTimeoutLifecycleEventHandler, and wire publishing fromWorkflowStartLifecycleEventHandler. - Add
TIMEOUTtoWorkflowLifecycleEventTypeto support routing the new event. - Add service/DAO plumbing for sending workflow-timeout alerts and refactor alert persistence helper naming; tighten task-timeout validation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/alert/WorkflowAlertManager.java | Adds sendWorkflowTimeoutAlert entrypoint to reach DAO alert creation. |
| dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandler.java | New handler to send workflow timeout alerts when the timeout event fires. |
| dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/handler/WorkflowStartLifecycleEventHandler.java | Publishes workflow timeout event at workflow start (if timeout configured). |
| dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java | New delay event representing workflow timeout. |
| dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/lifecycle/WorkflowLifecycleEventType.java | Adds TIMEOUT event type. |
| dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/task/lifecycle/event/TaskTimeoutLifecycleEvent.java | Tightens validation to require timeout minutes > 0. |
| dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/AlertDao.java | Adds validation for workflow timeout alert inputs and consolidates timeout-alert saving helper. |
| dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AlertType.java | Updates comment to reflect actual enum values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+53
to
+54
| long delayTime = System.currentTimeMillis() - workflowInstance.getStartTime().getTime() | ||
| + TimeUnit.MINUTES.toMillis(timeout); |
|
|
||
| public static WorkflowTimeoutLifecycleEvent of(IWorkflowExecutionRunnable workflowExecutionRunnable) { | ||
| final WorkflowInstance workflowInstance = workflowExecutionRunnable.getWorkflowInstance(); | ||
| checkState(workflowInstance != null, "The workflow instance must be initialized before retrying."); |
| private void workflowTimeoutMonitor(final IWorkflowExecutionRunnable workflowExecutionRunnable) { | ||
| final WorkflowInstance workflowInstance = workflowExecutionRunnable.getWorkflowInstance(); | ||
| if (workflowInstance.getTimeout() <= 0) { | ||
| log.debug("The workflow {} timeout {} is invalided, so the timeout monitor will not be started.", |
ruanwenjun
reviewed
Mar 18, 2026
| final int timeout = workflowInstance.getTimeout(); | ||
| checkState(timeout > 0, "The workflow timeout: %s must > 0 minutes", timeout); | ||
|
|
||
| long delayTime = System.currentTimeMillis() - workflowInstance.getStartTime().getTime() |
Member
There was a problem hiding this comment.
When the workflow instance is rerun the startTime will not change. You should use restartTime .
Contributor
Author
There was a problem hiding this comment.
When the workflow instance is rerun the startTime will not change. You should use
restartTime.
good
| private void workflowTimeoutMonitor(final IWorkflowExecutionRunnable workflowExecutionRunnable) { | ||
| final WorkflowInstance workflowInstance = workflowExecutionRunnable.getWorkflowInstance(); | ||
| if (workflowInstance.getTimeout() <= 0) { | ||
| log.debug("The workflow {} timeout {} is invalided, so the timeout monitor will not be started.", |
Contributor
Author
There was a problem hiding this comment.
The log is incorrect.
fix it
ruanwenjun
reviewed
Mar 18, 2026
| * Pause the workflow instance | ||
| */ | ||
| PAUSE, | ||
|
|
Member
There was a problem hiding this comment.
Please don't change the unrelated code.
Contributor
Author
There was a problem hiding this comment.
Please don't change the unrelated code.
ok
Contributor
Author
added |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Was this PR generated or assisted by AI?
NO
Purpose of the pull request
close #17817
This bug involves a critical feature and has been stagnant for several months. Since I actually resolved the task timeout and workflow timeout issue quite some time ago, I would like to finalize the process and get it closed.
Brief change log
Add workflow timeout event and handle
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
Pull Request Notice
Pull Request Notice
If your pull request contains incompatible change, you should also add it to
docs/docs/en/guide/upgrade/incompatible.md