You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/08/22 06:37:24 UTC
[GitHub] [doris] pengxiangyu opened a new pull request, #11959: [feature](remote)Add cache files cleaner for remote olap files
pengxiangyu opened a new pull request, #11959:
URL: https://github.com/apache/doris/pull/11959
# Proposed changes
Issue Number: close #11958
## Problem summary
Add cache files cleaner for remote olap files
## Checklist(Required)
1. Does it affect the original behavior:
- [ ] Yes
- [ ] No
- [ ] I don't know
2. Has unit tests been added:
- [ ] Yes
- [ ] No
- [ ] No Need
3. Has document been added or modified:
- [ ] Yes
- [ ] No
- [ ] No Need
4. Does it need to update dependencies:
- [ ] Yes
- [ ] No
5. Are there any changes that cannot be rolled back:
- [ ] Yes (If Yes, please explain WHY)
- [ ] No
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #11959: [feature](remote)Add cache files cleaner for remote olap files
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11959:
URL: https://github.com/apache/doris/pull/11959#issuecomment-1225744523
PR approved by at least one committer and no changes requested.
--
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
[GitHub] [doris] github-actions[bot] commented on pull request #11959: [feature](remote)Add cache files cleaner for remote olap files
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #11959:
URL: https://github.com/apache/doris/pull/11959#issuecomment-1225744572
PR approved by anyone and no changes requested.
--
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
[GitHub] [doris] yangzhg commented on a diff in pull request #11959: [feature](remote)Add cache files cleaner for remote olap files
Posted by GitBox <gi...@apache.org>.
yangzhg commented on code in PR #11959:
URL: https://github.com/apache/doris/pull/11959#discussion_r952228075
##########
be/src/io/cache/file_cache_manager.cpp:
##########
@@ -29,18 +32,110 @@ void FileCacheManager::add_file_cache(const Path& cache_path, FileCachePtr file_
}
void FileCacheManager::remove_file_cache(const Path& cache_path) {
- std::lock_guard<std::shared_mutex> wrlock(_cache_map_lock);
- _file_cache_map.erase(cache_path.native());
+ bool cache_path_exist = false;
+ {
+ std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
+ if (_file_cache_map.find(cache_path) == _file_cache_map.end()) {
+ bool cache_dir_exist = false;
+ if (global_local_filesystem()->exists(cache_path, &cache_dir_exist).ok()) {
+ if (cache_dir_exist) {
+ Status st = global_local_filesystem()->delete_directory(cache_path);
+ if (!st.ok()) {
+ LOG(WARNING) << st.to_string();
+ }
+ }
+ }
+ } else {
+ cache_path_exist = true;
+ _file_cache_map.find(cache_path)->second->clean_all_cache();
+ }
+ }
+ if (cache_path_exist) {
+ std::lock_guard<std::shared_mutex> wrlock(_cache_map_lock);
+ _file_cache_map.erase(cache_path.native());
+ }
}
void FileCacheManager::clean_timeout_caches() {
std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
for (std::map<std::string, FileCachePtr>::const_iterator iter = _file_cache_map.cbegin();
iter != _file_cache_map.cend(); ++iter) {
+ if (iter->second == nullptr) {
+ continue;
+ }
iter->second->clean_timeout_cache();
}
}
+void FileCacheManager::clean_timeout_file_not_in_mem(const Path& cache_path) {
+ std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
+ // Deal with caches not in _file_cache_map
+ if (_file_cache_map.find(cache_path.native()) == _file_cache_map.end()) {
+ std::vector<Path> cache_file_names;
+ if (io::global_local_filesystem()->list(cache_path, &cache_file_names).ok()) {
+ std::map<std::string, bool> cache_names;
+ std::list<std::string> done_names;
+ for (Path cache_file_name : cache_file_names) {
+ std::string filename = cache_file_name.native();
+ if (filename.find("_DONE") == std::string::npos) {
+ cache_names[filename] = true;
+ continue;
+ }
+ done_names.push_back(filename);
+ std::string done_file_path = (cache_path / filename).native();
+ time_t m_time;
+ if (!FileUtils::mtime(done_file_path, &m_time).ok()) {
+ continue;
+ }
+ if (time(nullptr) - m_time < config::file_cache_alive_time_sec) {
Review Comment:
mv time(nullptr) out of for block
--
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
[GitHub] [doris] yangzhg commented on a diff in pull request #11959: [feature](remote)Add cache files cleaner for remote olap files
Posted by GitBox <gi...@apache.org>.
yangzhg commented on code in PR #11959:
URL: https://github.com/apache/doris/pull/11959#discussion_r952227370
##########
be/src/io/cache/file_cache_manager.cpp:
##########
@@ -29,18 +32,110 @@ void FileCacheManager::add_file_cache(const Path& cache_path, FileCachePtr file_
}
void FileCacheManager::remove_file_cache(const Path& cache_path) {
- std::lock_guard<std::shared_mutex> wrlock(_cache_map_lock);
- _file_cache_map.erase(cache_path.native());
+ bool cache_path_exist = false;
+ {
+ std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
+ if (_file_cache_map.find(cache_path) == _file_cache_map.end()) {
+ bool cache_dir_exist = false;
+ if (global_local_filesystem()->exists(cache_path, &cache_dir_exist).ok()) {
+ if (cache_dir_exist) {
+ Status st = global_local_filesystem()->delete_directory(cache_path);
+ if (!st.ok()) {
+ LOG(WARNING) << st.to_string();
+ }
+ }
+ }
+ } else {
+ cache_path_exist = true;
+ _file_cache_map.find(cache_path)->second->clean_all_cache();
+ }
+ }
+ if (cache_path_exist) {
+ std::lock_guard<std::shared_mutex> wrlock(_cache_map_lock);
+ _file_cache_map.erase(cache_path.native());
+ }
}
void FileCacheManager::clean_timeout_caches() {
std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
for (std::map<std::string, FileCachePtr>::const_iterator iter = _file_cache_map.cbegin();
iter != _file_cache_map.cend(); ++iter) {
+ if (iter->second == nullptr) {
+ continue;
+ }
iter->second->clean_timeout_cache();
}
}
+void FileCacheManager::clean_timeout_file_not_in_mem(const Path& cache_path) {
+ std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
+ // Deal with caches not in _file_cache_map
+ if (_file_cache_map.find(cache_path.native()) == _file_cache_map.end()) {
+ std::vector<Path> cache_file_names;
+ if (io::global_local_filesystem()->list(cache_path, &cache_file_names).ok()) {
+ std::map<std::string, bool> cache_names;
+ std::list<std::string> done_names;
+ for (Path cache_file_name : cache_file_names) {
+ std::string filename = cache_file_name.native();
+ if (filename.find("_DONE") == std::string::npos) {
Review Comment:
how about a file named `/path_DONE/path/to/a/file` ?
--
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
[GitHub] [doris] HappenLee merged pull request #11959: [feature](remote)Add cache files cleaner for remote olap files
Posted by GitBox <gi...@apache.org>.
HappenLee merged PR #11959:
URL: https://github.com/apache/doris/pull/11959
--
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
[GitHub] [doris] zxealous commented on a diff in pull request #11959: [feature](remote)Add cache files cleaner for remote olap files
Posted by GitBox <gi...@apache.org>.
zxealous commented on code in PR #11959:
URL: https://github.com/apache/doris/pull/11959#discussion_r953629420
##########
docs/zh-CN/docs/admin-manual/config/be-config.md:
##########
@@ -1600,3 +1600,22 @@ webserver默认工作线程数
* 描述: 最少进行合并的版本数，当选中的小数据量的rowset个数，大于这个值是才会进行真正的合并
* 默认值: 10
+### `generate_cache_cleaner_task_interval_sec`
+* 类型：int64
+* 描述：缓存文件的清理间隔，单位：秒
+* 默认值：43200（12小时）
+
+### `file_cache_type`
+* 类型：string
+* 描述：缓存文件的类型。whole_file_cache：将segment文件整个下载，sub_file_cache：将segment文件按大小切分成多个文件。
+* 默认值：104857600（100MB）
Review Comment:
default value
##########
docs/en/docs/admin-manual/config/be-config.md:
##########
@@ -1576,3 +1576,22 @@ Translated with www.DeepL.com/Translator (free version)
* Description: at least the number of versions to be compaction, and the number of rowsets with a small amount of data in the selection. If it is greater than this value, the real compaction will be carried out
* Default: 10
+### `generate_cache_cleaner_task_interval_sec`
+* Type：int64
+* Description：Cleaning interval of cache files, in seconds
+* Default：43200（12 hours）
+
+### `file_cache_type`
+* Type：string
+* Description：Type of cache file. whole_ file_ Cache: download the entire segment file, sub_ file_ Cache: the segment file is divided into multiple files by size.
+* Default：104857600（100MB）
Review Comment:
the default value is an empty string
--
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
[GitHub] [doris] yangzhg commented on a diff in pull request #11959: [feature](remote)Add cache files cleaner for remote olap files
Posted by GitBox <gi...@apache.org>.
yangzhg commented on code in PR #11959:
URL: https://github.com/apache/doris/pull/11959#discussion_r952230504
##########
be/src/util/file_utils.cpp:
##########
@@ -211,6 +211,22 @@ Status FileUtils::md5sum(const std::string& file, std::string* md5sum) {
return Status::OK();
}
+Status FileUtils::mtime(const std::string& file, time_t* m_time) {
+ int fd = open(file.c_str(), O_RDONLY);
Review Comment:
use a defer to protect fd maybe mor simple
--
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
[GitHub] [doris] yangzhg commented on a diff in pull request #11959: [feature](remote)Add cache files cleaner for remote olap files
Posted by GitBox <gi...@apache.org>.
yangzhg commented on code in PR #11959:
URL: https://github.com/apache/doris/pull/11959#discussion_r952225944
##########
be/src/io/cache/file_cache_manager.cpp:
##########
@@ -29,18 +32,110 @@ void FileCacheManager::add_file_cache(const Path& cache_path, FileCachePtr file_
}
void FileCacheManager::remove_file_cache(const Path& cache_path) {
- std::lock_guard<std::shared_mutex> wrlock(_cache_map_lock);
- _file_cache_map.erase(cache_path.native());
+ bool cache_path_exist = false;
+ {
+ std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
+ if (_file_cache_map.find(cache_path) == _file_cache_map.end()) {
+ bool cache_dir_exist = false;
+ if (global_local_filesystem()->exists(cache_path, &cache_dir_exist).ok()) {
+ if (cache_dir_exist) {
+ Status st = global_local_filesystem()->delete_directory(cache_path);
+ if (!st.ok()) {
+ LOG(WARNING) << st.to_string();
+ }
+ }
+ }
+ } else {
+ cache_path_exist = true;
+ _file_cache_map.find(cache_path)->second->clean_all_cache();
+ }
+ }
+ if (cache_path_exist) {
+ std::lock_guard<std::shared_mutex> wrlock(_cache_map_lock);
+ _file_cache_map.erase(cache_path.native());
+ }
}
void FileCacheManager::clean_timeout_caches() {
std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
for (std::map<std::string, FileCachePtr>::const_iterator iter = _file_cache_map.cbegin();
iter != _file_cache_map.cend(); ++iter) {
+ if (iter->second == nullptr) {
+ continue;
+ }
iter->second->clean_timeout_cache();
}
}
+void FileCacheManager::clean_timeout_file_not_in_mem(const Path& cache_path) {
+ std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
Review Comment:
using `std::string_view` and `emplace_back` maybe better
--
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
[GitHub] [doris] yangzhg commented on a diff in pull request #11959: [feature](remote)Add cache files cleaner for remote olap files
Posted by GitBox <gi...@apache.org>.
yangzhg commented on code in PR #11959:
URL: https://github.com/apache/doris/pull/11959#discussion_r952221072
##########
be/src/io/cache/file_cache_manager.cpp:
##########
@@ -29,18 +32,110 @@ void FileCacheManager::add_file_cache(const Path& cache_path, FileCachePtr file_
}
void FileCacheManager::remove_file_cache(const Path& cache_path) {
- std::lock_guard<std::shared_mutex> wrlock(_cache_map_lock);
- _file_cache_map.erase(cache_path.native());
+ bool cache_path_exist = false;
+ {
+ std::shared_lock<std::shared_mutex> rdlock(_cache_map_lock);
+ if (_file_cache_map.find(cache_path) == _file_cache_map.end()) {
+ bool cache_dir_exist = false;
+ if (global_local_filesystem()->exists(cache_path, &cache_dir_exist).ok()) {
+ if (cache_dir_exist) {
Review Comment:
redundant if stmt and var `cache_dir_exist`
--
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