[GitHub] [incubator-doris] morningman commented on a diff in pull request #10153: [opt](compaction) optimize compaction in concurrent load

[GitBox <gi...@apache.org>]

morningman commented on code in PR #10153:
URL: https://github.com/apache/incubator-doris/pull/10153#discussion_r897627928


##########
be/src/olap/cumulative_compaction.cpp:
##########
@@ -95,16 +95,12 @@ Status CumulativeCompaction::execute_compact_impl() {
 Status CumulativeCompaction::pick_rowsets_to_compact() {
     std::vector<RowsetSharedPtr> candidate_rowsets;
 
-    _tablet->pick_candidate_rowsets_to_cumulative_compaction(
-            config::cumulative_compaction_skip_window_seconds, &candidate_rowsets);

Review Comment:
   remove `cumulative_compaction_skip_window_seconds` this from config.h



##########
be/src/common/config.h:
##########
@@ -282,17 +282,14 @@ CONF_mInt64(cumulative_size_based_compaction_lower_size_mbytes, "64");
 // cumulative compaction policy: min and max delta file's number
 CONF_mInt64(min_cumulative_compaction_num_singleton_deltas, "5");
 CONF_mInt64(max_cumulative_compaction_num_singleton_deltas, "1000");
-// cumulative compaction skips recently published deltas in order to prevent
-// compacting a version that might be queried (in case the query planning phase took some time).
-// the following config set the window size
-CONF_mInt32(cumulative_compaction_skip_window_seconds, "30");
 
 // if compaction of a tablet failed, this tablet should not be chosen to
 // compaction until this interval passes.
 CONF_mInt64(min_compaction_failure_interval_sec, "5"); // 5 seconds
 
 // This config can be set to limit thread number in compaction thread pool.
-CONF_mInt32(max_compaction_threads, "10");
+CONF_mInt32(max_base_compaction_threads, "4");
+CONF_mInt32(max_cumu_compaction_threads, "10");

Review Comment:
   Modify the document: 
   docs/zh-CN/docs/admin-manual/config/be-config.md
   docs/en/docs/admin-manual/config/be-config.md



##########
be/src/olap/cumulative_compaction.cpp:
##########
@@ -95,16 +95,12 @@ Status CumulativeCompaction::execute_compact_impl() {
 Status CumulativeCompaction::pick_rowsets_to_compact() {
     std::vector<RowsetSharedPtr> candidate_rowsets;
 
-    _tablet->pick_candidate_rowsets_to_cumulative_compaction(
-            config::cumulative_compaction_skip_window_seconds, &candidate_rowsets);
+    _tablet->pick_candidate_rowsets_to_cumulative_compaction(&candidate_rowsets);
 
     if (candidate_rowsets.empty()) {
         return Status::OLAPInternalError(OLAP_ERR_CUMULATIVE_NO_SUITABLE_VERSION);
     }
 
-    // candidate_rowsets may not be continuous. Because some rowset may not be selected
-    // because the protection time has not expired(config::cumulative_compaction_skip_window_seconds).
-    // So we need to choose the longest continuous path from it.

Review Comment:
   We still need this comment. Because even if we remove the `cumulative_compaction_skip_window_seconds`, the rowset picked may still not be consecutive, because of the concurrent publish.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

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