github-actions[bot] commented on code in PR #16258:
URL: https://github.com/apache/doris/pull/16258#discussion_r1095493925


##########
be/src/olap/tablet.cpp:
##########
@@ -1642,10 +1650,110 @@ Status Tablet::cooldown() {
         return Status::Error<TRY_LOCK_FAILED>();
     }
 
-    if (_tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) {
-        RETURN_IF_ERROR(_cooldown_data());
+    if (_need_deal_cooldown_delete
+            && _tablet_meta->cooldown_replica_id() == 
_tablet_meta->replica_id()) {
+        RETURN_IF_ERROR(_deal_cooldown_delete_files());
+    }
+
+    if (_need_cooldown) {

Review Comment:
   warning: use of undeclared identifier 'dest_fs' [clang-diagnostic-error]
   ```cpp
   _delete_files(dest_fs));
                 ^
   ```
   



##########
be/src/olap/tablet.cpp:
##########
@@ -1642,10 +1650,110 @@ Status Tablet::cooldown() {
         return Status::Error<TRY_LOCK_FAILED>();
     }
 
-    if (_tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) {
-        RETURN_IF_ERROR(_cooldown_data());
+    if (_need_deal_cooldown_delete
+            && _tablet_meta->cooldown_replica_id() == 
_tablet_meta->replica_id()) {
+        RETURN_IF_ERROR(_deal_cooldown_delete_files());
+    }
+
+    if (_need_cooldown) {
+        if (_tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) 
{
+            RETURN_IF_ERROR(_cooldown_data());
+        } else {
+            RETURN_IF_ERROR(_follow_cooldowned_data());
+        }
+    }
+
+    return Status::OK();
+}
+
+bool Tablet::get_cooldown_delete_id(TUniqueId* cooldown_delete_id) {
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return false;
+    }
+    if (!_cooldown_delete_flag && _cooldown_delete_files.size() > 0) {
+        *cooldown_delete_id = _cooldown_delete_id;
+        return true;
+    }
+    return false;
+}
+
+void Tablet::enable_cooldown_flag(const TUniqueId& cooldown_delete_id) {
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return;
+    }
+    if (cooldown_delete_id == _cooldown_delete_id) {
+        _cooldown_delete_flag = true;
+    }
+}
+
+bool Tablet::need_deal_cooldown_delete() {
+    if (_tablet_meta->cooldown_replica_id() != _tablet_meta->replica_id()) {
+        _need_deal_cooldown_delete = false;
+        return false;
+    }
+    _need_deal_cooldown_delete = _cooldown_delete_flag
+            || time(NULL) - _last_cooldown_delete_time >= 
config::cooldown_delete_interval_time_sec;

Review Comment:
   warning: use nullptr [modernize-use-nullptr]
   
   ```suggestion
   _delete = _cooldown_delete_flagnullptr
   ```
   



##########
be/src/olap/tablet.cpp:
##########
@@ -1642,10 +1650,110 @@ Status Tablet::cooldown() {
         return Status::Error<TRY_LOCK_FAILED>();
     }
 
-    if (_tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) {
-        RETURN_IF_ERROR(_cooldown_data());
+    if (_need_deal_cooldown_delete
+            && _tablet_meta->cooldown_replica_id() == 
_tablet_meta->replica_id()) {
+        RETURN_IF_ERROR(_deal_cooldown_delete_files());
+    }
+
+    if (_need_cooldown) {
+        if (_tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) 
{
+            RETURN_IF_ERROR(_cooldown_data());
+        } else {
+            RETURN_IF_ERROR(_follow_cooldowned_data());
+        }
+    }
+
+    return Status::OK();
+}
+
+bool Tablet::get_cooldown_delete_id(TUniqueId* cooldown_delete_id) {
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return false;
+    }
+    if (!_cooldown_delete_flag && _cooldown_delete_files.size() > 0) {
+        *cooldown_delete_id = _cooldown_delete_id;
+        return true;
+    }
+    return false;
+}
+
+void Tablet::enable_cooldown_flag(const TUniqueId& cooldown_delete_id) {
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return;
+    }
+    if (cooldown_delete_id == _cooldown_delete_id) {
+        _cooldown_delete_flag = true;
+    }
+}
+
+bool Tablet::need_deal_cooldown_delete() {
+    if (_tablet_meta->cooldown_replica_id() != _tablet_meta->replica_id()) {
+        _need_deal_cooldown_delete = false;
+        return false;
+    }
+    _need_deal_cooldown_delete = _cooldown_delete_flag
+            || time(NULL) - _last_cooldown_delete_time >= 
config::cooldown_delete_interval_time_sec;
+    return _need_deal_cooldown_delete;
+}
+
+Status Tablet::_deal_cooldown_delete_files() {
+    auto dest_fs = io::FileSystemMap::instance()->get(storage_policy());
+    if (!dest_fs) {
+        return Status::Error<UNINITIALIZED>();
+    }
+    if (dest_fs->type() != io::FileSystemType::S3) {
+        return Status::Error<UNINITIALIZED>();
+    }
+
+    TabletMetaPB remote_tablet_meta_pb;
+    _tablet_meta->to_meta_pb(true, &remote_tablet_meta_pb);
+    std::map<std::string, bool> remote_segment_path_map;
+    for (auto& rowset_meta_pb : remote_tablet_meta_pb.rs_metas()) {
+        for (int i = 0; i < rowset_meta_pb.num_segments(); ++i) {
+            std::string segment_path = BetaRowset::remote_segment_path(
+                    tablet_id(), rowset_meta_pb.rowset_id_v2(), i);
+            remote_segment_path_map.emplace(segment_path, true);
+        }
+    }
+    if (remote_segment_path_map.size() == 0) {
+        return Status::OK();
+    }
+
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return Status::Error<TRY_LOCK_FAILED>();
+    }
+    if (_cooldown_delete_flag) {
+        for (auto& file_path : _cooldown_delete_files) {
+            LOG(INFO) << "delete invalid remote file: " << file_path;
+            RETURN_IF_ERROR(fs->delete_file(file_path));
+        }

Review Comment:
   warning: unexpected namespace name 'fs': expected expression 
[clang-diagnostic-error]
   ```cpp
   OR(fs->delete_file(file_path));
      ^
   ```
   



##########
be/src/olap/tablet.cpp:
##########
@@ -1642,10 +1650,110 @@ Status Tablet::cooldown() {
         return Status::Error<TRY_LOCK_FAILED>();
     }
 
-    if (_tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) {
-        RETURN_IF_ERROR(_cooldown_data());
+    if (_need_deal_cooldown_delete
+            && _tablet_meta->cooldown_replica_id() == 
_tablet_meta->replica_id()) {
+        RETURN_IF_ERROR(_deal_cooldown_delete_files());
+    }
+
+    if (_need_cooldown) {
+        if (_tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) 
{
+            RETURN_IF_ERROR(_cooldown_data());
+        } else {
+            RETURN_IF_ERROR(_follow_cooldowned_data());
+        }
+    }
+
+    return Status::OK();
+}
+
+bool Tablet::get_cooldown_delete_id(TUniqueId* cooldown_delete_id) {
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return false;
+    }
+    if (!_cooldown_delete_flag && _cooldown_delete_files.size() > 0) {
+        *cooldown_delete_id = _cooldown_delete_id;
+        return true;
+    }
+    return false;
+}
+
+void Tablet::enable_cooldown_flag(const TUniqueId& cooldown_delete_id) {
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return;
+    }
+    if (cooldown_delete_id == _cooldown_delete_id) {
+        _cooldown_delete_flag = true;
+    }
+}
+
+bool Tablet::need_deal_cooldown_delete() {
+    if (_tablet_meta->cooldown_replica_id() != _tablet_meta->replica_id()) {
+        _need_deal_cooldown_delete = false;
+        return false;
+    }
+    _need_deal_cooldown_delete = _cooldown_delete_flag
+            || time(NULL) - _last_cooldown_delete_time >= 
config::cooldown_delete_interval_time_sec;
+    return _need_deal_cooldown_delete;
+}
+
+Status Tablet::_deal_cooldown_delete_files() {
+    auto dest_fs = io::FileSystemMap::instance()->get(storage_policy());

Review Comment:
   warning: out-of-line definition of '_deal_cooldown_delete_files' does not 
match any declaration in 'doris::Tablet' [clang-diagnostic-error]
   ```cpp
   _deal_cooldown_delete_files() {
   ^
   ```
   



##########
be/src/olap/tablet.cpp:
##########
@@ -1642,10 +1650,110 @@ Status Tablet::cooldown() {
         return Status::Error<TRY_LOCK_FAILED>();
     }
 
-    if (_tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) {
-        RETURN_IF_ERROR(_cooldown_data());
+    if (_need_deal_cooldown_delete
+            && _tablet_meta->cooldown_replica_id() == 
_tablet_meta->replica_id()) {
+        RETURN_IF_ERROR(_deal_cooldown_delete_files());
+    }
+
+    if (_need_cooldown) {
+        if (_tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) 
{
+            RETURN_IF_ERROR(_cooldown_data());
+        } else {
+            RETURN_IF_ERROR(_follow_cooldowned_data());
+        }
+    }
+
+    return Status::OK();
+}
+
+bool Tablet::get_cooldown_delete_id(TUniqueId* cooldown_delete_id) {
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return false;
+    }
+    if (!_cooldown_delete_flag && _cooldown_delete_files.size() > 0) {
+        *cooldown_delete_id = _cooldown_delete_id;
+        return true;
+    }
+    return false;
+}
+
+void Tablet::enable_cooldown_flag(const TUniqueId& cooldown_delete_id) {
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return;
+    }
+    if (cooldown_delete_id == _cooldown_delete_id) {
+        _cooldown_delete_flag = true;
+    }
+}
+
+bool Tablet::need_deal_cooldown_delete() {
+    if (_tablet_meta->cooldown_replica_id() != _tablet_meta->replica_id()) {
+        _need_deal_cooldown_delete = false;
+        return false;
+    }
+    _need_deal_cooldown_delete = _cooldown_delete_flag
+            || time(NULL) - _last_cooldown_delete_time >= 
config::cooldown_delete_interval_time_sec;
+    return _need_deal_cooldown_delete;
+}
+
+Status Tablet::_deal_cooldown_delete_files() {
+    auto dest_fs = io::FileSystemMap::instance()->get(storage_policy());
+    if (!dest_fs) {
+        return Status::Error<UNINITIALIZED>();
+    }
+    if (dest_fs->type() != io::FileSystemType::S3) {
+        return Status::Error<UNINITIALIZED>();
+    }
+
+    TabletMetaPB remote_tablet_meta_pb;
+    _tablet_meta->to_meta_pb(true, &remote_tablet_meta_pb);
+    std::map<std::string, bool> remote_segment_path_map;
+    for (auto& rowset_meta_pb : remote_tablet_meta_pb.rs_metas()) {
+        for (int i = 0; i < rowset_meta_pb.num_segments(); ++i) {
+            std::string segment_path = BetaRowset::remote_segment_path(
+                    tablet_id(), rowset_meta_pb.rowset_id_v2(), i);
+            remote_segment_path_map.emplace(segment_path, true);
+        }
+    }
+    if (remote_segment_path_map.size() == 0) {
+        return Status::OK();
+    }
+
+    std::unique_lock delete_lock(_cooldown_delete_lock, std::try_to_lock);
+    if (!delete_lock.owns_lock()) {
+        LOG(WARNING) << "Failed to own delete_lock. tablet=" << tablet_id();
+        return Status::Error<TRY_LOCK_FAILED>();
+    }
+    if (_cooldown_delete_flag) {
+        for (auto& file_path : _cooldown_delete_files) {
+            LOG(INFO) << "delete invalid remote file: " << file_path;
+            RETURN_IF_ERROR(fs->delete_file(file_path));
+        }
+        _cooldown_delete_flag = false;
+        _cooldown_delete_files.clear();
     } else {
-        RETURN_IF_ERROR(_follow_cooldowned_data());
+        if (time(NULL) - _last_cooldown_delete_time < 
config::cooldown_delete_interval_time_sec) {
+            return Status::OK();
+        }
+        _cooldown_delete_files.clear();
+        _cooldown_delete_id = generate_uuid();
+        io::Path remote_tablet_path = 
BetaRowset::remote_tablet_path(tablet_id());
+        std::vector<io::Path> segment_files;
+        RETURN_IF_ERROR(fs->list(remote_tablet_path, &segment_files));

Review Comment:
   warning: unexpected namespace name 'fs': expected expression 
[clang-diagnostic-error]
   ```cpp
   vector<io::Path> segment_files;
                                                           ^
   ```
   



-- 
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-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to