[Improvement-16979] Unify PriorityDelayQueue with AbstractDelayEventBus #17155
Conversation
reele
requested review from
caishunfeng,
SbloodyS and
ruanwenjun
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| this.data = checkNotNull(data, "data is null"); | ||
| } | ||
|
|
||
| public int compareTo(@NotNull AbstractTaskDispatchEntryEvent<V> other) { |
Check failure
Code scanning / CodeQL
Overloaded compareTo Error
| @Override | ||
| public int hashCode() { | ||
| return super.hashCode(); | ||
| public int compareTo(@NotNull AbstractTaskDispatchEntryEvent<V> other) { |
Check failure
Code scanning / CodeQL
Overloaded compareTo Error
| } | ||
|
|
||
| @Override | ||
| public int compareTo(@NotNull AbstractTaskDispatchEntryEvent<V> other) { |
Check failure
Code scanning / CodeQL
Overloaded compareTo Error
There was a problem hiding this comment.
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.
| public int compareTo(@NotNull AbstractTaskDispatchEntryEvent<V> other) { | ||
| return super.compareTo(other); | ||
| } |
There was a problem hiding this comment.
We may need to compare the data if super.compareTo(other) == 0.
|
Good job, after this PR merged, the master code will be clearer. |
i'll have a try. |
|
@reele Sorry, I miss something, we need to sort the tasks by priority under one worker group, so |
Purpose of the pull request
implement #16979
Brief change log
Refactored
DelayEntry.java,PriorityDelayQueue,PriorityAndDelayBasedTaskEntry,TimeBasedTaskExecutionRunnableComparableEntrywithTaskDispatchDelayEntryEvent,TaskDispatchPriorityEntryEvent,AbstractTaskDispatchEntryEvent,TaskDispatchEntryEventBuswhich inherited fromAbstractDelayEventBus,AbstractDelayEventFix
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
Pull Request Notice
If your pull request contains incompatible change, you should also add it to
docs/docs/en/guide/upgrade/incompatible.md