[GitHub] [doris] pengxiangyu commented on a diff in pull request #14920: [enhancement](remote) support local cache GC at the granularity of cache files

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

pengxiangyu commented on code in PR #14920:
URL: https://github.com/apache/doris/pull/14920#discussion_r1042980176


##########
be/src/io/cache/file_cache_manager.cpp:
##########
@@ -175,16 +193,21 @@ void FileCacheManager::gc_file_caches() {
     // policy2: GC file cache by disk size
     if (gc_conf_size > 0) {
         for (size_t i = 0; i < contexts.size(); ++i) {
-            std::list<FileCachePtr> gc_file_list;
-            contexts[i].get_gc_file_caches(gc_file_list);
-            for (auto item : gc_file_list) {
-                std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
-                // for dummy file cache, check already used or not again
-                if (item->is_dummy_file_cache() &&
-                    _file_cache_map.find(item->cache_dir().native()) != _file_cache_map.end()) {
-                    continue;
+            auto context = contexts[i];

Review Comment:
   context = contexts[i];  is not needed, it will create a copy of contexts[i].



##########
be/src/io/cache/sub_file_cache.cpp:
##########
@@ -199,23 +208,29 @@ Status SubFileCache::_get_need_cache_offsets(size_t offset, size_t req_size,
 }
 
 Status SubFileCache::clean_timeout_cache() {
+    SubGcQueue gc_queue;
+    _gc_lru_queue.swap(gc_queue);
     std::vector<size_t> timeout_keys;
     {
         std::shared_lock<std::shared_mutex> rlock(_cache_map_lock);
         for (std::map<size_t, int64_t>::const_iterator iter = _last_match_times.cbegin();
              iter != _last_match_times.cend(); ++iter) {
             if (time(nullptr) - iter->second > _alive_time_sec) {
                 timeout_keys.emplace_back(iter->first);
+            } else {
+                auto [cache_file, done_file] = _cache_path(iter->first);
+                _gc_lru_queue.push({iter->first, iter->second});

Review Comment:
   don't delete it in _gc_lru_queue, because this file is in cache, maybe used when deleting.
   time(nullptr) - iter->second > _alive_time_sec will cover all cases,  _gc_lru_queue is not needed.



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