[Improvement] Remove taskQueue and looper in worker #15292
Conversation
ruanwenjun
requested review from
caishunfeng,
SbloodyS and
zhuangchong
as code owners
ruanwenjun
force-pushed
the
dev_wenjun_renameMasterLogicTaskExecuteRunnable
branch
3 times, most recently
from
b43261d
to
c783836
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #15292 +/- ##
============================================
+ Coverage 37.80% 38.10% +0.29%
- Complexity 4681 4698 +17
============================================
Files 1304 1301 -3
Lines 44937 44800 -137
Branches 4812 4800 -12
============================================
+ Hits 16990 17071 +81
+ Misses 26098 25877 -221
- Partials 1849 1852 +3 ☔ View full report in Codecov by Sentry. |
ruanwenjun
force-pushed
the
dev_wenjun_renameMasterLogicTaskExecuteRunnable
branch
from
c783836
to
7f77e6c
Compare
ruanwenjun
force-pushed
the
dev_wenjun_renameMasterLogicTaskExecuteRunnable
branch
3 times, most recently
from
1782419
to
79d2e34
Compare
| // todo: remove this method | ||
| ThreadPoolExecutor getThreadPool() { | ||
| return threadPoolExecutor; | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you add more comments for it? You added this method but the todo is the remove method, which seems a bit strange.
| @@ -128,26 +107,4 @@ public void stop(String cause) { | |||
| close(cause); | |||
| } | |||
|
|
|||
| public void killAllRunningTasks() { | |||
There was a problem hiding this comment.
I think we keep this method to make sure all task are canceled when worker down, WDYT?
There was a problem hiding this comment.
Ok, I keep this method, and add todo.
caishunfeng
changed the title
|
Please add some pr description. |
ruanwenjun
force-pushed
the
dev_wenjun_renameMasterLogicTaskExecuteRunnable
branch
from
79d2e34
to
30a5d41
Compare
Done |
ruanwenjun
force-pushed
the
dev_wenjun_renameMasterLogicTaskExecuteRunnable
branch
from
30a5d41
to
7dc5447
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (ProcessUtils.kill(taskRequest)) { | ||
| TaskExecutionContext taskExecutionContext = workerTaskExecutor.getTaskExecutionContext(); | ||
| LogUtils.setTaskInstanceIdMDC(taskExecutionContext.getTaskInstanceId()); | ||
| if (ProcessUtils.kill(taskExecutionContext)) { |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
ruanwenjun
force-pushed
the
dev_wenjun_renameMasterLogicTaskExecuteRunnable
branch
from
7dc5447
to
283d52d
Compare
|
ruanwenjun
deleted the
dev_wenjun_renameMasterLogicTaskExecuteRunnable
branch
Purpose of the pull request
Since we have put the
delaylogic from worker to master, there is no need to add task queue in worker, this will cause it's not convenient to judge the task size, and we need to use a thread to loop the task queue.This pr will remove the extra task queue in worker, directly use the thread pool's queue as task queue.
Brief change log
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
If your pull request contain incompatible change, you should also add it to
docs/docs/en/guide/upgrede/incompatible.md