[DSIP-55][Master] Separate the waiting dispatched task into different queue by worker group by det101 · Pull Request #17037 · apache/dolphinscheduler

det101 · 2025-03-03T11:39:41Z

Purpose of the pull request

Simplify the GlobalTaskDispatchWaitingQueue to only handle sorting based on time.
Introduce a new WorkerGroup queue that sorts tasks based on priority and time, with priority being higher than time. Additionally, it is responsible for retrying failed tasks with a delay.

close #16260

Brief change log

This change added tests and can be verified as follows:

Manually verified the change by testing locally.

Pull Request Notice

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

boring-cyborg · 2025-03-03T11:39:43Z

Thanks for opening this pull request! Please check out our contributing guidelines. (https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md)

...src/main/java/org/apache/dolphinscheduler/server/master/runner/queue/PriorityDelayEntry.java

...er-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/WorkflowEngine.java

...in/java/org/apache/dolphinscheduler/server/master/runner/GlobalTaskDispatchWaitingQueue.java

...a/org/apache/dolphinscheduler/server/master/runner/GlobalTaskDispatchWaitingQueueLooper.java

...r/src/main/java/org/apache/dolphinscheduler/server/master/runner/WorkerGroupQueueLooper.java

...-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/queue/DelayEntry.java

...src/main/java/org/apache/dolphinscheduler/server/master/runner/queue/PriorityDelayEntry.java

...rc/main/java/org/apache/dolphinscheduler/server/master/runner/queue/WorkerGroupQueueMap.java

dolphinscheduler-ui/src/router/modules/ui-setting.ts

...er/src/main/java/org/apache/dolphinscheduler/server/master/runner/queue/ComparableEntry.java

.../apache/dolphinscheduler/server/master/runner/WorkerGroupTaskDispatchWaitingQueueLooper.java

...in/java/org/apache/dolphinscheduler/server/master/runner/WorkerGroupTaskDispatchManager.java

...in/java/org/apache/dolphinscheduler/server/master/runner/GlobalTaskDispatchWaitingQueue.java

...er-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/DispatchWorker.java

...e/dolphinscheduler/server/master/cluster/ThreadCreatingAndDestroyingWorkerGroupListener.java

...r-master/src/main/java/org/apache/dolphinscheduler/server/master/cluster/ClusterManager.java

...in/java/org/apache/dolphinscheduler/server/master/runner/GlobalTaskDispatchWaitingQueue.java

...r-master/src/main/java/org/apache/dolphinscheduler/server/master/cluster/ClusterManager.java

...er-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/DispatchWorker.java

...r-master/src/main/java/org/apache/dolphinscheduler/server/master/cluster/ClusterManager.java

The logic for the non-existence of workergroup needs to be supplemented.

…ce it back into the queue and wait.

…ario is rare and the logic is simplified for now, it will be left as a TODO.

…when adding tasks.

SbloodyS

I agree with this implementation, I just have some questions. After reviewing the code, I found that this PR is to put the failed tasks in the delayed queue and wait for a certain period of time before resubmitting, instead of actually dividing them into multiple queues according to the worker group as stated in the title, is that right? @det101

sonarqubecloud · 2025-04-16T03:03:10Z

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

det101 · 2025-04-16T03:14:17Z

I agree with this implementation, I just have some questions. After reviewing the code, I found that this PR is to put the failed tasks in the delayed queue and wait for a certain period of time before resubmitting, instead of actually dividing them into multiple queues according to the worker group as stated in the title, is that right? @det101

If the global queue fails to be distributed to the workergoup queue, the queue will be delayed. If it has been distributed to the workergoup queue, the failure will only be placed in the workergoup's delay queue.

SbloodyS · 2025-04-16T03:18:34Z

If the global queue fails to be distributed to the workergoup queue, the queue will be delayed. If it has been distributed to the workergoup queue, the failure will only be placed in the workergoup's delay queue.

Okay.

SbloodyS

+1

… queue by worker group (apache#17037)

det101 requested review from SbloodyS, caishunfeng, ruanwenjun and songjianet as code owners March 3, 2025 11:39

github-actions bot added UI ui and front end related backend test labels Mar 3, 2025

github-actions bot assigned det101 Mar 3, 2025

github-advanced-security bot found potential problems Mar 3, 2025

View reviewed changes

...src/main/java/org/apache/dolphinscheduler/server/master/runner/queue/PriorityDelayEntry.java Fixed Show fixed Hide fixed

SbloodyS changed the title ~~[Feature-16260][master]add WorkerGroup queue~~ [DSIP-55][Master] Separate the waiting dispatched task into different queue by worker group Mar 4, 2025

SbloodyS added the DSIP label Mar 4, 2025

SbloodyS added this to the 3.3.0 milestone Mar 4, 2025

ruanwenjun requested changes Mar 5, 2025

View reviewed changes

ruanwenjun reviewed Mar 5, 2025

View reviewed changes

dolphinscheduler-ui/src/router/modules/ui-setting.ts Outdated Show resolved Hide resolved

det101 marked this pull request as draft March 7, 2025 04:04

ruanwenjun added the priority:high label Mar 7, 2025

ruanwenjun marked this pull request as ready for review March 7, 2025 06:55

ruanwenjun force-pushed the dev branch from 049742f to 259bd24 Compare March 7, 2025 06:55

det101 force-pushed the dev branch from 7547725 to 013112b Compare March 9, 2025 09:01

ruanwenjun requested changes Mar 10, 2025

View reviewed changes

det101 force-pushed the dev branch from 06f7bf8 to 838ed3a Compare March 10, 2025 12:04

ruanwenjun requested changes Mar 10, 2025

View reviewed changes

ruanwenjun reviewed Mar 10, 2025

View reviewed changes

...er-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/DispatchWorker.java Outdated Show resolved Hide resolved

det101 marked this pull request as draft March 11, 2025 01:43

det101 marked this pull request as ready for review March 11, 2025 03:51

det101 marked this pull request as draft March 11, 2025 03:57

det101 marked this pull request as ready for review March 11, 2025 07:13

ruanwenjun reviewed Mar 12, 2025

View reviewed changes

...r-master/src/main/java/org/apache/dolphinscheduler/server/master/cluster/ClusterManager.java Outdated Show resolved Hide resolved

github-actions bot removed the UI ui and front end related label Mar 12, 2025

luxl added 19 commits April 15, 2025 20:32

Remove unnecessary resource cleanup

bd6d9ee

less unit test time

63ffb7c

fix jdk8 unit test

fcdec92

optimized code

ee3fae8

fix sonar

1e86477

override equals

c751272

Optimize concurrency issues

85fa740

The logic for the non-existence of workergroup needs to be supplemented.

\cannot find the workergroup, task will kill.

af6be6a

fix unit test

ca78f4f

fix unit test

48ebb78

spotless

9340610

retry e2e

7c9f842

jdk 11 unit test

057cac9

Maintain the original logic: if the worker group cannot be found, pla…

18cf5c0

…ce it back into the queue and wait.

optimized code

d86b44a

Remove the close logic for WorkerGroupTaskDispatcher. Since this scen…

b4abd0b

…ario is rare and the logic is simplified for now, it will be left as a TODO.

fix unit test

0fb49ae

Change the logic to dynamically create the workerGroupTaskDispatcher …

f5f0bfe

…when adding tasks.

fix unit test

c2487a1

ruanwenjun force-pushed the dev branch from 7ac49df to c2487a1 Compare April 15, 2025 12:32

optimization unit test

a8b6650

det101 dismissed ruanwenjun’s stale review via a8b6650 April 16, 2025 02:23

SbloodyS reviewed Apr 16, 2025

View reviewed changes

SbloodyS approved these changes Apr 16, 2025

View reviewed changes

ruanwenjun approved these changes Apr 17, 2025

View reviewed changes

ruanwenjun merged commit f5469d7 into apache:dev Apr 17, 2025
70 checks passed

davidzollo pushed a commit to davidzollo/dolphinscheduler that referenced this pull request Oct 27, 2025

[DSIP-55][Master] Separate the waiting dispatched task into different…

86aee32

… queue by worker group (apache#17037)

Conversation

det101 commented Mar 3, 2025 • edited by SbloodyS Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of the pull request

Brief change log

Pull Request Notice

Uh oh!

boring-cyborg bot commented Mar 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SbloodyS left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Apr 16, 2025

Uh oh!

det101 commented Apr 16, 2025

Uh oh!

SbloodyS commented Apr 16, 2025

Uh oh!

SbloodyS left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

det101 commented Mar 3, 2025 •

edited by SbloodyS

Loading