Skip to content

Conversation

@reele
Copy link
Contributor

@reele reele commented Apr 29, 2025

Purpose of the pull request

close #16767

Brief change log

  1. add try takeover sub-workflow in SubWorkflowTask.start() when failover, to avoid unhandled sub-workflow running in background.
  2. move sub-workflow's failover into SubWorkflowTask to make sure sub-workflow's failover under control by task.

Verify this pull request

This change added tests and can be verified as follows:

Added take-over sub-workflow in WorkflowInstanceFailoverTestCase.

@ruanwenjun

@ruanwenjun ruanwenjun added this to the 3.3.1 milestone Apr 29, 2025
@ruanwenjun ruanwenjun added the bug Something isn't working label Apr 29, 2025
@ruanwenjun ruanwenjun changed the title Improvement: Takeover sub-workflow in sub-workflow-task [Improvement-16767] Takeover sub-workflow in sub-workflow-task Apr 29, 2025
@ruanwenjun ruanwenjun added improvement make more easy to user or prompt friendly priority:high and removed bug Something isn't working labels Apr 29, 2025
ruanwenjun
ruanwenjun previously approved these changes Apr 29, 2025
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I will improve the BaseDao to reduce the method number.

Comment on lines 57 to 58
private List<WorkflowInstanceRelation> workflowInstanceRelations;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used? please initialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed.

Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, wait the CI pass.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
57.1% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@SbloodyS SbloodyS merged commit c236066 into apache:dev May 7, 2025
69 of 70 checks passed
davidzollo pushed a commit to davidzollo/dolphinscheduler that referenced this pull request Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend improvement make more easy to user or prompt friendly priority:high test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [TaskExecutionRunnable] Sub workflow task always repeat run in master-server failover

3 participants