platoneko commented on code in PR #16089: URL: https://github.com/apache/doris/pull/16089#discussion_r1091429337
########## be/src/olap/tablet_manager.cpp: ########## @@ -1281,39 +1281,44 @@ struct SortCtx { SortCtx(TabletSharedPtr tablet, int64_t cooldown_timestamp, int64_t file_size) : tablet(tablet), cooldown_timestamp(cooldown_timestamp), file_size(file_size) {} TabletSharedPtr tablet; - int64_t cooldown_timestamp; + // to ensure the tablet with -1 would always be greater than other + uint64_t cooldown_timestamp; int64_t file_size; + bool operator<(const SortCtx& other) const { + if (this->cooldown_timestamp == other.cooldown_timestamp) { + return this->file_size > other.file_size; + } + return this->cooldown_timestamp < other.cooldown_timestamp; + } }; -void TabletManager::get_cooldown_tablets(std::vector<TabletSharedPtr>* tablets) { +void TabletManager::get_cooldown_tablets( + std::vector<TabletSharedPtr>* tablets, + std::function<bool(const TabletSharedPtr&)> cooldown_pending_predicate) { std::vector<SortCtx> sort_ctx_vec; + std::vector<std::weak_ptr<Tablet>> candidates; for (const auto& tablets_shard : _tablets_shards) { std::shared_lock rdlock(tablets_shard.lock); - for (const auto& item : tablets_shard.tablet_map) { - const TabletSharedPtr& tablet = item.second; - int64_t cooldown_timestamp = -1; - size_t file_size = -1; - if (tablet->need_cooldown(&cooldown_timestamp, &file_size)) { - sort_ctx_vec.emplace_back(tablet, cooldown_timestamp, file_size); - } - } - } - - std::sort(sort_ctx_vec.begin(), sort_ctx_vec.end(), [](SortCtx a, SortCtx b) { - if (a.cooldown_timestamp != -1 && b.cooldown_timestamp != -1) { - return a.cooldown_timestamp < b.cooldown_timestamp; - } - - if (a.cooldown_timestamp != -1 && b.cooldown_timestamp == -1) { - return true; - } - - if (a.cooldown_timestamp == -1 && b.cooldown_timestamp != -1) { - return false; - } - - return a.file_size > b.file_size; - }); + std::for_each( + tablets_shard.tablet_map.begin(), tablets_shard.tablet_map.end(), + [&candidates](auto& tablet_pair) { candidates.emplace_back(tablet_pair.second); }); + } + std::for_each(candidates.begin(), candidates.end(), + [&sort_ctx_vec, &cooldown_pending_predicate](std::weak_ptr<Tablet>& t) { + if (UNLIKELY(t.expired())) { + return; + } + const TabletSharedPtr& tablet = t.lock(); + std::shared_lock rdlock(tablet->get_header_lock()); + int64_t cooldown_timestamp = -1; + size_t file_size = -1; + if (cooldown_pending_predicate(tablet) || + tablet->need_cooldown(&cooldown_timestamp, &file_size)) { Review Comment: `||` seems wrong -- 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