-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[chore](file-cache) Enable file cache for cloud mode by force #41357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[chore](file-cache) Enable file cache for cloud mode by force #41357
Conversation
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
|
run buildall |
dataroaring
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
c303196 to
5f68ebe
Compare
|
run buildall |
TPC-H: Total hot run time: 40821 ms |
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 191680 ms |
ClickBench: Total hot run time: 33.22 s |
be/src/cloud/cloud_rowset_writer.cpp
Outdated
| // In cloud mode, this branch implies it is an intermediate rowset for external merge sort, | ||
| // we use `global_local_filesystem` to write data to `tmp_file_dir`(see `local_segment_path`). | ||
| _context.tablet_path = io::FileCacheFactory::instance()->get_cache_path(); | ||
| if (auto maybe_cache_path = io::FileCacheFactory::instance()->get_cache_path(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so cache path will be used even if enable_file_cache = false ? This will increase user's cognitive load somehow. A new path should be used in my opinion.
FYI, with more Cache Storage being added to Doris, disk file system is not the only place for cached data, e.g. system RAM for Memory-based Cache Storage. In this case, cache path will have value but not a directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We enable file cache by false after duscussion. As for the availability of cache path, currently every cache has a working disk path. If mem cache is introduced, we should modify the implementation of FileCacheFactory then.
6cd5504 to
4887f68
Compare
|
run buildall |
|
TeamCity be ut coverage result: |
freemandealer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dataroaring
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
PR approved by at least one committer and no changes requested. |
4887f68 to
8f160b7
Compare
|
run buildall |
TPC-H: Total hot run time: 40906 ms |
TPC-DS: Total hot run time: 192204 ms |
|
TeamCity be ut coverage result: |
ClickBench: Total hot run time: 32.46 s |
8f160b7 to
cb4840c
Compare
|
run buildall |
|
TeamCity be ut coverage result: |
cb4840c to
fda89e2
Compare
|
run buildall |
TPC-H: Total hot run time: 40878 ms |
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 192183 ms |
ClickBench: Total hot run time: 32.81 s |
|
run external |
|
PR approved by at least one committer and no changes requested. |
…#41357) ## Proposed changes Temp local rowset writer for external sorting replies on file cache.
…#41357) ## Proposed changes Temp local rowset writer for external sorting replies on file cache.
Proposed changes
Temp local rowset writer for external sorting replies on file cache.