github-actions[bot] commented on code in PR #65722:
URL: https://github.com/apache/doris/pull/65722#discussion_r3637548911
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -1344,7 +1355,7 @@ bool BlockFileCache::try_reserve(const UInt128Wrapper&
hash, const CacheContext&
std::for_each(to_evict.begin(), to_evict.end(), remove_file_block_if);
if (is_overflow() &&
- !try_reserve_from_other_queue(context.cache_type, size, cur_time,
cache_lock)) {
+ !try_reserve_from_other_queue(context.cache_type, eviction_size,
cur_time, cache_lock)) {
Review Comment:
[P2] Continue one eviction plan after the query LRU
In resource-limit mode a 10 MiB admission targets 50 MiB. If this query
phase releases 10 MiB, the fallback starts a fresh 50 MiB counter; 40 MiB
elsewhere therefore frees the intended total but still returns false, while 50
MiB makes the request evict 60 MiB. The fallback also excludes
`context.cache_type`, so it returns `SKIP_CACHE` when the remaining cold blocks
are only in that global queue. Carry one removal total through other-queue and
current-queue phases, then test both combined-target and same-type cases while
asserting admission and final cache bytes.
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -1980,13 +1991,41 @@ int disk_used_percentage(const std::string& path,
std::pair<int, int>* percent)
return 0;
}
+namespace {
+
+const char* file_cache_storage_type_to_string(FileCacheStorageType type) {
+ switch (type) {
+ case FileCacheStorageType::DISK:
+ return "disk";
+ case FileCacheStorageType::MEMORY:
+ return "memory";
+ }
+ DORIS_CHECK(false) << "Unknown file cache storage type: " << type;
+ __builtin_unreachable();
+}
+
+} // namespace
+
+bool BlockFileCache::is_memory_storage() const {
+ return _storage->get_type() == FileCacheStorageType::MEMORY;
+}
+
+size_t
BlockFileCache::calc_size_percentage_unlocked(std::lock_guard<std::mutex>&)
const {
+ if (_capacity == 0) {
+ return 0;
+ }
+ return static_cast<size_t>(static_cast<double>(_cur_cache_size) /
Review Comment:
[P2] Preserve exact percentage thresholds
Converting the ratio to `double` and then truncating can round a
mathematically exact boundary down by one. With supported dynamic thresholds
`enter=58` and `exit=57`, 58 MiB / 100 MiB evaluates as `57.99999999999999` and
becomes 57, so the mode does not enter; 57 MiB similarly becomes 56 and exits
even though the hysteresis check is strictly below 57. This also replaces the
previous integer disk-size calculation. Use overflow-safe integer arithmetic or
cross-product comparisons, and cover entry and exit equality at an affected
boundary.
##########
be/src/io/cache/block_file_cache_factory.cpp:
##########
@@ -340,16 +341,35 @@ std::string validate_capacity(const std::string& path,
int64_t new_capacity,
return "";
}
+std::string validate_capacity(BlockFileCache* cache, int64_t new_capacity,
+ int64_t& valid_capacity) {
+ if (cache->get_storage()->get_type() == FileCacheStorageType::MEMORY) {
Review Comment:
[P2] Refresh live query limits when enabling memory reset
This branch now makes factory/HTTP RESET reach memory caches, but each
active `QueryFileCacheContext` froze `_capacity *
file_cache_query_limit_percent / 100` when the query started and
`reset_capacity()` only changes `_capacity`. A 50% query created at 100 MiB
therefore retains a 50 MiB ceiling after shrinking the cache to 20 MiB (so it
can occupy the whole cache), while a growth leaves it capped at the old smaller
value. Resize/reconcile live query contexts under the cache lock, and add a
holder-before-reset test for both shrink and growth.
--
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]
