github-actions[bot] commented on code in PR #16258: URL: https://github.com/apache/doris/pull/16258#discussion_r1098099725
########## be/src/olap/tablet.cpp: ########## @@ -1627,7 +1635,7 @@ Status Tablet::create_rowset(RowsetMetaSharedPtr rowset_meta, RowsetSharedPtr* r return RowsetFactory::create_rowset(tablet_schema(), tablet_path(), rowset_meta, rowset); } -Status Tablet::cooldown() { +Status Tablet::cooldown(io::RemoteFileSystem* fs) { Review Comment: warning: out-of-line definition of 'cooldown' does not match any declaration in 'doris::Tablet' [clang-diagnostic-error] ```cpp ath(), rowset_meta, rowset); ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1661,15 +1670,127 @@ } DCHECK(atol(dest_fs->id().c_str()) == storage_policy->resource_id); DCHECK(dest_fs->type() != io::FileSystemType::LOCAL); - if (cooldown_replica_id == replica_id()) { - RETURN_IF_ERROR(_cooldown_data(dest_fs)); + + if (_need_deal_cooldown_delete + && _tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) { Review Comment: warning: no member named 'cooldown_replica_id' in 'doris::TabletMeta' [clang-diagnostic-error] ```cpp if (_need_deal_cooldown_delete ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1661,15 +1670,127 @@ } DCHECK(atol(dest_fs->id().c_str()) == storage_policy->resource_id); DCHECK(dest_fs->type() != io::FileSystemType::LOCAL); - if (cooldown_replica_id == replica_id()) { - RETURN_IF_ERROR(_cooldown_data(dest_fs)); + + if (_need_deal_cooldown_delete + && _tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) { + RETURN_IF_ERROR(_deal_cooldown_delete_files(dest_fs)); + } + + if (_need_cooldown) { + if (cooldown_replica_id == replica_id()) { + RETURN_IF_ERROR(_cooldown_data(dest_fs)); + } else { + RETURN_IF_ERROR(_follow_cooldowned_data(dest_fs.get(), cooldown_replica_id)); + } + } + + 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()) { Review Comment: warning: no member named 'cooldown_replica_id' in 'doris::TabletMeta' [clang-diagnostic-error] ```cpp ::need_deal_cooldown_delete() { ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1661,15 +1670,127 @@ } DCHECK(atol(dest_fs->id().c_str()) == storage_policy->resource_id); DCHECK(dest_fs->type() != io::FileSystemType::LOCAL); - if (cooldown_replica_id == replica_id()) { - RETURN_IF_ERROR(_cooldown_data(dest_fs)); + + if (_need_deal_cooldown_delete + && _tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) { + RETURN_IF_ERROR(_deal_cooldown_delete_files(dest_fs)); + } + + if (_need_cooldown) { + if (cooldown_replica_id == replica_id()) { + RETURN_IF_ERROR(_cooldown_data(dest_fs)); + } else { + RETURN_IF_ERROR(_follow_cooldowned_data(dest_fs.get(), cooldown_replica_id)); + } + } + + 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(const std::shared_ptr<io::RemoteFileSystem>& dest_fs) { + auto fs = io::FileSystemMap::instance()->get(storage_policy()); + if (!fs) { Review Comment: warning: use of undeclared identifier 'storage_policy' [clang-diagnostic-error] ```cpp tance()->get(storage_policy()); ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1661,15 +1670,127 @@ } DCHECK(atol(dest_fs->id().c_str()) == storage_policy->resource_id); DCHECK(dest_fs->type() != io::FileSystemType::LOCAL); - if (cooldown_replica_id == replica_id()) { - RETURN_IF_ERROR(_cooldown_data(dest_fs)); + + if (_need_deal_cooldown_delete + && _tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) { + RETURN_IF_ERROR(_deal_cooldown_delete_files(dest_fs)); + } + + if (_need_cooldown) { + if (cooldown_replica_id == replica_id()) { + RETURN_IF_ERROR(_cooldown_data(dest_fs)); + } else { + RETURN_IF_ERROR(_follow_cooldowned_data(dest_fs.get(), cooldown_replica_id)); + } + } + + 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(const std::shared_ptr<io::RemoteFileSystem>& dest_fs) { + auto fs = io::FileSystemMap::instance()->get(storage_policy()); + if (!fs) { + return Status::Error<UNINITIALIZED>(); + } + if (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_name_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_filename = io::Path(BetaRowset::remote_segment_path( + tablet_id(), rowset_meta_pb.rowset_id_v2(), i)).filename(); + boost::replace_all(segment_filename, "\"", ""); + remote_segment_name_map.emplace(segment_filename, true); + } + } + if (remote_segment_name_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(dest_fs.get(), cooldown_replica_id)); + 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)); + + for (auto& path : segment_files) { + std::string filename = path.filename(); + boost::replace_all(filename, "\"", ""); + if (!ends_with(filename, ".dat")) { + continue; + } + if (remote_segment_name_map.find(filename) == remote_segment_name_map.end()) { + _cooldown_delete_files.emplace_back(path); + } + } + _last_cooldown_delete_time = time(NULL); } return Status::OK(); } -Status Tablet::_cooldown_data(const std::shared_ptr<io::RemoteFileSystem>& dest_fs) { +Status Tablet::_cooldown_data() { + auto dest_fs = io::FileSystemMap::instance()->get(storage_policy()); Review Comment: warning: out-of-line definition of '_cooldown_data' does not match any declaration in 'doris::Tablet' [clang-diagnostic-error] ```cpp atus Tablet::_cooldown_data() { ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1752,6 +1876,8 @@ return Status::InternalError("version not continuous"); } TabletMetaPB tablet_meta_pb; + tablet_meta_pb.mutable_cooldown_meta_id()->set_hi(_cooldown_meta_id.hi); + tablet_meta_pb.mutable_cooldown_meta_id()->set_lo(_cooldown_meta_id.lo); auto rs_metas = tablet_meta_pb.mutable_rs_metas(); Review Comment: warning: use of undeclared identifier '_cooldown_meta_id'; did you mean 'cooldown_meta_id'? [clang-diagnostic-error] ```suggestion ->set_lo(cooldown_meta_id.lo); ``` **be/src/olap/tablet.cpp:1862:** 'cooldown_meta_id' declared here ```cpp st TUniqueId& cooldown_meta_id, ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1661,15 +1670,127 @@ } DCHECK(atol(dest_fs->id().c_str()) == storage_policy->resource_id); DCHECK(dest_fs->type() != io::FileSystemType::LOCAL); - if (cooldown_replica_id == replica_id()) { - RETURN_IF_ERROR(_cooldown_data(dest_fs)); + + if (_need_deal_cooldown_delete + && _tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) { + RETURN_IF_ERROR(_deal_cooldown_delete_files(dest_fs)); + } + + if (_need_cooldown) { + if (cooldown_replica_id == replica_id()) { + RETURN_IF_ERROR(_cooldown_data(dest_fs)); + } else { + RETURN_IF_ERROR(_follow_cooldowned_data(dest_fs.get(), cooldown_replica_id)); + } + } + + 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(const std::shared_ptr<io::RemoteFileSystem>& dest_fs) { + auto fs = io::FileSystemMap::instance()->get(storage_policy()); + if (!fs) { + return Status::Error<UNINITIALIZED>(); + } + if (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_name_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_filename = io::Path(BetaRowset::remote_segment_path( + tablet_id(), rowset_meta_pb.rowset_id_v2(), i)).filename(); + boost::replace_all(segment_filename, "\"", ""); + remote_segment_name_map.emplace(segment_filename, true); + } + } + if (remote_segment_name_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(dest_fs.get(), cooldown_replica_id)); + 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)); + + for (auto& path : segment_files) { + std::string filename = path.filename(); + boost::replace_all(filename, "\"", ""); + if (!ends_with(filename, ".dat")) { + continue; + } + if (remote_segment_name_map.find(filename) == remote_segment_name_map.end()) { + _cooldown_delete_files.emplace_back(path); + } + } + _last_cooldown_delete_time = time(NULL); } return Status::OK(); } -Status Tablet::_cooldown_data(const std::shared_ptr<io::RemoteFileSystem>& dest_fs) { +Status Tablet::_cooldown_data() { + auto dest_fs = io::FileSystemMap::instance()->get(storage_policy()); + if (!dest_fs) { Review Comment: warning: use of undeclared identifier 'storage_policy' [clang-diagnostic-error] ```cpp tance()->get(storage_policy()); ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1752,6 +1876,8 @@ return Status::InternalError("version not continuous"); } TabletMetaPB tablet_meta_pb; + tablet_meta_pb.mutable_cooldown_meta_id()->set_hi(_cooldown_meta_id.hi); + tablet_meta_pb.mutable_cooldown_meta_id()->set_lo(_cooldown_meta_id.lo); Review Comment: warning: use of undeclared identifier '_cooldown_meta_id'; did you mean 'cooldown_meta_id'? [clang-diagnostic-error] ```suggestion ->set_hi(cooldown_meta_id.hi); ``` **be/src/olap/tablet.cpp:1862:** 'cooldown_meta_id' declared here ```cpp st TUniqueId& cooldown_meta_id, ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1661,15 +1670,127 @@ } DCHECK(atol(dest_fs->id().c_str()) == storage_policy->resource_id); DCHECK(dest_fs->type() != io::FileSystemType::LOCAL); - if (cooldown_replica_id == replica_id()) { - RETURN_IF_ERROR(_cooldown_data(dest_fs)); + + if (_need_deal_cooldown_delete + && _tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) { + RETURN_IF_ERROR(_deal_cooldown_delete_files(dest_fs)); + } + + if (_need_cooldown) { + if (cooldown_replica_id == replica_id()) { + RETURN_IF_ERROR(_cooldown_data(dest_fs)); + } else { + RETURN_IF_ERROR(_follow_cooldowned_data(dest_fs.get(), cooldown_replica_id)); + } + } + + 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(const std::shared_ptr<io::RemoteFileSystem>& dest_fs) { + auto fs = io::FileSystemMap::instance()->get(storage_policy()); Review Comment: warning: no member named 'FileSystemMap' in namespace 'doris::io' [clang-diagnostic-error] ```cpp ::RemoteFileSystem>& dest_fs) { ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1661,15 +1670,127 @@ } DCHECK(atol(dest_fs->id().c_str()) == storage_policy->resource_id); DCHECK(dest_fs->type() != io::FileSystemType::LOCAL); - if (cooldown_replica_id == replica_id()) { - RETURN_IF_ERROR(_cooldown_data(dest_fs)); + + if (_need_deal_cooldown_delete + && _tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) { + RETURN_IF_ERROR(_deal_cooldown_delete_files(dest_fs)); + } + + if (_need_cooldown) { + if (cooldown_replica_id == replica_id()) { + RETURN_IF_ERROR(_cooldown_data(dest_fs)); + } else { + RETURN_IF_ERROR(_follow_cooldowned_data(dest_fs.get(), cooldown_replica_id)); + } + } + + 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(const std::shared_ptr<io::RemoteFileSystem>& dest_fs) { + auto fs = io::FileSystemMap::instance()->get(storage_policy()); + if (!fs) { + return Status::Error<UNINITIALIZED>(); + } + if (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_name_map; Review Comment: warning: too many arguments to function call, expected single argument 'tablet_meta_pb', have 2 arguments [clang-diagnostic-error] ```cpp (true, &remote_tablet_meta_pb); ^ ``` **be/src/olap/tablet_meta.h:112:** 'to_meta_pb' declared here ```cpp void to_meta_pb(TabletMetaPB* tablet_meta_pb); ^ ``` ########## be/src/olap/tablet_meta.cpp: ########## @@ -501,11 +501,17 @@ void TabletMeta::init_from_pb(const TabletMetaPB& tablet_meta_pb) { // init _schema _schema->init_from_pb(tablet_meta_pb.schema()); + _cooldowned_version = -1; // init _rs_metas for (auto& it : tablet_meta_pb.rs_metas()) { RowsetMetaSharedPtr rs_meta(new RowsetMeta()); rs_meta->init_from_pb(it); _rs_metas.push_back(std::move(rs_meta)); + if (tablet_meta_pb.storage_policy() != "" && Review Comment: warning: no member named 'storage_policy' in 'doris::TabletMetaPB' [clang-diagnostic-error] ```cpp if (tablet_meta_pb.storage_policy() != "" && ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1830,6 +1956,7 @@ } _tablet_meta->modify_rs_metas(to_add, to_delete); + _tablet_meta->set_cooldown_meta_id(cooldown_meta_pb.cooldown_meta_id()); Review Comment: warning: no viable conversion from 'const ::doris::PUniqueId' to 'const doris::TUniqueId' [clang-diagnostic-error] ```cpp fy_rs_metas(to_add, to_delete); ^ ``` **gensrc/build/gen_cpp/Types_types.h:935:** candidate constructor not viable: no known conversion from 'const ::doris::PUniqueId' to 'const doris::TUniqueId &' for 1st argument ```cpp TUniqueId(const TUniqueId&); ^ ``` **be/src/olap/tablet_meta.h:195:** passing argument to parameter 'cooldown_meta_id' here ```cpp void set_cooldown_meta_id(const TUniqueId& cooldown_meta_id) { ^ ``` ########## be/src/olap/tablet.cpp: ########## @@ -1661,15 +1670,127 @@ } DCHECK(atol(dest_fs->id().c_str()) == storage_policy->resource_id); DCHECK(dest_fs->type() != io::FileSystemType::LOCAL); - if (cooldown_replica_id == replica_id()) { - RETURN_IF_ERROR(_cooldown_data(dest_fs)); + + if (_need_deal_cooldown_delete + && _tablet_meta->cooldown_replica_id() == _tablet_meta->replica_id()) { + RETURN_IF_ERROR(_deal_cooldown_delete_files(dest_fs)); + } + + if (_need_cooldown) { + if (cooldown_replica_id == replica_id()) { + RETURN_IF_ERROR(_cooldown_data(dest_fs)); + } else { + RETURN_IF_ERROR(_follow_cooldowned_data(dest_fs.get(), cooldown_replica_id)); + } + } + + 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(const std::shared_ptr<io::RemoteFileSystem>& dest_fs) { + auto fs = io::FileSystemMap::instance()->get(storage_policy()); + if (!fs) { + return Status::Error<UNINITIALIZED>(); + } + if (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_name_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_filename = io::Path(BetaRowset::remote_segment_path( + tablet_id(), rowset_meta_pb.rowset_id_v2(), i)).filename(); + boost::replace_all(segment_filename, "\"", ""); + remote_segment_name_map.emplace(segment_filename, true); + } + } + if (remote_segment_name_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(dest_fs.get(), cooldown_replica_id)); + 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)); + + for (auto& path : segment_files) { + std::string filename = path.filename(); + boost::replace_all(filename, "\"", ""); + if (!ends_with(filename, ".dat")) { + continue; + } + if (remote_segment_name_map.find(filename) == remote_segment_name_map.end()) { + _cooldown_delete_files.emplace_back(path); + } + } + _last_cooldown_delete_time = time(NULL); } return Status::OK(); } -Status Tablet::_cooldown_data(const std::shared_ptr<io::RemoteFileSystem>& dest_fs) { +Status Tablet::_cooldown_data() { + auto dest_fs = io::FileSystemMap::instance()->get(storage_policy()); Review Comment: warning: no member named 'FileSystemMap' in namespace 'doris::io' [clang-diagnostic-error] ```cpp atus Tablet::_cooldown_data() { ^ ``` -- 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