You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/04/28 12:32:50 UTC
[GitHub] [incubator-doris] vagetablechicken opened a new issue #3416: Replace the thread pool in FragmentMgr
vagetablechicken opened a new issue #3416:
URL: https://github.com/apache/incubator-doris/issues/3416
FragmentMgr will create new pthreads if the fragment_map size is more than config::fragment_pool_thread_num.
https://github.com/apache/incubator-doris/blob/0430714ca9b0d910ba9b290276da05adfc752f8f/be/src/runtime/fragment_mgr.cpp#L461
If plan fragments are too much, be has no defence, it'll create too much threads. And the threads in thread pool may starve, because fragment map size is big but the fragments may be executed by pthread(not by the thread in threadpool).
### Solution
ref #2915, I think it's a better thread pool for fragment mgr. If func_num > max_thread_num+queue, it will abort the new plan fragment.
### Follow-up
After replaced the thread pool, we need to consider about the dtor of FragmentExecState's members.
https://github.com/apache/incubator-doris/blob/0430714ca9b0d910ba9b290276da05adfc752f8f/be/src/runtime/fragment_mgr.cpp#L433
If threadpool.submit() failed, PlanFragmentExecutor will be just prepared, and then closed with OK status.
https://github.com/apache/incubator-doris/blob/0430714ca9b0d910ba9b290276da05adfc752f8f/be/src/runtime/plan_fragment_executor.cpp#L548-L553
It may hide some bugs. Currently I found OlapTableSink can't close with OK status when it didn't do open()/send().
**Bugs may be hidden elsewhere, I will test it in our test & product envs. If it works fine, I'll submit a pull request.**
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken closed issue #3416: Replace the thread pool in FragmentMgr
Posted by GitBox <gi...@apache.org>.
vagetablechicken closed issue #3416:
URL: https://github.com/apache/incubator-doris/issues/3416
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken edited a comment on issue #3416: Replace the thread pool in FragmentMgr
Posted by GitBox <gi...@apache.org>.
vagetablechicken edited a comment on issue #3416:
URL: https://github.com/apache/incubator-doris/issues/3416#issuecomment-626102191
### Step 1
Current fragment_mgr use
https://github.com/apache/incubator-doris/blob/0430714ca9b0d910ba9b290276da05adfc752f8f/be/src/runtime/fragment_mgr.cpp#L461-L463
to avoid ThreadPool::offer() failed. If we replace it with kudu thread pool, we must handle this situation:
```exec_state created -> prepare -> submit to thread pool failed (means never execute)-> destory```
I'll force fragment_mgr to use thread pool ( not create pthread) to test it. No need to replace thread pool firstly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken closed issue #3416: Replace the thread pool in FragmentMgr
Posted by GitBox <gi...@apache.org>.
vagetablechicken closed issue #3416:
URL: https://github.com/apache/incubator-doris/issues/3416
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken commented on issue #3416: Replace the thread pool in FragmentMgr
Posted by GitBox <gi...@apache.org>.
vagetablechicken commented on issue #3416:
URL: https://github.com/apache/incubator-doris/issues/3416#issuecomment-626102191
### Step 1
One FragmentExecState obj called exec_state.
After exec_state inited and prepared, we try to put it in threadpool to execute.
If threadpool is full, it should be aborted. So it never execute.
exec_state's member _executor(PlanFragmentExecutor) is just prepared, skip open, and do close in `~PlanFragmentExecutor()`.
Thus the executor's member nodes( _plan-plan node, _sink-sink node) are prepared, then closed.
And the sink node close() will consider about the executor's status.
So the sink node close() has two situation:
1. close with ok status: means plan finished.
1. close with error status: means cancel, may send cancel rpc to other BEs.
We should add the third situation:
close with abort status: means just close, clean the prepared resource, don't need to cancel anything.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken closed issue #3416: Replace the thread pool in FragmentMgr
Posted by GitBox <gi...@apache.org>.
vagetablechicken closed issue #3416:
URL: https://github.com/apache/incubator-doris/issues/3416
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken edited a comment on issue #3416: Replace the thread pool in FragmentMgr
Posted by GitBox <gi...@apache.org>.
vagetablechicken edited a comment on issue #3416:
URL: https://github.com/apache/incubator-doris/issues/3416#issuecomment-626102191
### Step 1
Current fragment_mgr use
https://github.com/apache/incubator-doris/blob/0430714ca9b0d910ba9b290276da05adfc752f8f/be/src/runtime/fragment_mgr.cpp#L461-L463
to avoid ThreadPool::offer() failed.
If we replace it with kudu thread pool, we'll met offer() failed. So we must handle this situation:
```exec_state created -> prepare -> submit to thread pool failed (means never execute)-> destory```
I'll force fragment_mgr to use thread pool ( not create pthread) to test it. No need to replace thread pool firstly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] vagetablechicken edited a comment on issue #3416: Replace the thread pool in FragmentMgr
Posted by GitBox <gi...@apache.org>.
vagetablechicken edited a comment on issue #3416:
URL: https://github.com/apache/incubator-doris/issues/3416#issuecomment-626102191
Current fragment_mgr use
https://github.com/apache/incubator-doris/blob/0430714ca9b0d910ba9b290276da05adfc752f8f/be/src/runtime/fragment_mgr.cpp#L461-L463
to avoid ThreadPool::offer() failed.
If we replace it with kudu thread pool, we'll met offer() failed. So we must handle this situation:
```exec_state created -> prepare -> submit to thread pool failed (means never execute)-> destory```
FragmentMgr's thread pool is `PriorityThreadPool`, it use blocking_put() to offer queue.
### So we need replace it with thread pool first.
### Then add a UT to test if offer failed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org