sql-wj opened a new issue, #18698: URL: https://github.com/apache/dolphinscheduler/issues/18698
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/dolphinscheduler/issues?q=is%3Aissue) and found no similar issues. ### What happened Describe the bug After #18406 (fixed by #18415 in 3.4.3), an architectural gap remains: the "persist task failure to DB" step and the "advance the workflow" step are two non-atomic operations. If the master crashes / restarts / fails over between them, the DB is left with `task = FAILURE` while `workflow = RUNNING`. The serial lock (`t_ds_serial_command`) is not released, so subsequent serial schedules of the same workflow can never trigger. #18406 only fixes one specific DAG shape (Continue failure strategy + a blocked intermediate predecessor). It does not provide any independent reconciliation for the serial lock. ## Which version of DolphinScheduler 3.4.2 (observed); 3.4.3 partially fixed (#18406/#18415). The non-atomicity and the missing serial-lock reconciliation are NOT fixed in 3.4.3. ## To Reproduce 1. Use a workflow whose task can fail. 2. Make the master crash/restart/failover in the window between the task-failure write to DB and the workflow-advance steps (or otherwise get the workflow stuck while a task is FAILURE). 3. Observe: `t_ds_task_instance.state = FAILURE`, `t_ds_workflow_instance.state = RUNNING_EXECUTION`, and a stale row remains in `t_ds_serial_command`. 4. Subsequent serial schedules of the same workflow never trigger. ## Root cause In `AbstractTaskStateAction.onFailedEvent()`: ```java releaseTaskInstanceResourcesIfNeeded(taskExecution); persistentTaskInstanceFailedEventToDB(taskExecution, ...); // (1) persisted to DB ... graph.markTaskExecutionChainFailure(taskExecution); // (2) in-memory only publishWorkflowInstanceTopologyLogicalTransitionEvent(...); // (3) in-memory only ``` `persistentTaskInstanceFailedEventToDB` is a bare `taskInstanceDao.updateById(...)` with no `@Transactional`. Steps (2) and (3) live only in master memory. If the master dies after (1) but before (2)/(3) complete, steps (2)/(3) vanish with the memory, leaving `task = FAILURE, workflow = RUNNING` in the DB. On failover, the graph is rebuilt from the DB, but the failed task is not reliably re-marked into a failure chain and the workflow terminal event is not triggered — `emitWorkflowFinishedEventIfApplicable` returns early when `!isWorkflowFinishable`. The workflow stays RUNNING. Additionally, `WorkflowSerialCoordinator.doStart()` (lines 93–109) only handles `SERIAL_WAIT` / `DISCARD` / `PRIORITY`. There is no logic to scan terminal/zombie workflows and recycle their stale `t_ds_serial_command` rows. Serial-lock cleanup depends 100% on `workflowFinish` being reached. ## Proposed improvement Add a reconciliation task to `WorkflowSerialCoordinator` that scans already-terminal / zombie workflows and recycles their residual `t_ds_serial_command` rows. This is complementary to the #18406 fix (which only covers the Continue + blocked-predecessor shape). ## Additional context - #18406 / #18415 fixed the Continue + blocked-predecessor case only. - The 3.4.3 fix is verifiable in `AbstractWorkflowStateAction.tryToTriggerSuccessorsAfterTaskFinish` (adds an `isAllTaskExecutionChainFinish` OR-condition), but it does not address the non-atomic write or the serial-lock reconciliation. ### What you expected to happen Once a task is persisted as FAILURE, the workflow should reliably advance to its terminal state (FAILURE), and its serial lock (t_ds_serial_command) should be released. Even if the master crashes or fails over in between, the failover rebuild should recognize the already-failed task, drive the workflow to a terminal state, and clean up the stale serial command. Persisting the task failure and advancing the workflow are two separate, non-atomic steps in AbstractTaskStateAction.onFailedEvent(): 1. persistentTaskInstanceFailedEventToDB() -> bare updateById, no @Transactional, written to DB immediately 2. markTaskExecutionChainFailure() -> in-memory only 3. publishWorkflowInstanceTopologyLogicalTransitionEvent() -> in-memory only Steps 2 and 3 live only in master memory. If the master crashes/restarts/ fails over after step 1 but before 2-3 finish, steps 2-3 are lost with the memory, leaving task = FAILURE but workflow = RUNNING in the DB. Failover rebuilds the graph from the DB but does not reliably re-mark the failed task into a failure chain or trigger the workflow terminal event (emitWorkflowFinishedEventIfApplicable returns early when the workflow is not "finishable"), so the workflow stays RUNNING. Separately, the serial lock has no independent reconciliation: WorkflowSerialCoordinator.doStart() only handles SERIAL_WAIT / DISCARD / PRIORITY and never scans terminal/zombie workflows to recycle their stale t_ds_serial_command rows. So serial-lock cleanup depends entirely on workflowFinish being reached, which never happens here. This is the gap NOT covered by #18406 / #18415 (which only fixed the Continue + blocked-predecessor DAG shape). ### How to reproduce This is a crash-window / failover defect, so it is best shown by observing the inconsistent state directly, plus an optional fault-injection trigger. ### Path 1 — observe the inconsistent state directly (reliable) 1. Query for workflows whose task has failed but whose workflow instance is still RUNNING. In DolphinScheduler 3.4.x: -- task_state / wf_state enum values: verify against TaskExecutionStatus -- and WorkflowExecutionStatus; the query below filters on -- task = FAILURE and workflow = RUNNING_EXECUTION SELECT w.id, w.name, w.state AS wf_state, t.state AS task_state FROM t_ds_workflow_instance w JOIN t_ds_task_instance t ON t.workflow_instance_id = w.id WHERE w.state = /* RUNNING_EXECUTION */ AND t.state = /* FAILURE */; 2. For those workflow ids, a stale serial command remains: SELECT * FROM t_ds_serial_command WHERE workflow_instance_id IN (<ids from step 1>); 3. Trigger the same workflow again with serial scheduling: the next run never starts because the stale serial lock is never released. ### Path 2 — fault injection (best effort, window is hard to hit) 1. Deploy 3.4.2 and configure a workflow with a task that fails, with serial scheduling enabled. 2. Immediately after the task failure is written to the DB but before the workflow advances, force-kill the master (or trigger a failover). The window between "persist task failure" and "advance workflow" is small, so this may take several attempts. 3. Restart the master / let failover rebuild the graph from the DB. 4. Observe: t_ds_task_instance.state = FAILURE, t_ds_workflow_instance.state = RUNNING_EXECUTION, and the t_ds_serial_command row is never cleaned up. ### Anything else _No response_ ### Version 3.4.2 ### Are you willing to submit PR? - [ ] Yes I am willing to submit a PR! ### Code of Conduct - [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct) -- 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]
