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

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

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


##########
be/src/olap/cumulative_compaction_policy.cpp:
##########
@@ -460,16 +461,11 @@ void NumBasedCumulativeCompactionPolicy::calculate_cumulative_point(
 }
 
 void CumulativeCompactionPolicy::pick_candidate_rowsets(
-        int64_t skip_window_sec,
         const std::unordered_map<Version, RowsetSharedPtr, HashOfVersion>& rs_version_map,
         int64_t cumulative_point, std::vector<RowsetSharedPtr>* candidate_rowsets) {
-    int64_t now = UnixSeconds();
     for (auto& it : rs_version_map) {
         // find all rowset version greater than cumulative_point and skip the create time in skip_window_sec
-        if (it.first.first >= cumulative_point &&
-            ((it.second->creation_time() + skip_window_sec < now)
-             // this case means a rowset has been compacted before which is not a new published rowset, so it should participate compaction
-             || (it.first.first != it.first.second))) {

Review Comment:
   We should not remove this condition?



##########
be/src/olap/tablet.cpp:
##########
@@ -713,21 +713,6 @@ bool Tablet::can_do_compaction(size_t path_hash, CompactionType compaction_type)
         return false;
     }
 
-    if (tablet_state() == TABLET_RUNNING) {
-        // if tablet state is running, we need to check if it has consistent versions.
-        // tablet in other state such as TABLET_NOTREADY may not have complete versions.
-        std::shared_lock rdlock(_meta_lock);
-        const RowsetSharedPtr lastest_delta = rowset_with_max_version();

Review Comment:
   the `rowset_with_max_version` method checked un-published version, we'd better change the implementation here.
   e.g. if the published rowset have missing version, we should cancel compaction.



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