[Improvement-16979] Unify PriorityDelayQueue with AbstractDelayEventBus #17155

reele · 2025-04-30T02:55:49Z

Purpose of the pull request

implement #16979

Brief change log

Refactored DelayEntry.java,PriorityDelayQueue,PriorityAndDelayBasedTaskEntry,TimeBasedTaskExecutionRunnableComparableEntry with TaskDispatchEntryEvent,TaskDispatchEntryEventBus which inherited from AbstractDelayEventBus,AbstractDelayEvent
Fix PriorityAndDelayBasedTaskEntry's compare issue, old object compare task priority after compare time in millisecond, cause the task priority only affect in same millisecond, now it will compare task priority first, then the create time.

Verify this pull request

the test codes were refactored.

Pull Request Notice

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

.../org/apache/dolphinscheduler/server/master/runner/events/AbstractTaskDispatchEntryEvent.java

...ava/org/apache/dolphinscheduler/server/master/runner/events/TaskDispatchDelayEntryEvent.java

.../org/apache/dolphinscheduler/server/master/runner/events/TaskDispatchPriorityEntryEvent.java

ruanwenjun

Can we remove GlobalTaskDispatchWaitingQueue and GlobalTaskDispatchWaitingQueueLooper, this queue is used to handle the delay time, but bring some extra problems, e.g. there are existing many thread to deal with the task lifecycle in master, we need to do extra concurrency control, e.g. one task is in dispatching, and then receive a kill event.

Since TaskDispatchLifecycleEvent is already an DelayEvent so we can directly set the delay time to it, then we can removed TimeBasedTaskExecutionRunnableComparableEntry and WorkerGroupTaskDispatcher. We directly use ITaskExecutorClient to dispatch task in dispatchEventAction, if dispatch failed then regenerate a TaskDispatchLifecycleEvent with retry delay time.

ruanwenjun · 2025-04-30T06:11:40Z

.../org/apache/dolphinscheduler/server/master/runner/events/AbstractTaskDispatchEntryEvent.java

+    public int compareTo(@NotNull AbstractTaskDispatchEntryEvent<V> other) {
+        return super.compareTo(other);
+    }


We may need to compare the data if super.compareTo(other) == 0.

ok, i'll handle it

ruanwenjun · 2025-04-30T06:28:49Z

Good job, after this PR merged, the master code will be clearer.
Base on this, we can provide more EventBus to expose the workflow instance/task instance lifecycle event to exposed to outside.

reele · 2025-04-30T06:43:13Z

Can we remove GlobalTaskDispatchWaitingQueue and GlobalTaskDispatchWaitingQueueLooper, this queue is used to handle the delay time, but bring some extra problems, e.g. there are existing many thread to deal with the task lifecycle in master, we need to do extra concurrency control, e.g. one task is in dispatching, and then receive a kill event.

Since TaskDispatchLifecycleEvent is already an DelayEvent so we can directly set the delay time to it, then we can removed TimeBasedTaskExecutionRunnableComparableEntry and WorkerGroupTaskDispatcher. We directly use ITaskExecutorClient to dispatch task in dispatchEventAction, if dispatch failed then regenerate a TaskDispatchLifecycleEvent with retry delay time.

i'll have a try.

ruanwenjun · 2025-04-30T16:20:32Z

@reele Sorry, I miss something, we need to sort the tasks by priority under one worker group, so WorkerGroupTaskDispatcher cannot be removed, but GlobalTaskDispatchWaitingQueue and GlobalTaskDispatchWaitingQueueLooper can be removed.

reele · 2025-05-06T03:01:26Z

@reele Sorry, I miss something, we need to sort the tasks by priority under one worker group, so WorkerGroupTaskDispatcher cannot be removed, but GlobalTaskDispatchWaitingQueue and GlobalTaskDispatchWaitingQueueLooper can be removed.

OK.

ruanwenjun · 2025-05-06T07:03:10Z

@reele Sorry, I miss something, we need to sort the tasks by priority under one worker group, so WorkerGroupTaskDispatcher cannot be removed, but GlobalTaskDispatchWaitingQueue and GlobalTaskDispatchWaitingQueueLooper can be removed.

OK.

I'll figure out how to fix this prioritization problem, which is in fact very inaccurate, and it's not sorting globally.

ruanwenjun · 2025-05-13T15:55:36Z

I have removed GlobalTaskDispatchWaitingQueue at #17180

reele · 2025-05-17T08:42:28Z

I have removed GlobalTaskDispatchWaitingQueue at #17180

Oh! sorry for not solving this pr in time......

ruanwenjun · 2025-05-23T01:52:04Z

I have removed GlobalTaskDispatchWaitingQueue at #17180

Oh! sorry for not solving this pr in time......

Hi @reele, I reopen this PR, this PR still needed.

…jects' into improvement-16979-unify-queue-objects # Conflicts: # dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/task/dispatcher/WorkerGroupDispatcher.java # dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/GlobalTaskDispatchWaitingQueue.java

reele · 2025-06-04T09:12:15Z

@ruanwenjun recommitted.

...ache/dolphinscheduler/server/master/engine/task/dispatcher/event/TaskDispatchEntryEvent.java

.../apache/dolphinscheduler/server/master/engine/task/dispatcher/TaskDispatchEntryEventBus.java

…ler/server/master/engine/task/dispatcher/event/TaskDispatchEntryEvent.java Co-authored-by: Wenjun Ruan <[email protected]>

…ler/server/master/engine/task/dispatcher/TaskDispatchEntryEventBus.java Co-authored-by: Wenjun Ruan <[email protected]>

ruanwenjun

LGTM. Some class names could be improved.

ruanwenjun · 2025-08-06T10:04:12Z

...e/dolphinscheduler/server/master/engine/task/dispatcher/event/TaskReadyForDispatchEvent.java

+import static com.google.common.base.Preconditions.checkNotNull;
+
+@Getter
+public class TaskReadyForDispatchEvent<V extends Comparable<V>> extends AbstractDelayEvent {


Suggested change

public class TaskReadyForDispatchEvent<V extends Comparable<V>> extends AbstractDelayEvent {

public class TaskDispatchableEvent<ITaskExecutionRunnable> extends AbstractDelayEvent {

ruanwenjun · 2025-08-06T10:05:31Z

...va/org/apache/dolphinscheduler/server/master/engine/task/dispatcher/WorkerGroupEventBus.java

+public class WorkerGroupEventBus<V extends TaskReadyForDispatchEvent<T>, T extends Comparable<T>>
+        extends
+            AbstractDelayEventBus<V> {


Suggested change

public class WorkerGroupEventBus<V extends TaskReadyForDispatchEvent<T>, T extends Comparable<T>>

extends

AbstractDelayEventBus<V> {

public class TaskDispatchableEventBus

extends

AbstractDelayEventBus< TaskDispatchableEvent > {

...e/dolphinscheduler/server/master/engine/task/dispatcher/event/TaskReadyForDispatchEvent.java

ruanwenjun

LGTM

...pache/dolphinscheduler/server/master/engine/task/dispatcher/event/TaskDispatchableEvent.java

+import lombok.Getter;
+
+@Getter
+public class TaskDispatchableEvent<V extends Comparable<V>> extends AbstractDelayEvent {


SbloodyS

+1

sonarqubecloud · 2025-08-07T03:15:04Z

Quality Gate passed

Issues
9 New issues
0 Accepted issues

Measures
0 Security Hotspots
78.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

…us (apache#17155)

lile added 3 commits April 29, 2025 18:16

Improvement: unify queue objects

8376630

Improvement: unify queue objects

cda00ae

[Improvement-16979] Unify PriorityDelayQueue with AbstractDelayEventBus

66d005d

reele requested review from SbloodyS, caishunfeng and ruanwenjun as code owners April 30, 2025 02:55

github-actions bot added backend test labels Apr 30, 2025

github-actions bot assigned reele Apr 30, 2025

github-advanced-security bot found potential problems Apr 30, 2025

View reviewed changes

ruanwenjun reviewed Apr 30, 2025

View reviewed changes

ruanwenjun added this to the 3.3.1 milestone Apr 30, 2025

ruanwenjun added the improvement make more easy to user or prompt friendly label Apr 30, 2025

reele closed this May 17, 2025

ruanwenjun reopened this May 23, 2025

reele closed this Jun 2, 2025

reele force-pushed the improvement-16979-unify-queue-objects branch from 66d005d to 22ceff8 Compare June 2, 2025 08:56

lile added 2 commits June 3, 2025 10:05

[Improvement-16979] Unify PriorityDelayQueue with AbstractDelayEventBus

d640551

reele reopened this Jun 3, 2025

[Improvement-16979] Unify PriorityDelayQueue with AbstractDelayEventBus

007e0c7

Merge branch 'dev' into improvement-16979-unify-queue-objects

a373365

github-advanced-security bot found potential problems Jun 5, 2025

View reviewed changes

...ache/dolphinscheduler/server/master/engine/task/dispatcher/event/TaskDispatchEntryEvent.java Fixed Show fixed Hide fixed

reele and others added 3 commits June 5, 2025 15:36

Merge branch 'dev' into improvement-16979-unify-queue-objects

5f42817

Merge branch 'dev' into improvement-16979-unify-queue-objects

51c57f0

Merge branch 'dev' into improvement-16979-unify-queue-objects

1e75abb

ruanwenjun reviewed Jun 9, 2025

View reviewed changes

...ache/dolphinscheduler/server/master/engine/task/dispatcher/event/TaskDispatchEntryEvent.java Outdated Show resolved Hide resolved

.../apache/dolphinscheduler/server/master/engine/task/dispatcher/TaskDispatchEntryEventBus.java Outdated Show resolved Hide resolved

reele and others added 10 commits June 9, 2025 23:10

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

8aaa5c2

…ler/server/master/engine/task/dispatcher/event/TaskDispatchEntryEvent.java Co-authored-by: Wenjun Ruan <[email protected]>

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

bf6fcf4

…ler/server/master/engine/task/dispatcher/TaskDispatchEntryEventBus.java Co-authored-by: Wenjun Ruan <[email protected]>

Merge branch 'dev' into improvement-16979-unify-queue-objects

6bf31e7

Merge branch 'dev' into improvement-16979-unify-queue-objects

cdf7d23

Update TaskDispatchEntryEvent.java

a55d61a

Merge branch 'dev' into improvement-16979-unify-queue-objects

6777e1c

Merge branch 'dev' into improvement-16979-unify-queue-objects

af7eef6

fix code error.

85a9e7c

fix code error.

6f51fac

Merge branch 'dev' into improvement-16979-unify-queue-objects

fdea15e

ruanwenjun reviewed Aug 6, 2025

View reviewed changes

ruanwenjun added the priority:low label Aug 6, 2025

github-advanced-security bot found potential problems Aug 6, 2025

View reviewed changes

...e/dolphinscheduler/server/master/engine/task/dispatcher/event/TaskReadyForDispatchEvent.java Fixed Show fixed Hide fixed

lile added 2 commits August 6, 2025 18:47

apply spotless

d6d12f5

rename object

b98b57f

ruanwenjun approved these changes Aug 7, 2025

View reviewed changes

github-advanced-security bot found potential problems Aug 7, 2025

View reviewed changes

SbloodyS approved these changes Aug 7, 2025

View reviewed changes

SbloodyS added the ready-to-merge label Aug 7, 2025

SbloodyS merged commit 2f57e05 into apache:dev Aug 7, 2025
71 checks passed

eco8848 pushed a commit to eco8848/dolphinscheduler that referenced this pull request Aug 8, 2025

[Improvement-16979] Unify PriorityDelayQueue with AbstractDelayEventB…

b54ec5e

…us (apache#17155)

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

[Improvement-16979] Unify PriorityDelayQueue with AbstractDelayEventB…

c19a554

…us (apache#17155)

	public class TaskReadyForDispatchEvent<V extends Comparable<V>> extends AbstractDelayEvent {
	public class TaskDispatchableEvent<ITaskExecutionRunnable> extends AbstractDelayEvent {

[Improvement-16979] Unify PriorityDelayQueue with AbstractDelayEventBus #17155

[Improvement-16979] Unify PriorityDelayQueue with AbstractDelayEventBus #17155

Uh oh!

Conversation

reele commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of the pull request

Brief change log

Verify this pull request

Pull Request Notice

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ruanwenjun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruanwenjun Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

reele Apr 30, 2025

Choose a reason for hiding this comment

Uh oh!

ruanwenjun commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reele commented Apr 30, 2025

Uh oh!

ruanwenjun commented Apr 30, 2025

Uh oh!

reele commented May 6, 2025

Uh oh!

ruanwenjun commented May 6, 2025

Uh oh!

ruanwenjun commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reele commented May 17, 2025

Uh oh!

ruanwenjun commented May 23, 2025

Uh oh!

reele commented Jun 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

ruanwenjun Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

reele Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

ruanwenjun Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

reele Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ruanwenjun left a comment

Choose a reason for hiding this comment

Uh oh!

Check warning

Uh oh!

SbloodyS left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Aug 7, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

reele commented Apr 30, 2025 •

edited

Loading

ruanwenjun left a comment •

edited

Loading

ruanwenjun commented Apr 30, 2025 •

edited

Loading

ruanwenjun commented May 13, 2025 •

edited

Loading