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


##########
be/src/io/cache/whole_file_cache.h:
##########
@@ -48,15 +48,17 @@ class WholeFileCache final : public FileCache {
 
     io::FileReaderSPtr remote_file_reader() const override { return 
_remote_file_reader; }
 
-    Status clean_timeout_cache() override;
+    Status clean_timeout_cache();
+
+    Status clean_cache_normal() override;
 
     Status clean_all_cache() override;
 
     Status clean_one_cache(size_t* cleaned_size) override;
 
-    int64_t get_oldest_match_time() const override { return _last_match_time; 
};
+    int64_t get_oldest_match_time() const override { return _gc_match_time; };

Review Comment:
   结尾多了一个分号



##########
be/src/io/cache/file_cache.cpp:
##########
@@ -87,32 +89,87 @@ Status FileCache::download_cache_to_local(const Path& 
cache_file, const Path& ca
     return Status::OK();
 }
 
-Status FileCache::_remove_file(const Path& cache_file, const Path& 
cache_done_file,
-                               size_t* cleaned_size) {
-    bool done_file_exist = false;
-    RETURN_NOT_OK_STATUS_WITH_WARN(
-            io::global_local_filesystem()->exists(cache_done_file, 
&done_file_exist),
-            "Check local done file exist failed.");
-    if (done_file_exist) {
-        RETURN_NOT_OK_STATUS_WITH_WARN(
-                io::global_local_filesystem()->delete_file(cache_done_file),
-                fmt::format("Delete local done file failed: {}", 
cache_done_file.native()));
-    }
+Status FileCache::_remove_file(const Path& file, size_t* cleaned_size) {
     bool cache_file_exist = false;
-    RETURN_NOT_OK_STATUS_WITH_WARN(
-            io::global_local_filesystem()->exists(cache_file, 
&cache_file_exist),
-            "Check local cache file exist failed.");
+    RETURN_NOT_OK_STATUS_WITH_WARN(io::global_local_filesystem()->exists(file, 
&cache_file_exist),
+                                   "Check local cache file exist failed.");
     if (cache_file_exist) {
         if (cleaned_size) {
             RETURN_NOT_OK_STATUS_WITH_WARN(
-                    io::global_local_filesystem()->file_size(cache_file, 
cleaned_size),
-                    fmt::format("get local cache file size failed: {}", 
cache_file.native()));
+                    io::global_local_filesystem()->file_size(file, 
cleaned_size),
+                    fmt::format("get local cache file size failed: {}", 
file.native()));
         }
         RETURN_NOT_OK_STATUS_WITH_WARN(
-                io::global_local_filesystem()->delete_file(cache_file),
-                fmt::format("Delete local cache file failed: {}", 
cache_file.native()));
+                io::global_local_filesystem()->delete_file(file),
+                fmt::format("Delete local cache file failed: {}", 
file.native()));
+    }
+    LOG(INFO) << "Delete local cache file successfully: " << file.native();
+    return Status::OK();
+}
+
+Status FileCache::_remove_cache_and_done(const Path& cache_file, const Path& 
cache_done_file,
+                                         size_t* cleaned_size) {
+    RETURN_IF_ERROR(_remove_file(cache_done_file, nullptr));
+    RETURN_IF_ERROR(_remove_file(cache_file, cleaned_size));
+    return Status::OK();
+}
+
+Status FileCache::_get_dir_file(const Path& cache_dir, std::vector<Path>& 
cache_names,

Review Comment:
   cache_names作为返回值,最好使用指针



##########
be/src/io/cache/file_cache.cpp:
##########
@@ -87,32 +89,87 @@ Status FileCache::download_cache_to_local(const Path& 
cache_file, const Path& ca
     return Status::OK();
 }
 
-Status FileCache::_remove_file(const Path& cache_file, const Path& 
cache_done_file,
-                               size_t* cleaned_size) {
-    bool done_file_exist = false;
-    RETURN_NOT_OK_STATUS_WITH_WARN(
-            io::global_local_filesystem()->exists(cache_done_file, 
&done_file_exist),
-            "Check local done file exist failed.");
-    if (done_file_exist) {
-        RETURN_NOT_OK_STATUS_WITH_WARN(
-                io::global_local_filesystem()->delete_file(cache_done_file),
-                fmt::format("Delete local done file failed: {}", 
cache_done_file.native()));
-    }
+Status FileCache::_remove_file(const Path& file, size_t* cleaned_size) {
     bool cache_file_exist = false;
-    RETURN_NOT_OK_STATUS_WITH_WARN(
-            io::global_local_filesystem()->exists(cache_file, 
&cache_file_exist),
-            "Check local cache file exist failed.");
+    RETURN_NOT_OK_STATUS_WITH_WARN(io::global_local_filesystem()->exists(file, 
&cache_file_exist),
+                                   "Check local cache file exist failed.");
     if (cache_file_exist) {
         if (cleaned_size) {
             RETURN_NOT_OK_STATUS_WITH_WARN(
-                    io::global_local_filesystem()->file_size(cache_file, 
cleaned_size),
-                    fmt::format("get local cache file size failed: {}", 
cache_file.native()));
+                    io::global_local_filesystem()->file_size(file, 
cleaned_size),
+                    fmt::format("get local cache file size failed: {}", 
file.native()));
         }
         RETURN_NOT_OK_STATUS_WITH_WARN(
-                io::global_local_filesystem()->delete_file(cache_file),
-                fmt::format("Delete local cache file failed: {}", 
cache_file.native()));
+                io::global_local_filesystem()->delete_file(file),
+                fmt::format("Delete local cache file failed: {}", 
file.native()));
+    }
+    LOG(INFO) << "Delete local cache file successfully: " << file.native();
+    return Status::OK();
+}
+
+Status FileCache::_remove_cache_and_done(const Path& cache_file, const Path& 
cache_done_file,
+                                         size_t* cleaned_size) {
+    RETURN_IF_ERROR(_remove_file(cache_done_file, nullptr));
+    RETURN_IF_ERROR(_remove_file(cache_file, cleaned_size));
+    return Status::OK();
+}
+
+Status FileCache::_get_dir_file(const Path& cache_dir, std::vector<Path>& 
cache_names,
+                                std::vector<Path>& unfinished_files) {
+    // list all files
+    std::vector<Path> cache_file_names;
+    RETURN_NOT_OK_STATUS_WITH_WARN(
+            io::global_local_filesystem()->list(cache_dir, &cache_file_names),
+            fmt::format("List dir failed: {}", cache_dir.native()))
+
+    // separate DATA file and DONE file
+    std::set<Path> cache_names_temp;
+    std::list<Path> done_names_temp;
+    for (auto& cache_file_name : cache_file_names) {
+        if (ends_with(cache_file_name.native(), CACHE_DONE_FILE_SUFFIX)) {
+            done_names_temp.push_back(std::move(cache_file_name));
+        } else {
+            cache_names_temp.insert(std::move(cache_file_name));
+        }
+    }
+
+    // match DONE file with DATA file
+    for (auto done_file : done_names_temp) {
+        Path cache_filename = StringReplace(done_file.native(), 
CACHE_DONE_FILE_SUFFIX, "", true);
+        if (auto cache_iter = cache_names_temp.find(cache_filename);
+            cache_iter != cache_names_temp.end()) {
+            cache_names_temp.erase(cache_iter);
+            cache_names.push_back(std::move(cache_filename));
+        } else {
+            // not data file, but with DONE file
+            unfinished_files.push_back(std::move(done_file));
+        }
+    }
+    // data file without DONE file
+    for (auto& file : cache_names_temp) {

Review Comment:
   
cache_name存在,done_name不存在的情况,有可能是正在下载文件过程中,还没下载完,这种情况应该检查其是不是在FileCache里,如果是的话,不应该用gc来管理。



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