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 2021/04/09 05:20:20 UTC
[GitHub] [incubator-doris] weizuo93 opened a new pull request #5618: [Compaction] Use separate thread pool for base and cumulative compaction
weizuo93 opened a new pull request #5618:
URL: https://github.com/apache/incubator-doris/pull/5618
## Proposed changes
We hope that `cumulative compaction` can be implemented as soon as possible, but `base compaction` tasks are likely to execute for a long time and occupy most of the threads in thread pool, which may result in high CPU load.
This patch submits `cumulative compaction` and `base compaction` into separate thread pool. So we have more flexibility to adjust the execution of `cumulative compaction` and `base compaction`according to the status of cluster.
## Types of changes
What types of changes does your code introduce to Doris?
_Put an `x` in the boxes that apply_
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Documentation Update (if none of the other choices apply)
- [x] Code refactor (Modify the code structure, format the code, etc...)
--
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] acelyc111 commented on a change in pull request #5618: [Compaction] Use separate thread pool for base and cumulative compaction
Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #5618:
URL: https://github.com/apache/incubator-doris/pull/5618#discussion_r611290555
##########
File path: be/src/common/config.h
##########
@@ -283,8 +283,10 @@ CONF_mInt32(cumulative_compaction_skip_window_seconds, "30");
CONF_mInt64(min_compaction_failure_interval_sec, "600"); // 10 min
// This config can be set to limit thread number in compaction thread pool.
-CONF_mInt32(min_compaction_threads, "10");
-CONF_mInt32(max_compaction_threads, "10");
+CONF_mInt32(min_base_compaction_threads, "10");
+CONF_mInt32(max_base_compaction_threads, "10");
+CONF_mInt32(min_cumulative_compaction_threads, "10");
+CONF_mInt32(max_cumulative_compaction_threads, "10");
Review comment:
They are not mutable, use CONF_Int32 instead.
##########
File path: be/src/olap/storage_engine.h
##########
@@ -334,12 +336,15 @@ class StorageEngine {
HeartbeatFlags* _heartbeat_flags;
- std::unique_ptr<ThreadPool> _compaction_thread_pool;
+ std::unique_ptr<ThreadPool> _base_compaction_thread_pool;
+ std::unique_ptr<ThreadPool> _cumulative_compaction_thread_pool;
CompactionPermitLimiter _permit_limiter;
- std::mutex _tablet_submitted_compaction_mutex;
- std::map<DataDir*, vector<TTabletId>> _tablet_submitted_compaction;
+ RWMutex _tablet_submitted_base_compaction_mutex;
+ std::map<DataDir*, std::set<TTabletId>> _tablet_submitted_base_compaction;
+ RWMutex _tablet_submitted_cumulative_compaction_mutex;
+ std::map<DataDir*, std::set<TTabletId>> _tablet_submitted_cumulative_compaction;
Review comment:
How about use a two elements array, one for base compaction, the other for cumulative compaction, to simplify code?
##########
File path: be/src/olap/tablet_manager.cpp
##########
@@ -693,12 +694,16 @@ TabletSharedPtr TabletManager::find_best_tablet_to_compaction(
ReadLock rlock(tablets_shard.lock.get());
for (const auto& tablet_map : tablets_shard.tablet_map) {
for (const TabletSharedPtr& tablet_ptr : tablet_map.second.table_arr) {
- std::vector<TTabletId>::iterator it_tablet =
- find(tablet_submitted_compaction.begin(), tablet_submitted_compaction.end(),
- tablet_ptr->tablet_id());
- if (it_tablet != tablet_submitted_compaction.end()) {
+ std::set<TTabletId>::iterator it_tablet =
+ (*tablet_submitted_base_compaction).find(tablet_ptr->tablet_id());
+ if (it_tablet != (*tablet_submitted_base_compaction).end()) {
continue;
}
+ it_tablet = (*tablet_submitted_cumulative_compaction).find(tablet_ptr->tablet_id());
+ if (it_tablet != (*tablet_submitted_cumulative_compaction).end()) {
+ continue;
+ }
+
Review comment:
You can use `ContainsKey` in be/src/gutil/map-util.h instead to simplify code.
--
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] weizuo93 commented on a change in pull request #5618: [Compaction] Use separate thread pool for base and cumulative compaction
Posted by GitBox <gi...@apache.org>.
weizuo93 commented on a change in pull request #5618:
URL: https://github.com/apache/incubator-doris/pull/5618#discussion_r612195790
##########
File path: be/src/olap/storage_engine.h
##########
@@ -334,12 +336,15 @@ class StorageEngine {
HeartbeatFlags* _heartbeat_flags;
- std::unique_ptr<ThreadPool> _compaction_thread_pool;
+ std::unique_ptr<ThreadPool> _base_compaction_thread_pool;
+ std::unique_ptr<ThreadPool> _cumulative_compaction_thread_pool;
CompactionPermitLimiter _permit_limiter;
- std::mutex _tablet_submitted_compaction_mutex;
- std::map<DataDir*, vector<TTabletId>> _tablet_submitted_compaction;
+ RWMutex _tablet_submitted_base_compaction_mutex;
+ std::map<DataDir*, std::set<TTabletId>> _tablet_submitted_base_compaction;
+ RWMutex _tablet_submitted_cumulative_compaction_mutex;
+ std::map<DataDir*, std::set<TTabletId>> _tablet_submitted_cumulative_compaction;
Review comment:
> How about use a two elements array, one for base compaction, the other for cumulative compaction, to simplify code?
OK, Thank you.
--
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] weizuo93 closed pull request #5618: [Compaction] Use separate thread pool for base and cumulative compaction
Posted by GitBox <gi...@apache.org>.
weizuo93 closed pull request #5618:
URL: https://github.com/apache/incubator-doris/pull/5618
--
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] weizuo93 commented on a change in pull request #5618: [Compaction] Use separate thread pool for base and cumulative compaction
Posted by GitBox <gi...@apache.org>.
weizuo93 commented on a change in pull request #5618:
URL: https://github.com/apache/incubator-doris/pull/5618#discussion_r612174615
##########
File path: be/src/olap/tablet_manager.cpp
##########
@@ -693,12 +694,16 @@ TabletSharedPtr TabletManager::find_best_tablet_to_compaction(
ReadLock rlock(tablets_shard.lock.get());
for (const auto& tablet_map : tablets_shard.tablet_map) {
for (const TabletSharedPtr& tablet_ptr : tablet_map.second.table_arr) {
- std::vector<TTabletId>::iterator it_tablet =
- find(tablet_submitted_compaction.begin(), tablet_submitted_compaction.end(),
- tablet_ptr->tablet_id());
- if (it_tablet != tablet_submitted_compaction.end()) {
+ std::set<TTabletId>::iterator it_tablet =
+ (*tablet_submitted_base_compaction).find(tablet_ptr->tablet_id());
+ if (it_tablet != (*tablet_submitted_base_compaction).end()) {
continue;
}
+ it_tablet = (*tablet_submitted_cumulative_compaction).find(tablet_ptr->tablet_id());
+ if (it_tablet != (*tablet_submitted_cumulative_compaction).end()) {
+ continue;
+ }
+
Review comment:
> You can use `ContainsKey` in be/src/gutil/map-util.h instead to simplify code.
We need to judge whether an element exists in `set`rather than `map`.
--
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] weizuo93 commented on a change in pull request #5618: [Compaction] Use separate thread pool for base and cumulative compaction
Posted by GitBox <gi...@apache.org>.
weizuo93 commented on a change in pull request #5618:
URL: https://github.com/apache/incubator-doris/pull/5618#discussion_r612172025
##########
File path: be/src/common/config.h
##########
@@ -283,8 +283,10 @@ CONF_mInt32(cumulative_compaction_skip_window_seconds, "30");
CONF_mInt64(min_compaction_failure_interval_sec, "600"); // 10 min
// This config can be set to limit thread number in compaction thread pool.
-CONF_mInt32(min_compaction_threads, "10");
-CONF_mInt32(max_compaction_threads, "10");
+CONF_mInt32(min_base_compaction_threads, "10");
+CONF_mInt32(max_base_compaction_threads, "10");
+CONF_mInt32(min_cumulative_compaction_threads, "10");
+CONF_mInt32(max_cumulative_compaction_threads, "10");
Review comment:
> They are not mutable, use CONF_Int32 instead.
OK, thanks.
--
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