deardeng commented on code in PR #67313:
URL: https://github.com/apache/doris/pull/67313#discussion_r3888544937
##########
be/test/io/cache/block_file_cache_test.cpp:
##########
@@ -3034,6 +3034,114 @@ TEST_F(BlockFileCacheTest, test_async_load_with_limit) {
}
}
+TEST_F(BlockFileCacheTest, test_async_load_respects_capacity_after_reset) {
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+ fs::create_directories(cache_base_path);
+ const auto original_monitor_interval =
config::file_cache_background_monitor_interval_ms;
+ config::file_cache_background_monitor_interval_ms = 1000000;
+ Defer cleanup {[&] {
+ config::file_cache_background_monitor_interval_ms =
original_monitor_interval;
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+ }};
+
+ io::FileCacheSettings settings;
+ settings.query_queue_size = 100;
+ settings.query_queue_elements = 20;
+ settings.capacity = 100;
+ settings.max_file_block_size = 5;
+ settings.max_query_cache_size = 0;
+ io::BlockFileCache cache(cache_base_path, settings);
+ ASSERT_TRUE(cache.initialize());
+ wait_until_cache_ready(cache);
+
+ const bool original_async_open_done =
cache._async_open_done.exchange(false);
+ Defer restore_async_open_done {[&] { cache._async_open_done =
original_async_open_done; }};
+ cache._disk_resource_limit_mode = false;
Review Comment:
You're right that wait_until_cache_ready() only waits on _async_open_done
and that raising file_cache_background_monitor_interval_ms does not order the
monitor's first check_disk_resource_limit(). Rather than gate that first check
with a test-side SyncPoint, I removed the race itself:
_disk_resource_limit_mode and _need_evict_cache_in_advance are now
std::atomic<bool>.
The race is not confined to the tests. run_background_monitor() writes both
flags without holding the cache lock, while try_reserve() and is_overflow()
read them under the cache lock and run_background_gc() reads
_need_evict_cache_in_advance at block_file_cache.cpp:2492. Both plain bools
were already racy in production; a test barrier would only have hidden the
report. The redundant _disk_resource_limit_mode = false writes in the two async
tests are gone as well — the flag defaults to false and the adjacent assertions
state the precondition.
--
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]
