[GitHub] [doris] cambyzju commented on a diff in pull request #13865: [feature](remote)Only query can use local cache when reading remote files.

[GitBox]

cambyzju commented on code in PR #13865:
URL: https://github.com/apache/doris/pull/13865#discussion_r1011141592


##########
be/src/olap/segment_loader.cpp:
##########
@@ -35,45 +36,52 @@ SegmentLoader::SegmentLoader(size_t capacity) {
     _cache = std::unique_ptr<Cache>(new_lru_cache("SegmentCache", capacity, 
LRUCacheType::NUMBER));
 }
 
-bool SegmentLoader::_lookup(const SegmentLoader::CacheKey& key, 
SegmentCacheHandle* handle) {
-    auto lru_handle = _cache->lookup(key.encode());
+bool SegmentLoader::_lookup(const std::string& key, SegmentCacheHandle* 
handle) {
+    auto lru_handle = _cache->lookup(key);
     if (lru_handle == nullptr) {
         return false;
     }
     *handle = SegmentCacheHandle(_cache.get(), lru_handle);
     return true;
 }
 
-void SegmentLoader::_insert(const SegmentLoader::CacheKey& key, 
SegmentLoader::CacheValue& value,
+void SegmentLoader::_insert(const std::string& key, SegmentLoader::CacheValue& 
value,
                             SegmentCacheHandle* handle) {
     auto deleter = [](const doris::CacheKey& key, void* value) {
         SegmentLoader::CacheValue* cache_value = 
(SegmentLoader::CacheValue*)value;
         cache_value->segments.clear();
         delete cache_value;
     };
 
-    auto lru_handle = _cache->insert(key.encode(), &value, 
sizeof(SegmentLoader::CacheValue),
-                                     deleter, CachePriority::NORMAL);
+    auto lru_handle = _cache->insert(key, &value, 
sizeof(SegmentLoader::CacheValue), deleter,
+                                     CachePriority::NORMAL);
     *handle = SegmentCacheHandle(_cache.get(), lru_handle);
 }
 
 Status SegmentLoader::load_segments(const BetaRowsetSharedPtr& rowset,
-                                    SegmentCacheHandle* cache_handle, bool 
use_cache) {
+                                    SegmentCacheHandle* cache_handle, bool 
use_cache,
+                                    bool use_local_file_cache) {
     SegmentLoader::CacheKey cache_key(rowset->rowset_id());
-    if (_lookup(cache_key, cache_handle)) {
+    std::string key = "";
+    if (use_local_file_cache) {
+        key = fmt::format("{}{}", LOCAL_FILE_CACHE_KEY_PREFIX, 
cache_key.encode());
+    } else {
+        key = cache_key.encode();
+    }
+    if (_lookup(key, cache_handle)) {
         cache_handle->owned = false;
         return Status::OK();
     }
     cache_handle->owned = !use_cache;
 
     std::vector<segment_v2::SegmentSharedPtr> segments;
-    RETURN_NOT_OK(rowset->load_segments(&segments));
+    RETURN_NOT_OK(rowset->load_segments(use_local_file_cache, &segments));
 
     if (use_cache) {
         // memory of SegmentLoader::CacheValue will be handled by SegmentLoader
         SegmentLoader::CacheValue* cache_value = new 
SegmentLoader::CacheValue();
         cache_value->segments = std::move(segments);
-        _insert(cache_key, *cache_value, cache_handle);
+        _insert(key, *cache_value, cache_handle);

Review Comment:
   here maybe two segments cache key, one cache key is "${rowset_id}", the 
other cache key is "local_file_cache_${rowset_id}", and both of them point to 
the same segments?
   
   Is there any problem? for example, open twice or close twice, or some other 
cases?



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