github-actions[bot] commented on code in PR #65058:
URL: https://github.com/apache/doris/pull/65058#discussion_r3541262623
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -900,10 +901,21 @@ FileBlocks BlockFileCache::split_range_into_cells(const
UInt128Wrapper& hash,
while (current_pos < end_pos_non_included) {
current_size = std::min(remaining_size, _max_file_block_size);
remaining_size -= current_size;
- state = try_reserve(hash, context, current_pos, current_size,
cache_lock)
- ? state
- : FileBlock::State::SKIP_CACHE;
- if (state == FileBlock::State::SKIP_CACHE) [[unlikely]] {
+ auto block_state = state;
+ if (block_state != FileBlock::State::SKIP_CACHE &&
+ context.admit_cache_write_by_remote_scan_limiter) {
+ auto* limiter = context.remote_scan_cache_write_limiter;
+ DCHECK(limiter != nullptr);
+ if
(!limiter->try_admit_cache_write(static_cast<int64_t>(current_size))) {
Review Comment:
Here the limiter budget is consumed before we know the cache block can be
reserved. If `try_reserve()` then returns false, for example because eviction
cannot free enough space, this path emits a `SKIP_CACHE` block, so no
file-cache write was admitted, but `_admitted_data_bytes` has already been
increased. On a full or non-releasable cache this can make the query enter
remote-only-on-miss early and suppress later writes even though the threshold
of actual cache writes was not reached. Please either charge after a successful
reservation or roll the charge back on reservation failure, and add a test that
forces reservation failure after admission.
--
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]
