[Fix-17817] [Master] Fix workflow timeout alerts failed by Zzih96 · Pull Request #17819 · apache/dolphinscheduler

Zzih96 · 2025-12-24T02:00:41Z

Purpose of the pull request

fix #17817

Brief change log

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

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

...inscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java

ruanwenjun

LGTM, it's better to add IT case.

ruanwenjun · 2025-12-29T03:31:18Z

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/AlertDao.java

+     * @param modifyBy      modifyBy
     */
-    public void sendWorkflowTimeoutAlert(WorkflowInstance workflowInstance, ProjectUser projectUser) {
+    public void sendWorkflowTimeoutAlert(WorkflowInstance workflowInstance, ProjectUser projectUser, String modifyBy) {


Please don't change this, this PR should only fix the alert bug.

ruanwenjun · 2025-12-29T03:36:04Z

...inscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java

+        // Calculate remaining time until timeout: timeout - elapsed time
+        long delayTime = TimeUnit.MINUTES.toMillis(timeout)
+                - (System.currentTimeMillis() - workflowInstance.getStartTime().getTime());
+        // Ensure delayTime is not negative (trigger immediately if already timeout)


I am not clear in which case the delayTime might be negative, since System.currentTimeMillis() - workflowInstance.getStartTime().getTime() should always > 0.

ruanwenjun · 2025-12-29T03:38:00Z

...er/server/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandler.java

+    private void doWorkflowTimeoutAlert(final WorkflowInstance workflowInstance) {
+        // ProjectUser will be built in WorkflowAlertManager
+        workflowAlertManager.sendWorkflowTimeoutAlert(workflowInstance, null);


Should fix like #17818, otherwise will throw NPE

njnu-seafish · 2026-01-09T07:58:38Z

...er/server/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandler.java

In our environment, we've already resolved the task timeout alerts and workflow timeout alerts. Here's how I determine whether a workflow has completed:

final IWorkflowExecutionGraph workflowExecutionGraph = workflowExecutionRunnable.getWorkflowExecutionGraph();
if (workflowExecutionGraph.isAllTaskExecutionRunnableChainFinish()) {
// all the TaskExecutionRunnable chain in the graph is finish, means the workflow is already finished.
return;
}

isFinalState() reflects the persistent final state, making it more reliable. isAllTaskExecutionRunnableChainFinish(), on the other hand, only reflects the task completion state in memory; it may not yet have transitioned to the final workflow state.

isFinalState() reflects the persistent final state, making it more reliable. isAllTaskExecutionRunnableChainFinish(), on the other hand, only reflects the task completion state in memory; it may not yet have transitioned to the final workflow state.

In my opinion, If the workflow is already in a completed state in memory, does that mean timeout handling is no longer meaningful?

njnu-seafish · 2026-01-09T08:05:46Z

...inscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java

Being equal to zero doesn't seem reasonable either.

final int timeout = workflowInstance.getTimeout();
checkState(timeout > 0, "The workflow timeout: %s must >0 minutes", timeout);

I think it shouldn't be 0, but I need to double-check.

…orkflow_timeout

Zzih96 · 2026-01-09T09:52:39Z

@ruanwenjun I've already modified it as requested. Could you please check it again?

ruanwenjun

Please removed the UT, this kind of ut help little, we should add IT case.

...er/server/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandler.java

ruanwenjun · 2026-01-19T11:31:18Z

...erver/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandlerTest.java

+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class WorkflowTimeoutLifecycleEventHandlerTest {


Remove this UT.

ruanwenjun · 2026-01-19T11:31:29Z

.../server/master/engine/workflow/lifecycle/handler/WorkflowStartLifecycleEventHandlerTest.java

+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class WorkflowStartLifecycleEventHandlerTest {


Remove this UT.

ruanwenjun · 2026-01-19T11:32:02Z

...heduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEventTest.java

+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class WorkflowTimeoutLifecycleEventTest {


Remove this UT.

sonarqubecloud · 2026-01-19T12:17:51Z

Please retry analysis of this Pull-Request directly on SonarQube Cloud

…ler/server/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandler.java Co-authored-by: Wenjun Ruan <wenjun@apache.org>

honkerjha · 2026-03-12T09:07:00Z

In DolphinScheduler 3.4.1, when both "Timeout Alert" and "Timeout Failure" are selected in the timeout policy settings, only the alert is sent, but the task does not fail as expected. Will this be fixed later?

njnu-seafish · 2026-03-12T12:32:34Z

In DolphinScheduler 3.4.1, when both "Timeout Alert" and "Timeout Failure" are selected in the timeout policy settings, only the alert is sent, but the task does not fail as expected. Will this be fixed later?

Task timeout fail bug is fixed by this PR: #17818

I test it as follow:

2026-03-12 20:24:10.819 INFO - Final Script Content:
====================
#!/bin/bash
BASEDIR=$(cd dirname $0; pwd)
cd $BASEDIR
source /usr/local/apache-dolphinscheduler-dev-SNAPSHOT-bin/bin/env/dolphinscheduler_env.sh
sleep 2m;
====================
2026-03-12 20:24:10.820 INFO - Executing shell command : sudo -u root -i /tmp/dolphinscheduler/exec/process/42/42.sh
2026-03-12 20:24:10.830 INFO - process start, process id is: 59604
2026-03-12 20:25:10.866 INFO - Begin killing task instance, processId: 59604
2026-03-12 20:25:12.268 INFO - prepare to parse pid, raw pid string: sudo(59604)---42.sh(59609)---sleep(59802)

2026-03-12 20:25:15.785 INFO - Sending 2 to process group: 59604 59609 59802, command: sudo -u root -i kill -2 59604 59609 59802
2026-03-12 20:25:30.036 INFO - Kill command: sudo -u root -i kill -2 59604 59609 59802, timed out, still running PIDs: 59604 59609 59802
2026-03-12 20:25:33.314 INFO - Sending 15 to process group: 59604 59609 59802, command: sudo -u root -i kill -15 59604 59609 59802
2026-03-12 20:25:34.401 ERROR - process has failure due to timeout kill, timeout value is:60, timeoutStrategy is:WARNFAILED
2026-03-12 20:25:34.404 INFO - process has killed. execute path:/tmp/dolphinscheduler/exec/process/42, processId:59604 ,exitStatusCode:-1 ,processWaitForStatus:false ,processExitValue:143
2026-03-12 20:25:37.716 INFO - Successfully killed process tree by SIGTERM, processId: 59604
2026-03-12 20:25:37.717 INFO - Process tree for task: 42 is killed or already finished, pid: 59604

honkerjha · 2026-03-13T02:13:53Z

In DolphinScheduler 3.4.1, when both "Timeout Alert" and "Timeout Failure" are selected in the timeout policy settings, only the alert is sent, but the task does not fail as expected. Will this be fixed later?

Task timeout fail bug is fixed by this PR: #17818

Has this issue been fixed in 3.4.1? I set up timeout failure for stored procedure tasks in DolphinScheduler 3.4.1, but it's not working as expected. @njnu-seafish

Looking at the execution logs of the stored procedure, the following content is present:

2026-03-13 10:00:13.781 INFO  - setSqlParamsMap: Property with paramName: value1 put in sqlParamsMap of content call aSP_test20260204(null,null,${value1}); successfully.
2026-03-13 10:01:13.783 INFO  - Try to cancel this procedure task
2026-03-13 10:01:13.791 INFO  - This procedure task was canceled
2026-03-13 10:02:44.029 INFO  - out parameter varchar key : value1 , value : 0

So does the timeout failure strategy only cancel the execution of the stored procedure, rather than changing the status of this node to "failed"? I am wondering whether this is a bug or a software feature.

njnu-seafish · 2026-03-13T02:42:37Z

In DolphinScheduler 3.4.1, when both "Timeout Alert" and "Timeout Failure" are selected in the timeout policy settings, only the alert is sent, but the task does not fail as expected. Will this be fixed later?

Task timeout fail bug is fixed by this PR: #17818

Has this issue been fixed in 3.4.1? I set up timeout failure for stored procedure tasks in DolphinScheduler 3.4.1, but it's not working as expected. @njnu-seafish

Looking at the execution logs of the stored procedure, the following content is present:
2026-03-13 10:00:13.781 INFO  - setSqlParamsMap: Property with paramName: value1 put in sqlParamsMap of content call aSP_test20260204(null,null,${value1}); successfully.
2026-03-13 10:01:13.783 INFO  - Try to cancel this procedure task
2026-03-13 10:01:13.791 INFO  - This procedure task was canceled
2026-03-13 10:02:44.029 INFO  - out parameter varchar key : value1 , value : 0
So does the timeout failure strategy only cancel the execution of the stored procedure, rather than changing the status of this node to "failed"? I am wondering whether this is a bug or a software feature.

Other Task Type timeout is ok? The logs indicate that the timeout cancellation logic was triggered; however, it appears the stored procedure failed to cancel successfully.

njnu-seafish · 2026-03-13T03:00:22Z

@Zzih96 @ruanwenjun 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.

honkerjha · 2026-03-13T05:15:05Z

In DolphinScheduler 3.4.1, when both "Timeout Alert" and "Timeout Failure" are selected in the timeout policy settings, only the alert is sent, but the task does not fail as expected. Will this be fixed later?

Task timeout fail bug is fixed by this PR: #17818

Has this issue been fixed in 3.4.1? I set up timeout failure for stored procedure tasks in DolphinScheduler 3.4.1, but it's not working as expected. @njnu-seafish

Looking at the execution logs of the stored procedure, the following content is present:
2026-03-13 10:00:13.781 INFO  - setSqlParamsMap: Property with paramName: value1 put in sqlParamsMap of content call aSP_test20260204(null,null,${value1}); successfully.
2026-03-13 10:01:13.783 INFO  - Try to cancel this procedure task
2026-03-13 10:01:13.791 INFO  - This procedure task was canceled
2026-03-13 10:02:44.029 INFO  - out parameter varchar key : value1 , value : 0
So does the timeout failure strategy only cancel the execution of the stored procedure, rather than changing the status of this node to "failed"? I am wondering whether this is a bug or a software feature.
Other Task Type timeout is ok? The logs indicate that the timeout cancellation logic was triggered; however, it appears the stored procedure failed to cancel successfully.

Other tasks work fine, but stored procedures with SLEEP cannot be stopped at all—this is likely a database-specific behavior.

njnu-seafish · 2026-03-13T06:04:19Z

In DolphinScheduler 3.4.1, when both "Timeout Alert" and "Timeout Failure" are selected in the timeout policy settings, only the alert is sent, but the task does not fail as expected. Will this be fixed later?

Task timeout fail bug is fixed by this PR: #17818

Has this issue been fixed in 3.4.1? I set up timeout failure for stored procedure tasks in DolphinScheduler 3.4.1, but it's not working as expected. @njnu-seafish

Looking at the execution logs of the stored procedure, the following content is present:
2026-03-13 10:00:13.781 INFO  - setSqlParamsMap: Property with paramName: value1 put in sqlParamsMap of content call aSP_test20260204(null,null,${value1}); successfully.
2026-03-13 10:01:13.783 INFO  - Try to cancel this procedure task
2026-03-13 10:01:13.791 INFO  - This procedure task was canceled
2026-03-13 10:02:44.029 INFO  - out parameter varchar key : value1 , value : 0
So does the timeout failure strategy only cancel the execution of the stored procedure, rather than changing the status of this node to "failed"? I am wondering whether this is a bug or a software feature.
Other Task Type timeout is ok? The logs indicate that the timeout cancellation logic was triggered; however, it appears the stored procedure failed to cancel successfully.
Other tasks work fine, but stored procedures with SLEEP cannot be stopped at all—this is likely a database-specific behavior.

Both SqlTask and ProcedureTask may fail to respond to interruption signals under certain circumstances. Specifically, the database can enter an 'uninterruptible' state where even simple SQL statements become atomic operations during specific execution phases, preventing them from responding to cancellation requests immediately.

Zzih96 requested review from SbloodyS, caishunfeng and ruanwenjun as code owners December 24, 2025 02:00

github-actions bot added backend test labels Dec 24, 2025

github-actions bot assigned Zzih96 Dec 24, 2025

SbloodyS changed the title ~~[Fix-17817] [Master]add workflow timeout~~ [Fix-17817] [Master] Fix workflow timeout alerts failed Dec 24, 2025

SbloodyS added the bug Something isn't working label Dec 24, 2025

SbloodyS added this to the 3.4.0 milestone Dec 24, 2025

github-advanced-security bot found potential problems Dec 24, 2025

View reviewed changes

...inscheduler/server/master/engine/workflow/lifecycle/event/WorkflowTimeoutLifecycleEvent.java Fixed Show fixed Hide fixed

ruanwenjun reviewed Dec 29, 2025

View reviewed changes

SbloodyS modified the milestones: 3.4.0, 3.4.1 Jan 5, 2026

Zzih96 added 2 commits January 8, 2026 21:00

add workflow timeout

0b705ad

add getWorkflowExecutionRunnable Override

3de2674

ruanwenjun force-pushed the add_workflow_timeout branch from 8e10927 to 3de2674 Compare January 8, 2026 13:00

njnu-seafish reviewed Jan 9, 2026

View reviewed changes

Zzih96 and others added 6 commits January 9, 2026 17:06

add workflow timeout

876e3bd

add getWorkflowExecutionRunnable Override

dd87e65

Merge remote-tracking branch 'origin/add_workflow_timeout' into add_w…

e460464

…orkflow_timeout

Merge branch 'dev' into add_workflow_timeout

58dd475

Merge remote-tracking branch 'origin/add_workflow_timeout' into add_w…

65e7d02

…orkflow_timeout

update

b123cb9

Zzih96 and others added 2 commits January 9, 2026 17:59

update

3057bf1

Merge branch 'dev' into add_workflow_timeout

ee1a419

ruanwenjun reviewed Jan 19, 2026

View reviewed changes

Update dolphinscheduler-master/src/main/java/org/apache/dolphinschedu…

e666889

…ler/server/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandler.java Co-authored-by: Wenjun Ruan <wenjun@apache.org>

Zzih96 and others added 2 commits January 26, 2026 16:33

Update dolphinscheduler-master/src/main/java/org/apache/dolphinschedu…

500c718

…ler/server/master/engine/workflow/lifecycle/handler/WorkflowTimeoutLifecycleEventHandler.java Co-authored-by: Wenjun Ruan <wenjun@apache.org>

remove UT

03bff5f

SbloodyS modified the milestones: 3.4.1, 3.4.2 Feb 28, 2026

Conversation

Zzih96 commented Dec 24, 2025

Purpose of the pull request

Brief change log

Verify this pull request

Pull Request Notice

Uh oh!

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zzih96 commented Jan 9, 2026

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 19, 2026

Uh oh!

honkerjha commented Mar 12, 2026

Uh oh!

njnu-seafish commented Mar 12, 2026

Uh oh!

honkerjha commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

njnu-seafish commented Mar 13, 2026

Uh oh!

njnu-seafish commented Mar 13, 2026

Uh oh!

honkerjha commented Mar 13, 2026

Uh oh!

njnu-seafish commented Mar 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

honkerjha commented Mar 13, 2026 •

edited

Loading