diff --git a/be/src/io/cache/block_file_cache.cpp b/be/src/io/cache/block_file_cache.cpp index 4ba15fafaa75b8..b8f4f3984eb398 100644 --- a/be/src/io/cache/block_file_cache.cpp +++ b/be/src/io/cache/block_file_cache.cpp @@ -2397,7 +2397,6 @@ void BlockFileCache::run_background_monitor() { (double)_no_warmup_num_read_blocks_1h->get_value()); } } - _lru_recorder->update_shadow_queue_element_count_metrics(); } } diff --git a/be/src/io/cache/lru_queue_recorder.cpp b/be/src/io/cache/lru_queue_recorder.cpp index c8227129bdac57..8bd202ee8476bb 100644 --- a/be/src/io/cache/lru_queue_recorder.cpp +++ b/be/src/io/cache/lru_queue_recorder.cpp @@ -181,11 +181,6 @@ size_t LRUQueueRecorder::lru_log_queue_size(FileCacheType type) const { return _lru_log_queue_size[file_cache_type_index(type)].load(std::memory_order_relaxed); } -void LRUQueueRecorder::update_shadow_queue_element_count_metrics() { - std::lock_guard lru_log_lock(_mutex_lru_log); - update_shadow_queue_element_count_metrics_unlocked(lru_log_lock); -} - void LRUQueueRecorder::limit_shadow_queue_size(LRUQueue& shadow_queue, std::lock_guard& lru_log_lock) { int64_t queue_limit = config::file_cache_background_lru_dump_tail_record_num; @@ -202,16 +197,6 @@ void LRUQueueRecorder::limit_shadow_queue_size(LRUQueue& shadow_queue, } } -void LRUQueueRecorder::update_shadow_queue_element_count_metrics_unlocked( - std::lock_guard& lru_log_lock) { - for (FileCacheType type : {FileCacheType::DISPOSABLE, FileCacheType::NORMAL, - FileCacheType::INDEX, FileCacheType::TTL}) { - size_t idx = file_cache_type_index(type); - _mgr->_lru_recorder_shadow_queue_element_count_metrics[idx]->set_value( - get_shadow_queue(type).get_elements_num(lru_log_lock)); - } -} - bool LRUQueueRecorder::reserve_lru_log_queue_slot(FileCacheType type) { int64_t queue_limit = config::file_cache_background_lru_log_queue_max_size; if (queue_limit <= 0) { diff --git a/be/src/io/cache/lru_queue_recorder.h b/be/src/io/cache/lru_queue_recorder.h index 1edd0f5ab85bc9..058b78194ff82a 100644 --- a/be/src/io/cache/lru_queue_recorder.h +++ b/be/src/io/cache/lru_queue_recorder.h @@ -67,7 +67,6 @@ class LRUQueueRecorder { size_t get_lru_queue_update_cnt_from_last_dump(FileCacheType type); void reset_lru_queue_update_cnt_from_last_dump(FileCacheType type); size_t lru_log_queue_size(FileCacheType type) const; - void update_shadow_queue_element_count_metrics(); CacheLRULogQueue& get_lru_log_queue(FileCacheType type); LRUQueue& get_shadow_queue(FileCacheType type); @@ -94,8 +93,6 @@ class LRUQueueRecorder { bool reserve_lru_log_queue_slot(FileCacheType type); void release_lru_log_queue_slot(FileCacheType type); void limit_shadow_queue_size(LRUQueue& shadow_queue, std::lock_guard& lru_log_lock); - void update_shadow_queue_element_count_metrics_unlocked( - std::lock_guard& lru_log_lock); }; } // namespace doris::io diff --git a/be/test/io/cache/cache_lru_dumper_test.cpp b/be/test/io/cache/cache_lru_dumper_test.cpp index 6256f52d74a8be..e49a142bb2c9e6 100644 --- a/be/test/io/cache/cache_lru_dumper_test.cpp +++ b/be/test/io/cache/cache_lru_dumper_test.cpp @@ -311,28 +311,29 @@ TEST_F(CacheLRUDumperTest, test_remove_event_trims_existing_oversized_shadow_que EXPECT_EQ(offsets, std::vector({1, 2})); } -TEST_F(CacheLRUDumperTest, test_update_shadow_queue_metric_does_not_trim_queue) { +TEST_F(CacheLRUDumperTest, test_replay_publishes_shadow_queue_metric) { const auto old_tail_record_num = config::file_cache_background_lru_dump_tail_record_num; - Defer defer {[old_tail_record_num] { + const auto old_queue_limit = config::file_cache_background_lru_log_queue_max_size; + Defer defer {[old_tail_record_num, old_queue_limit] { config::file_cache_background_lru_dump_tail_record_num = old_tail_record_num; + config::file_cache_background_lru_log_queue_max_size = old_queue_limit; }}; - config::file_cache_background_lru_dump_tail_record_num = 1; + config::file_cache_background_lru_dump_tail_record_num = 100; + config::file_cache_background_lru_log_queue_max_size = 100; - UInt128Wrapper hash(778899ULL); - { - std::lock_guard lru_log_lock(recorder->_mutex_lru_log); - auto& shadow_queue = recorder->get_shadow_queue(FileCacheType::INDEX); - for (size_t offset = 0; offset < 3; ++offset) { - shadow_queue.add(hash, offset, 4096, lru_log_lock); - } + UInt128Wrapper hash(556677ULL); + for (size_t offset = 0; offset < 5; ++offset) { + recorder->record_queue_event(FileCacheType::NORMAL, CacheLRULogType::ADD, hash, offset, + 4096); } - recorder->update_shadow_queue_element_count_metrics(); - - EXPECT_EQ(recorder->get_shadow_queue(FileCacheType::INDEX).get_elements_num_unsafe(), 3); + // replay_queue_event() is the only thing that publishes this gauge, and it does so under the + // same lock that mutates the shadow queue. That is what lets run_background_monitor() stay + // off _mutex_lru_log. + EXPECT_EQ(recorder->replay_queue_event(FileCacheType::NORMAL), 5); auto stats = mock_cache->get_stats_unsafe(); - EXPECT_EQ(stats["lru_recorder_index_shadow_queue_curr_elements"], 3); + EXPECT_EQ(stats["lru_recorder_normal_shadow_queue_curr_elements"], 5); } TEST_F(CacheLRUDumperTest, test_remove_event_still_obeys_replay_queue_cap) {