diff --git a/be/src/io/cache/block_file_cache.cpp b/be/src/io/cache/block_file_cache.cpp index d0ea528bb3e4cc..3f961ce9437765 100644 --- a/be/src/io/cache/block_file_cache.cpp +++ b/be/src/io/cache/block_file_cache.cpp @@ -2194,8 +2194,6 @@ std::string BlockFileCache::reset_capacity(size_t new_capacity) { queue_released = remove_blocks(_ttl_queue); ss << " ttl_queue released " << queue_released; - _disk_resource_limit_mode = true; - _disk_limit_mode_metrics->set_value(1); ss << " total_space_released=" << space_released; } old_capacity = _capacity; @@ -2215,11 +2213,6 @@ void BlockFileCache::check_disk_resource_limit() { return; } - bool previous_mode = _disk_resource_limit_mode; - if (_capacity > _cur_cache_size) { - _disk_resource_limit_mode = false; - _disk_limit_mode_metrics->set_value(0); - } std::pair percent; int ret = disk_used_percentage(_cache_base_path, &percent); if (ret != 0) { @@ -2244,18 +2237,21 @@ void BlockFileCache::check_disk_resource_limit() { config::file_cache_enter_disk_resource_limit_mode_percent = 88; config::file_cache_exit_disk_resource_limit_mode_percent = 80; } + bool previous_mode = _disk_resource_limit_mode.load(); bool is_space_insufficient = is_insufficient(space_percentage); bool is_inode_insufficient = is_insufficient(inode_percentage); + // Enter when either resource reaches the enter threshold, but exit only after both + // resources fall below the exit threshold. Values in [exit, enter) preserve the previous + // mode through _disk_resource_limit_mode. if (is_space_insufficient || is_inode_insufficient) { _disk_resource_limit_mode = true; - _disk_limit_mode_metrics->set_value(1); } else if (_disk_resource_limit_mode && (space_percentage < config::file_cache_exit_disk_resource_limit_mode_percent) && (inode_percentage < config::file_cache_exit_disk_resource_limit_mode_percent)) { _disk_resource_limit_mode = false; - _disk_limit_mode_metrics->set_value(0); } - if (previous_mode != _disk_resource_limit_mode) { + _disk_limit_mode_metrics->set_value(_disk_resource_limit_mode.load()); + if (previous_mode != _disk_resource_limit_mode.load()) { // add log for disk resource limit mode switching if (_disk_resource_limit_mode) { LOG(WARNING) << "Entering disk resource limit mode: file_cache=" << get_base_path() @@ -2312,7 +2308,7 @@ void BlockFileCache::check_need_evict_cache_in_advance() { config::file_cache_enter_need_evict_cache_in_advance_percent = 78; config::file_cache_exit_need_evict_cache_in_advance_percent = 75; } - bool previous_mode = _need_evict_cache_in_advance; + bool previous_mode = _need_evict_cache_in_advance.load(); bool is_space_insufficient = is_insufficient(space_percentage); bool is_inode_insufficient = is_insufficient(inode_percentage); bool is_size_insufficient = is_insufficient(size_percentage); @@ -2326,7 +2322,7 @@ void BlockFileCache::check_need_evict_cache_in_advance() { _need_evict_cache_in_advance = false; _need_evict_cache_in_advance_metrics->set_value(0); } - if (previous_mode != _need_evict_cache_in_advance) { + if (previous_mode != _need_evict_cache_in_advance.load()) { // add log for evict cache in advance mode switching if (_need_evict_cache_in_advance) { LOG(WARNING) << "Entering evict cache in advance mode: " @@ -2764,8 +2760,8 @@ std::map BlockFileCache::get_stats() { (double)_lru_recorder_shadow_queue_element_count_metrics[FileCacheType::DISPOSABLE] ->get_value(); - stats["need_evict_cache_in_advance"] = (double)_need_evict_cache_in_advance; - stats["disk_resource_limit_mode"] = (double)_disk_resource_limit_mode; + stats["need_evict_cache_in_advance"] = (double)_need_evict_cache_in_advance.load(); + stats["disk_resource_limit_mode"] = (double)_disk_resource_limit_mode.load(); stats["total_removed_counts"] = (double)_num_removed_blocks->get_value(); stats["total_hit_counts"] = (double)_num_hit_blocks->get_value(); @@ -2817,8 +2813,8 @@ std::map BlockFileCache::get_stats_unsafe() { (double)_lru_recorder_shadow_queue_element_count_metrics[FileCacheType::DISPOSABLE] ->get_value(); - stats["need_evict_cache_in_advance"] = (double)_need_evict_cache_in_advance; - stats["disk_resource_limit_mode"] = (double)_disk_resource_limit_mode; + stats["need_evict_cache_in_advance"] = (double)_need_evict_cache_in_advance.load(); + stats["disk_resource_limit_mode"] = (double)_disk_resource_limit_mode.load(); stats["total_removed_counts"] = (double)_num_removed_blocks->get_value(); stats["total_hit_counts"] = (double)_num_hit_blocks->get_value(); diff --git a/be/src/io/cache/block_file_cache.h b/be/src/io/cache/block_file_cache.h index 91c453e12aa414..cab6b4f2e06048 100644 --- a/be/src/io/cache/block_file_cache.h +++ b/be/src/io/cache/block_file_cache.h @@ -555,8 +555,8 @@ class BlockFileCache { std::thread _cache_background_block_lru_update_thread; std::atomic_bool _async_open_done {false}; // disk space or inode is less than the specified value - bool _disk_resource_limit_mode {false}; - bool _need_evict_cache_in_advance {false}; + std::atomic _disk_resource_limit_mode {false}; + std::atomic _need_evict_cache_in_advance {false}; bool _is_initialized {false}; // strategy diff --git a/be/test/io/cache/block_file_cache_test.cpp b/be/test/io/cache/block_file_cache_test.cpp index 22e6bae5d80be7..657bb00d7102ce 100644 --- a/be/test/io/cache/block_file_cache_test.cpp +++ b/be/test/io/cache/block_file_cache_test.cpp @@ -5917,7 +5917,7 @@ TEST_F(BlockFileCacheTest, test_check_disk_reource_limit_2) { std::this_thread::sleep_for(std::chrono::milliseconds(10)); EXPECT_EQ(config::file_cache_enter_disk_resource_limit_mode_percent, 2); EXPECT_EQ(config::file_cache_exit_disk_resource_limit_mode_percent, 1); - EXPECT_TRUE(cache._disk_resource_limit_mode); + EXPECT_TRUE(cache._disk_resource_limit_mode.load()); config::file_cache_enter_disk_resource_limit_mode_percent = 99; if (fs::exists(cache_base_path)) { fs::remove_all(cache_base_path); @@ -5946,13 +5946,93 @@ TEST_F(BlockFileCacheTest, test_check_disk_reource_limit_3) { std::this_thread::sleep_for(std::chrono::milliseconds(1)); } std::this_thread::sleep_for(std::chrono::milliseconds(10)); - EXPECT_FALSE(cache._disk_resource_limit_mode); + EXPECT_FALSE(cache._disk_resource_limit_mode.load()); config::file_cache_exit_disk_resource_limit_mode_percent = 80; if (fs::exists(cache_base_path)) { fs::remove_all(cache_base_path); } } +TEST_F(BlockFileCacheTest, test_check_disk_resource_limit_hysteresis) { + if (fs::exists(cache_base_path)) { + fs::remove_all(cache_base_path); + } + fs::create_directories(cache_base_path); + + const auto origin_enter = config::file_cache_enter_disk_resource_limit_mode_percent; + const auto origin_exit = config::file_cache_exit_disk_resource_limit_mode_percent; + auto* sp = SyncPoint::get_instance(); + Defer defer {[&] { + config::file_cache_enter_disk_resource_limit_mode_percent = origin_enter; + config::file_cache_exit_disk_resource_limit_mode_percent = origin_exit; + sp->disable_processing(); + sp->clear_call_back("BlockFileCache::disk_used_percentage:1"); + if (fs::exists(cache_base_path)) { + fs::remove_all(cache_base_path); + } + }}; + + config::file_cache_enter_disk_resource_limit_mode_percent = 85; + config::file_cache_exit_disk_resource_limit_mode_percent = 80; + + io::FileCacheSettings settings; + settings.capacity = 100_mb; + settings.storage = "disk"; + io::BlockFileCache cache(cache_base_path, settings); + + std::pair disk_usage {90, 70}; + sp->set_call_back("BlockFileCache::disk_used_percentage:1", [&](auto&& values) { + *try_any_cast*>(values.back()) = disk_usage; + }); + sp->enable_processing(); + + cache.check_disk_resource_limit(); + EXPECT_TRUE(cache._disk_resource_limit_mode.load()); + EXPECT_EQ(cache._disk_limit_mode_metrics->get_value(), cache._disk_resource_limit_mode.load()); + + cache._disk_resource_limit_mode = false; + disk_usage = {70, 90}; + cache.check_disk_resource_limit(); + EXPECT_TRUE(cache._disk_resource_limit_mode.load()); + EXPECT_EQ(cache._disk_limit_mode_metrics->get_value(), cache._disk_resource_limit_mode.load()); + + ASSERT_GT(cache._capacity, cache._cur_cache_size); + disk_usage = {82, 70}; + cache.check_disk_resource_limit(); + EXPECT_TRUE(cache._disk_resource_limit_mode.load()); + EXPECT_EQ(cache._disk_limit_mode_metrics->get_value(), cache._disk_resource_limit_mode.load()); + + disk_usage = {70, 70}; + cache.check_disk_resource_limit(); + EXPECT_FALSE(cache._disk_resource_limit_mode.load()); + EXPECT_EQ(cache._disk_limit_mode_metrics->get_value(), cache._disk_resource_limit_mode.load()); +} + +TEST_F(BlockFileCacheTest, test_check_disk_resource_limit_statfs_failure_preserves_state) { + if (fs::exists(cache_base_path)) { + fs::remove_all(cache_base_path); + } + fs::create_directories(cache_base_path); + Defer cleanup {[&] { + if (fs::exists(cache_base_path)) { + fs::remove_all(cache_base_path); + } + }}; + + io::FileCacheSettings settings; + settings.capacity = 100_mb; + settings.storage = "disk"; + io::BlockFileCache cache(cache_base_path, settings); + cache._disk_resource_limit_mode = true; + cache._disk_limit_mode_metrics->set_value(1); + cache._cache_base_path = "/non/existent/path/OOXXOO"; + + cache.check_disk_resource_limit(); + + EXPECT_TRUE(cache._disk_resource_limit_mode.load()); + EXPECT_EQ(cache._disk_limit_mode_metrics->get_value(), 1); +} + TEST_F(BlockFileCacheTest, test_align_size) { const size_t total_size = 10_mb + 10086; { @@ -6424,9 +6504,13 @@ TEST_F(BlockFileCacheTest, reset_capacity) { assert_range(1, segments[0], io::FileBlock::Range(offset, offset + 4), io::FileBlock::State::DOWNLOADED); } + cache._disk_resource_limit_mode = false; + cache._disk_limit_mode_metrics->set_value(0); std::cout << cache.reset_capacity(30) << std::endl; EXPECT_EQ(cache._cur_cache_size, 30); + EXPECT_FALSE(cache._disk_resource_limit_mode.load()); + EXPECT_EQ(cache._disk_limit_mode_metrics->get_value(), 0); if (fs::exists(cache_base_path)) { fs::remove_all(cache_base_path); } @@ -8246,9 +8330,9 @@ TEST_F(BlockFileCacheTest, test_check_need_evict_cache_in_advance) { { settings.storage = "memory"; io::BlockFileCache cache(cache_base_path, settings); - ASSERT_FALSE(cache._need_evict_cache_in_advance); + ASSERT_FALSE(cache._need_evict_cache_in_advance.load()); cache.check_need_evict_cache_in_advance(); - ASSERT_FALSE(cache._need_evict_cache_in_advance); + ASSERT_FALSE(cache._need_evict_cache_in_advance.load()); } // the rest for disk @@ -8257,17 +8341,17 @@ TEST_F(BlockFileCacheTest, test_check_need_evict_cache_in_advance) { // bad disk path { io::BlockFileCache cache(cache_base_path, settings); - ASSERT_FALSE(cache._need_evict_cache_in_advance); + ASSERT_FALSE(cache._need_evict_cache_in_advance.load()); cache._cache_base_path = "/non/existent/path/OOXXOO"; cache.check_need_evict_cache_in_advance(); - ASSERT_FALSE(cache._need_evict_cache_in_advance); + ASSERT_FALSE(cache._need_evict_cache_in_advance.load()); } // conditions for enter need evict cache in advance { io::BlockFileCache cache(cache_base_path, settings); - ASSERT_FALSE(cache._need_evict_cache_in_advance); + ASSERT_FALSE(cache._need_evict_cache_in_advance.load()); // condition1 space usage rate exceed threshold config::file_cache_enter_need_evict_cache_in_advance_percent = 70; @@ -8282,7 +8366,7 @@ TEST_F(BlockFileCacheTest, test_check_need_evict_cache_in_advance) { SyncPoint::get_instance()->enable_processing(); cache.check_need_evict_cache_in_advance(); - ASSERT_TRUE(cache._need_evict_cache_in_advance); + ASSERT_TRUE(cache._need_evict_cache_in_advance.load()); SyncPoint::get_instance()->disable_processing(); SyncPoint::get_instance()->clear_all_call_backs(); @@ -8298,7 +8382,7 @@ TEST_F(BlockFileCacheTest, test_check_need_evict_cache_in_advance) { SyncPoint::get_instance()->enable_processing(); cache.check_need_evict_cache_in_advance(); - ASSERT_TRUE(cache._need_evict_cache_in_advance); + ASSERT_TRUE(cache._need_evict_cache_in_advance.load()); SyncPoint::get_instance()->disable_processing(); SyncPoint::get_instance()->clear_all_call_backs(); @@ -8306,7 +8390,7 @@ TEST_F(BlockFileCacheTest, test_check_need_evict_cache_in_advance) { cache._need_evict_cache_in_advance = false; cache._cur_cache_size = 80_mb; // set high cache.check_need_evict_cache_in_advance(); - ASSERT_TRUE(cache._need_evict_cache_in_advance); + ASSERT_TRUE(cache._need_evict_cache_in_advance.load()); } // conditions for exit need evict cache in advance @@ -8324,7 +8408,7 @@ TEST_F(BlockFileCacheTest, test_check_need_evict_cache_in_advance) { SyncPoint::get_instance()->enable_processing(); cache.check_need_evict_cache_in_advance(); - ASSERT_FALSE(cache._need_evict_cache_in_advance); + ASSERT_FALSE(cache._need_evict_cache_in_advance.load()); SyncPoint::get_instance()->disable_processing(); SyncPoint::get_instance()->clear_all_call_backs(); } @@ -8398,7 +8482,7 @@ TEST_F(BlockFileCacheTest, test_evict_cache_in_advance_skip) { ASSERT_TRUE(cache.get_async_open_success()); cache.check_need_evict_cache_in_advance(); - ASSERT_TRUE(cache._need_evict_cache_in_advance); + ASSERT_TRUE(cache._need_evict_cache_in_advance.load()); // Set recycle keys threshold and fill with enough keys config::file_cache_evict_in_advance_recycle_keys_num_threshold = 10;