github-actions[bot] commented on code in PR #67313:
URL: https://github.com/apache/doris/pull/67313#discussion_r3888416135
##########
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;
Review Comment:
[P1] Preserve shrink enforcement while metadata is loading
`FSFileCacheStorage::init()` publishes the cache before `_async_open_done`
is set, and `try_reserve()` bypasses normal capacity enforcement during that
window. After a 100-to-30 reset that leaves 30 bytes cached, removing this
assignment leaves `_disk_resource_limit_mode` false; a subsequent 5-byte miss
makes `try_reserve_during_async_load()` evict nothing and return true, so
`add_cell()` grows the cache to 35. The old assignment supplied the only guard
on this path. Please enforce the new capacity independently of disk-pressure
hysteresis (or gate reset/admission until loading completes), and add a test
that holds the loader before readiness across the shrink.
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -2244,17 +2234,19 @@ 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;
}
Review Comment:
[P1] Keep the eviction target separate from the admitted size
When mode is retained in the hysteresis band, `try_reserve()` multiplies a
5-byte request to 25 for eviction pressure and then passes that value to
`QueryFileCacheContext::reserve()`, while `add_cell()` still creates a 5-byte
cell. With a 30-byte query limit, the next 5-byte miss finds a recorded size of
25 for a 5-byte cell and hits the size `DCHECK`; release builds still overcount
query usage and evict/admit using incorrect byte totals. This was masked on the
base by the capacity-slack pre-clear. Please preserve the original block size
for query accounting and use a separate inflated eviction target, with an
in-band query-limit regression test.
--
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]
