This is an automated email from the ASF dual-hosted git repository.
gavinchou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 295346ad242 [fix](file cache) count evicted bytes only for blocks that
were downloaded (#67317)
295346ad242 is described below
commit 295346ad2429c8f213b13aa68945509d72dd2507
Author: deardeng <[email protected]>
AuthorDate: Fri Sep 18 12:33:46 2026 +0800
[fix](file cache) count evicted bytes only for blocks that were downloaded
(#67317)
remove() bumped _queue_evict_size_metrics and _total_evict_size_metrics
unconditionally, while the physical delete happened only for DOWNLOADED
blocks. Removing an EMPTY or SKIP_CACHE block therefore charged a full
range().size() -- for an EMPTY block the entire preallocated block size
-- while freeing nothing on disk. That is why the reported eviction rate
ran far above read plus write traffic combined: 300MiB/s average against
70MiB/s of real traffic, with the difference coming from blocks created
and dropped without ever being downloaded.
Read the block state once, move both counters into the DOWNLOADED
branch, and add evict_not_downloaded_size / evict_not_downloaded_num so
that churn becomes measurable instead of hiding inside the eviction
rate. DOWNLOADING blocks are not counted here: they are only tagged for
deletion and get counted on the removal that actually frees them.
test_evict_metrics_only_count_downloaded_blocks covers both directions,
including the per-queue counter, which the total alone would not catch.
The evict_in_advance test now restores the two configs it changes, so it
cannot leave background eviction enabled for whatever runs after it
under --gtest_shuffle.
---
be/src/io/cache/block_file_cache.cpp | 23 ++++++--
be/src/io/cache/block_file_cache.h | 2 +
be/test/io/cache/block_file_cache_test.cpp | 91 ++++++++++++++++++++++++++++++
3 files changed, 110 insertions(+), 6 deletions(-)
diff --git a/be/src/io/cache/block_file_cache.cpp
b/be/src/io/cache/block_file_cache.cpp
index 6b9ad1d84e4..e3eaa691b9f 100644
--- a/be/src/io/cache/block_file_cache.cpp
+++ b/be/src/io/cache/block_file_cache.cpp
@@ -226,6 +226,10 @@ BlockFileCache::BlockFileCache(const std::string&
cache_base_path,
_cache_base_path.c_str(), "file_cache_ttl_cache_evict_size");
_total_evict_size_metrics = std::make_shared<bvar::Adder<size_t>>(
_cache_base_path.c_str(), "file_cache_total_evict_size");
+ _evict_not_downloaded_size_metrics = std::make_shared<bvar::Adder<size_t>>(
+ _cache_base_path.c_str(), "file_cache_evict_not_downloaded_size");
+ _evict_not_downloaded_num_metrics = std::make_shared<bvar::Adder<size_t>>(
+ _cache_base_path.c_str(), "file_cache_evict_not_downloaded_num");
_total_read_size_metrics =
std::make_shared<bvar::Adder<size_t>>(_cache_base_path.c_str(),
"file_cache_total_read_size");
_total_hit_size_metrics =
std::make_shared<bvar::Adder<size_t>>(_cache_base_path.c_str(),
@@ -1836,15 +1840,15 @@ void BlockFileCache::remove(FileBlockSPtr file_block,
T& cache_lock, U& block_lo
cell->file_block->get_hash_value(),
cell->file_block->offset(),
cell->size());
}
- *_queue_evict_size_metrics[file_cache_type_index(file_block->cache_type())]
- << file_block->range().size();
- *_total_evict_size_metrics << file_block->range().size();
-
VLOG_DEBUG << "Removing file block from cache. hash: " << hash.to_string()
<< ", offset: " << offset << ", size: " <<
file_block->range().size()
<< ", type: " << cache_type_to_string(type);
- if (file_block->state_unlock(block_lock) == FileBlock::State::DOWNLOADED) {
+ const auto state = file_block->state_unlock(block_lock);
+ if (state == FileBlock::State::DOWNLOADED) {
+
*_queue_evict_size_metrics[file_cache_type_index(file_block->cache_type())]
+ << file_block->range().size();
+ *_total_evict_size_metrics << file_block->range().size();
FileCacheKey key;
key.hash = hash;
key.offset = offset;
@@ -1883,9 +1887,12 @@ void BlockFileCache::remove(FileBlockSPtr file_block, T&
cache_lock, U& block_lo
}
}
}
- } else if (file_block->state_unlock(block_lock) ==
FileBlock::State::DOWNLOADING) {
+ } else if (state == FileBlock::State::DOWNLOADING) {
file_block->set_deleting();
return;
+ } else {
+ *_evict_not_downloaded_size_metrics << file_block->range().size();
+ *_evict_not_downloaded_num_metrics << 1;
}
_cur_cache_size -= file_block->range().size();
if (FileCacheType::TTL == type) {
@@ -2773,6 +2780,8 @@ std::map<std::string, double> BlockFileCache::get_stats()
{
stats["total_read_size"] = (double)_total_read_size_metrics->get_value();
stats["total_hit_size"] = (double)_total_hit_size_metrics->get_value();
stats["total_removed_size"] =
(double)_total_evict_size_metrics->get_value();
+ stats["evict_not_downloaded_size"] =
(double)_evict_not_downloaded_size_metrics->get_value();
+ stats["evict_not_downloaded_num"] =
(double)_evict_not_downloaded_num_metrics->get_value();
return stats;
}
@@ -2826,6 +2835,8 @@ std::map<std::string, double>
BlockFileCache::get_stats_unsafe() {
stats["total_read_size"] = (double)_total_read_size_metrics->get_value();
stats["total_hit_size"] = (double)_total_hit_size_metrics->get_value();
stats["total_removed_size"] =
(double)_total_evict_size_metrics->get_value();
+ stats["evict_not_downloaded_size"] =
(double)_evict_not_downloaded_size_metrics->get_value();
+ stats["evict_not_downloaded_num"] =
(double)_evict_not_downloaded_num_metrics->get_value();
return stats;
}
diff --git a/be/src/io/cache/block_file_cache.h
b/be/src/io/cache/block_file_cache.h
index 99830cad839..2de0182ba7f 100644
--- a/be/src/io/cache/block_file_cache.h
+++ b/be/src/io/cache/block_file_cache.h
@@ -609,6 +609,8 @@ private:
std::shared_ptr<bvar::Adder<size_t>> _total_read_size_metrics;
std::shared_ptr<bvar::Adder<size_t>> _total_hit_size_metrics;
std::shared_ptr<bvar::Adder<size_t>> _total_evict_size_metrics;
+ std::shared_ptr<bvar::Adder<size_t>> _evict_not_downloaded_size_metrics;
+ std::shared_ptr<bvar::Adder<size_t>> _evict_not_downloaded_num_metrics;
std::shared_ptr<bvar::Adder<size_t>> _gc_evict_bytes_metrics;
std::shared_ptr<bvar::Adder<size_t>> _gc_evict_count_metrics;
std::shared_ptr<bvar::Adder<size_t>> _evict_by_time_metrics_matrix[4][4];
diff --git a/be/test/io/cache/block_file_cache_test.cpp
b/be/test/io/cache/block_file_cache_test.cpp
index 22e6bae5d80..68d1d09db15 100644
--- a/be/test/io/cache/block_file_cache_test.cpp
+++ b/be/test/io/cache/block_file_cache_test.cpp
@@ -3848,6 +3848,89 @@ TEST_F(BlockFileCacheTest, remove_directly) {
}
}
+TEST_F(BlockFileCacheTest, test_evict_metrics_only_count_downloaded_blocks) {
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+ fs::create_directories(cache_base_path);
+
+ const auto original_enable_evict_in_advance =
config::enable_evict_file_cache_in_advance;
+ Defer restore_evict_in_advance {
+ [&] { config::enable_evict_file_cache_in_advance =
original_enable_evict_in_advance; }};
+ config::enable_evict_file_cache_in_advance = false;
+
+ io::FileCacheSettings settings;
+ settings.query_queue_size = 30;
+ settings.query_queue_elements = 5;
+ settings.capacity = 90;
+ settings.max_file_block_size = 30;
+ settings.max_query_cache_size = 30;
+ io::BlockFileCache cache(cache_base_path, settings);
+ ASSERT_TRUE(cache.initialize());
+ wait_until_cache_ready(cache);
+
+ io::CacheContext context;
+ ReadStatistics rstats;
+ context.stats = &rstats;
+ context.cache_type = io::FileCacheType::NORMAL;
+ const auto downloaded_key = io::BlockFileCache::hash("downloaded-key");
+ const auto empty_key = io::BlockFileCache::hash("empty-key");
+
+ {
+ auto holder = cache.get_or_set(downloaded_key, 0, 5, context);
+ ASSERT_EQ(holder.file_blocks.size(), 1);
+ ASSERT_TRUE(holder.file_blocks.front()->get_or_set_downloader() ==
+ io::FileBlock::get_caller_id());
+ download(holder.file_blocks.front());
+ }
+ EXPECT_EQ(cache._cur_cache_size, 5);
+ const auto before_downloaded_remove = cache.get_stats_unsafe();
+ const auto before_downloaded_queue_evict_size =
+
cache._queue_evict_size_metrics[file_cache_type_index(context.cache_type)]->get_value();
+ cache.remove_if_cached(downloaded_key);
+ const auto after_downloaded_remove = cache.get_stats_unsafe();
+ EXPECT_EQ(after_downloaded_remove.at("total_removed_size") -
+ before_downloaded_remove.at("total_removed_size"),
+ 5);
+
EXPECT_EQ(cache._queue_evict_size_metrics[file_cache_type_index(context.cache_type)]
+ ->get_value() -
+ before_downloaded_queue_evict_size,
+ 5);
+ EXPECT_EQ(cache._cur_cache_size, 0);
+
+ const auto before_empty_remove = cache.get_stats_unsafe();
+ const auto before_empty_queue_evict_size =
+
cache._queue_evict_size_metrics[file_cache_type_index(context.cache_type)]->get_value();
+ {
+ auto holder = cache.get_or_set(empty_key, 0, 5, context);
+ ASSERT_EQ(holder.file_blocks.size(), 1);
+ EXPECT_EQ(holder.file_blocks.front()->state(),
io::FileBlock::State::EMPTY);
+ EXPECT_EQ(cache._cur_cache_size, 5);
+ }
+ const auto after_empty_remove = cache.get_stats_unsafe();
+ EXPECT_EQ(after_empty_remove.at("total_removed_size"),
+ before_empty_remove.at("total_removed_size"));
+ EXPECT_EQ(
+
cache._queue_evict_size_metrics[file_cache_type_index(context.cache_type)]->get_value(),
+ before_empty_queue_evict_size);
+ EXPECT_EQ(cache._cur_cache_size, 0);
+
+ EXPECT_EQ(after_downloaded_remove.at("evict_not_downloaded_size"),
+ before_downloaded_remove.at("evict_not_downloaded_size"));
+ EXPECT_EQ(after_downloaded_remove.at("evict_not_downloaded_num"),
+ before_downloaded_remove.at("evict_not_downloaded_num"));
+ EXPECT_EQ(after_empty_remove.at("evict_not_downloaded_size") -
+ before_empty_remove.at("evict_not_downloaded_size"),
+ 5);
+ EXPECT_EQ(after_empty_remove.at("evict_not_downloaded_num") -
+ before_empty_remove.at("evict_not_downloaded_num"),
+ 1);
+
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+}
+
TEST_F(BlockFileCacheTest, late_holder_remove_skips_missing_cache_cell) {
if (fs::exists(cache_base_path)) {
fs::remove_all(cache_base_path);
@@ -8083,6 +8166,14 @@ TEST_F(BlockFileCacheTest, evict_in_advance) {
settings.max_file_block_size = 100000;
settings.max_query_cache_size = 30;
+ const auto original_enable_evict_in_advance =
config::enable_evict_file_cache_in_advance;
+ const auto original_evict_in_advance_batch_bytes =
+ config::file_cache_evict_in_advance_batch_bytes;
+ Defer restore_evict_in_advance_config {[&] {
+ config::enable_evict_file_cache_in_advance =
original_enable_evict_in_advance;
+ config::file_cache_evict_in_advance_batch_bytes =
original_evict_in_advance_batch_bytes;
+ }};
+
size_t limit = 1000000;
size_t cache_max = 10000000;
io::CacheContext context;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
