response to the reviewer

Signed-off-by: zhengyu <[email protected]>

Author: freemandealer

be/src/cloud/cloud_storage_engine.cpp

@@ -280,8 +280,9 @@ bool CloudStorageEngine::stopped() {
 
 Result<BaseTabletSPtr> CloudStorageEngine::get_tablet(int64_t tablet_id,
                                                       SyncRowsetStats* sync_stats,
-                                                      bool force_use_cache) {
-    return _tablet_mgr->get_tablet(tablet_id, false, true, sync_stats, force_use_cache)
+                                                      bool force_use_cache, bool cache_on_miss) {
+    return _tablet_mgr
+            ->get_tablet(tablet_id, false, true, sync_stats, force_use_cache, cache_on_miss)
             .transform([](auto&& t) { return static_pointer_cast<BaseTablet>(std::move(t)); });
 }
 

be/src/cloud/cloud_storage_engine.h

@@ -63,7 +63,8 @@ class CloudStorageEngine final : public BaseStorageEngine {
     bool stopped() override;
 
     Result<BaseTabletSPtr> get_tablet(int64_t tablet_id, SyncRowsetStats* sync_stats = nullptr,
-                                      bool force_use_cache = false) override;
+                                      bool force_use_cache = false,
+                                      bool cache_on_miss = true) override;
 
     Status start_bg_threads(std::shared_ptr<WorkloadGroup> wg_sptr = nullptr) override;
 

be/src/cloud/cloud_tablet_mgr.cpp

@@ -161,7 +161,8 @@ void set_tablet_access_time_ms(CloudTablet* tablet) {
 Result<std::shared_ptr<CloudTablet>> CloudTabletMgr::get_tablet(int64_t tablet_id, bool warmup_data,
                                                                 bool sync_delete_bitmap,
                                                                 SyncRowsetStats* sync_stats,
-                                                                bool local_only) {
+                                                                bool local_only,
+                                                                bool cache_on_miss) {
     // LRU value type. `Value`'s lifetime MUST NOT be longer than `CloudTabletMgr`
     class Value : public LRUCacheValueBase {
     public:
@@ -195,7 +196,7 @@ Result<std::shared_ptr<CloudTablet>> CloudTabletMgr::get_tablet(int64_t tablet_i
         if (sync_stats) {
             ++sync_stats->tablet_meta_cache_miss;
         }
-        auto load_tablet = [this, &key, warmup_data, sync_delete_bitmap,
+        auto load_tablet = [this, warmup_data, sync_delete_bitmap,
                             sync_stats](int64_t tablet_id) -> Result<std::shared_ptr<CloudTablet>> {
             TabletMetaSharedPtr tablet_meta;
             auto start = std::chrono::steady_clock::now();
@@ -211,7 +212,6 @@ Result<std::shared_ptr<CloudTablet>> CloudTabletMgr::get_tablet(int64_t tablet_i
             }
 
             auto tablet = std::make_shared<CloudTablet>(_engine, std::move(tablet_meta));
-            auto value = std::make_unique<Value>(tablet, *_tablet_map);
             // MUST sync stats to let compaction scheduler work correctly
             SyncOptions options;
             options.warmup_delta_data = warmup_data;
@@ -221,16 +221,7 @@ Result<std::shared_ptr<CloudTablet>> CloudTabletMgr::get_tablet(int64_t tablet_i
                 LOG(WARNING) << "failed to sync tablet " << tablet_id << ": " << st;
                 return ResultError(st);
             }
-
-            auto* handle = _cache->insert(key, value.release(), 1, sizeof(CloudTablet),
-                                          CachePriority::NORMAL);
-            auto ret =
-                    std::shared_ptr<CloudTablet>(tablet.get(), [this, handle](CloudTablet* tablet) {
-                        set_tablet_access_time_ms(tablet);
-                        _cache->release(handle);
-                    });
-            _tablet_map->put(std::move(tablet));
-            return ret;
+            return tablet;
         };
 
         auto load_result = s_singleflight_load_tablet.load(tablet_id, std::move(load_tablet));
@@ -239,8 +230,22 @@ Result<std::shared_ptr<CloudTablet>> CloudTabletMgr::get_tablet(int64_t tablet_i
                                                      load_result.error()));
         }
         auto tablet = load_result.value();
-        set_tablet_access_time_ms(tablet.get());
-        return tablet;
+        if (!cache_on_miss) {
+            set_tablet_access_time_ms(tablet.get());
+            return tablet;
+        }
+
+        auto value = std::make_unique<Value>(tablet, *_tablet_map);
+        auto* insert_handle =
+                _cache->insert(key, value.release(), 1, sizeof(CloudTablet), CachePriority::NORMAL);
+        auto ret = std::shared_ptr<CloudTablet>(tablet.get(),
+                                                [this, insert_handle](CloudTablet* tablet_ptr) {
+                                                    set_tablet_access_time_ms(tablet_ptr);
+                                                    _cache->release(insert_handle);
+                                                });
+        _tablet_map->put(std::move(tablet));
+        set_tablet_access_time_ms(ret.get());
+        return ret;
     }
     if (sync_stats) {
         ++sync_stats->tablet_meta_cache_hit;

be/src/cloud/cloud_tablet_mgr.h

@@ -47,7 +47,8 @@ class CloudTabletMgr {
     Result<std::shared_ptr<CloudTablet>> get_tablet(int64_t tablet_id, bool warmup_data = false,
                                                     bool sync_delete_bitmap = true,
                                                     SyncRowsetStats* sync_stats = nullptr,
-                                                    bool local_only = false);
+                                                    bool local_only = false,
+                                                    bool cache_on_miss = true);
 
     void erase_tablet(int64_t tablet_id);
 

be/src/common/config.cpp

@@ -1161,7 +1161,8 @@ DEFINE_mInt64(file_cache_background_block_lru_update_interval_ms, "5000");
 DEFINE_mInt64(file_cache_background_block_lru_update_qps_limit, "1000");
 DEFINE_mBool(enable_reader_dryrun_when_download_file_cache, "true");
 DEFINE_mInt64(file_cache_background_monitor_interval_ms, "5000");
-DEFINE_mInt64(file_cache_background_ttl_gc_interval_ms, "3000");
+DEFINE_mInt64(file_cache_background_ttl_gc_interval_ms, "180000");
+DEFINE_mInt64(file_cache_background_ttl_info_update_interval_ms, "180000");
 DEFINE_mInt64(file_cache_background_ttl_gc_batch, "1000");
 DEFINE_mInt64(file_cache_background_lru_dump_interval_ms, "60000");
 // dump queue only if the queue update specific times through several dump intervals

be/src/common/config.h

@@ -1183,6 +1183,7 @@ DECLARE_mInt64(file_cache_background_block_lru_update_qps_limit);
 DECLARE_mBool(enable_reader_dryrun_when_download_file_cache);
 DECLARE_mInt64(file_cache_background_monitor_interval_ms);
 DECLARE_mInt64(file_cache_background_ttl_gc_interval_ms);
+DECLARE_mInt64(file_cache_background_ttl_info_update_interval_ms);
 DECLARE_mInt64(file_cache_background_ttl_gc_batch);
 DECLARE_Int32(file_cache_downloader_thread_num_min);
 DECLARE_Int32(file_cache_downloader_thread_num_max);

be/src/io/cache/block_file_cache.cpp

@@ -285,7 +285,6 @@ BlockFileCache::BlockFileCache(const std::string& cache_base_path,
 
     _lru_recorder = std::make_unique<LRUQueueRecorder>(this);
     _lru_dumper = std::make_unique<CacheLRUDumper>(this, _lru_recorder.get());
-    _ttl_mgr = std::make_unique<BlockFileCacheTtlMgr>(this, _meta_store.get());
     if (cache_settings.storage == "memory") {
         _storage = std::make_unique<MemFileCacheStorage>();
         _cache_base_path = "memory";
@@ -384,6 +383,13 @@ Status BlockFileCache::initialize_unlocked(std::lock_guard<std::mutex>& cache_lo
         restore_lru_queues_from_disk(cache_lock);
     }
     RETURN_IF_ERROR(_storage->init(this));
+    if (!_ttl_mgr) {
+        if (auto* fs_storage = dynamic_cast<FSFileCacheStorage*>(_storage.get())) {
+            if (auto* meta_store = fs_storage->get_meta_store()) {
+                _ttl_mgr = std::make_unique<BlockFileCacheTtlMgr>(this, meta_store);
+            }
+        }
+    }
     _cache_background_monitor_thread = std::thread(&BlockFileCache::run_background_monitor, this);
     _cache_background_gc_thread = std::thread(&BlockFileCache::run_background_gc, this);
     _cache_background_evict_in_advance_thread =
@@ -641,7 +647,9 @@ FileBlocks BlockFileCache::split_range_into_cells(const UInt128Wrapper& hash,
                     cell->update_atime();
                 }
             }
-            _ttl_mgr->register_tablet_id(context.tablet_id);
+            if (_ttl_mgr && context.tablet_id != 0) {
+                _ttl_mgr->register_tablet_id(context.tablet_id);
+            }
         }
 
         current_pos += current_size;
@@ -2140,6 +2148,21 @@ std::map<size_t, FileBlockSPtr> BlockFileCache::get_blocks_by_key(const UInt128W
     return offset_to_block;
 }
 
+void BlockFileCache::modify_expiration_time(const UInt128Wrapper& hash, uint64_t expiration_time) {
+    SCOPED_CACHE_LOCK(_mutex, this);
+    if (auto iter = _files.find(hash); iter != _files.end()) {
+        for (auto& [_, cell] : iter->second) {
+            if (cell.file_block) {
+                auto st = cell.file_block->update_expiration_time(expiration_time);
+                if (!st.ok()) {
+                    LOG(WARNING) << "Failed to update expiration time for block "
+                                 << cell.file_block->get_info_for_log() << ", error=" << st;
+                }
+            }
+        }
+    }
+}
+
 void BlockFileCache::update_ttl_atime(const UInt128Wrapper& hash) {
     SCOPED_CACHE_LOCK(_mutex, this);
     if (auto iter = _files.find(hash); iter != _files.end()) {

be/src/io/cache/block_file_cache.h

@@ -27,6 +27,7 @@
 #include <optional>
 #include <thread>
 
+#include "io/cache/block_file_cache_ttl_mgr.h"
 #include "io/cache/cache_lru_dumper.h"
 #include "io/cache/file_block.h"
 #include "io/cache/file_cache_common.h"
@@ -72,7 +73,6 @@ class LockScopedTimer {
     LockScopedTimer cache_lock_timer;
 
 class FSFileCacheStorage;
-class BlockFileCacheTtlMgr;
 
 // The BlockFileCache is responsible for the management of the blocks
 // The current strategies are lru and ttl.
@@ -207,6 +207,8 @@ class BlockFileCache {
     std::string reset_capacity(size_t new_capacity);
 
     std::map<size_t, FileBlockSPtr> get_blocks_by_key(const UInt128Wrapper& hash);
+    /// Adjust expiration time for every block sharing the specified hash key.
+    void modify_expiration_time(const UInt128Wrapper& hash, uint64_t expiration_time);
     /// For debug and UT
     std::string dump_structure(const UInt128Wrapper& hash);
     std::string dump_single_cache_type(const UInt128Wrapper& hash, size_t offset);

be/src/io/cache/block_file_cache_ttl_mgr.cpp

@@ -29,6 +29,8 @@
 #include "io/cache/block_file_cache.h"
 #include "io/cache/cache_block_meta_store.h"
 #include "io/cache/file_block.h"
+#include "olap/base_tablet.h"
+#include "runtime/exec_env.h"
 #include "util/time.h"
 
 namespace doris::io {
@@ -71,7 +73,6 @@ FileBlocks BlockFileCacheTtlMgr::get_file_blocks_from_tablet_id(int64_t tablet_i
 
     while (iterator->valid()) {
         BlockMetaKey key = iterator->key();
-        BlockMeta meta = iterator->value();
 
         // Get all blocks for this hash using get_blocks_by_key
         try {
@@ -108,32 +109,45 @@ void BlockFileCacheTtlMgr::run_backgroud_update_ttl_info_map() {
             }
 
             for (int64_t tablet_id : tablet_ids_to_process) {
-                // TODO(zhengyu): Implement actual CloudMetaMgr or CloudTabletMgr integration
-                // For now, we'll use placeholder values
-                uint64_t create_time = 0;
+                uint64_t tablet_ctime = 0;
                 uint64_t ttl = 0;
 
-                // Simulate getting tablet metadata
-                // This should be replaced with actual CloudMetaMgr::get_tablet_meta call
-                bool has_ttl = (tablet_id % 10 == 0); // Example condition
-                if (has_ttl) {
-                    create_time = UnixSeconds(); // Current time as create time
-                    ttl = 3600;                  // 1 hour TTL
+                auto res = ExecEnv::get_tablet(tablet_id, nullptr, false, false);
+                if (!res.has_value()) {
+                    LOG(WARNING) << "Failed to get tablet for tablet_id: " << tablet_id
+                                 << ", err: " << res.error();
+                    continue;
+                }
+
+                auto tablet = res.value();
+                const auto& tablet_meta = tablet->tablet_meta();
+                if (tablet_meta != nullptr) {
+                    tablet_ctime = tablet_meta->creation_time();
+                }
+
+                int64_t ttl_seconds = tablet->ttl_seconds();
+                if (ttl_seconds > 0 && tablet_ctime > 0) {
+                    ttl = static_cast<uint64_t>(ttl_seconds);
                 }
 
                 // Update TTL info map
                 {
                     std::lock_guard<std::mutex> lock(_ttl_info_mutex);
                     if (ttl > 0) {
-                        _ttl_info_map[tablet_id] = TtlInfo {ttl, create_time};
+                        auto old_info_it = _ttl_info_map.find(tablet_id);
+                        bool was_zero_ttl = (old_info_it == _ttl_info_map.end() ||
+                                             old_info_it->second.ttl == 0);
+                        _ttl_info_map[tablet_id] = TtlInfo {ttl, tablet_ctime};
 
                         // If TTL changed from 0 to non-zero, convert blocks to TTL type
-                        auto old_info_it = _ttl_info_map.find(tablet_id);
-                        if (old_info_it == _ttl_info_map.end() || old_info_it->second.ttl == 0) {
+                        if (was_zero_ttl) {
                             FileBlocks blocks = get_file_blocks_from_tablet_id(tablet_id);
                             for (auto& block : blocks) {
                                 if (block->cache_type() != FileCacheType::TTL) {
-                                    block->change_cache_type(FileCacheType::TTL);
+                                    auto st = block->change_cache_type(FileCacheType::TTL);
+                                    if (!st.ok()) {
+                                        LOG(WARNING) << "Failed to convert block to TTL cache_type";
+                                    }
                                 }
                             }
                         }
@@ -144,9 +158,8 @@ void BlockFileCacheTtlMgr::run_backgroud_update_ttl_info_map() {
                 }
             }
 
-            // Sleep for configured interval (use existing TTL GC interval)
-            std::this_thread::sleep_for(
-                    std::chrono::milliseconds(config::file_cache_background_ttl_gc_interval_ms));
+            std::this_thread::sleep_for(std::chrono::milliseconds(
+                    config::file_cache_background_ttl_info_update_interval_ms));
 
         } catch (const std::exception& e) {
             LOG(WARNING) << "Exception in TTL update thread: " << e.what();
@@ -160,7 +173,7 @@ void BlockFileCacheTtlMgr::run_backgroud_expiration_check() {
 
     while (!_stop_background.load(std::memory_order_acquire)) {
         try {
-            std::unordered_map<int64_t, TtlInfo> ttl_info_copy;
+            std::map<int64_t, TtlInfo> ttl_info_copy;
 
             // Copy TTL info for processing
             {
@@ -171,12 +184,15 @@ void BlockFileCacheTtlMgr::run_backgroud_expiration_check() {
             uint64_t current_time = UnixSeconds();
 
             for (const auto& [tablet_id, ttl_info] : ttl_info_copy) {
-                if (ttl_info.create_time + ttl_info.ttl < current_time) {
+                if (ttl_info.tablet_ctime + ttl_info.ttl < current_time) {
                     // Tablet has expired, convert TTL blocks back to NORMAL type
                     FileBlocks blocks = get_file_blocks_from_tablet_id(tablet_id);
                     for (auto& block : blocks) {
                         if (block->cache_type() == FileCacheType::TTL) {
-                            block->change_cache_type(FileCacheType::NORMAL);
+                            auto st = block->change_cache_type(FileCacheType::NORMAL);
+                            if (!st.ok()) {
+                                LOG(WARNING) << "Failed to convert block back to NORMAL cache_type";
+                            }
                         }
                     }
 

be/src/io/cache/block_file_cache_ttl_mgr.h

@@ -26,6 +26,7 @@
 #include <thread>
 #include <unordered_set>
 
+#include "io/cache/file_block.h"
 #include "io/cache/file_cache_common.h"
 
 namespace doris::io {
@@ -35,7 +36,7 @@ class CacheBlockMetaStore;
 
 struct TtlInfo {
     uint64_t ttl;
-    uint64_t create_time;
+    uint64_t tablet_ctime;
 };
 
 class BlockFileCacheTtlMgr {
@@ -45,16 +46,18 @@ class BlockFileCacheTtlMgr {
 
     void register_tablet_id(int64_t tablet_id);
 
-    // Background thread functions
+    // Background thread to update ttl_info_map
     void run_backgroud_update_ttl_info_map();
+    // Background thread to find expired tablet and evict from ttl queue
     void run_backgroud_expiration_check();
 
 private:
     FileBlocks get_file_blocks_from_tablet_id(int64_t tablet_id);
 
 private:
-    std::unordered_set<int64_t> _tablet_id_set;
-    std::map<int64_t, TtlInfo> _ttl_info_map;
+    // the set contains all the tablet ids which has cache data
+    std::unordered_set<int64_t> _tablet_id_set; //TODO(zhengyu): clean up old tablet ids
+    std::map<int64_t /* tablet_id */, TtlInfo> _ttl_info_map;
     BlockFileCache* _mgr;
     CacheBlockMetaStore* _meta_store;