freemandealer commented on code in PR #41357:
URL: https://github.com/apache/doris/pull/41357#discussion_r1776919615


##########
be/src/cloud/cloud_rowset_writer.cpp:
##########
@@ -34,7 +35,14 @@ Status CloudRowsetWriter::init(const RowsetWriterContext& 
rowset_writer_context)
     if (_context.is_local_rowset()) {
         // 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();

Review Comment:
   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.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to