github-actions[bot] commented on code in PR #65722:
URL: https://github.com/apache/doris/pull/65722#discussion_r3652897093
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -1263,19 +1271,22 @@ bool BlockFileCache::try_reserve(const UInt128Wrapper&
hash, const CacheContext&
}
// use this strategy in scenarios where there is insufficient disk
capacity or insufficient number of inodes remaining
// directly eliminate 5 times the size of the space
- if (_disk_resource_limit_mode) {
- size = 5 * size;
- }
+ const bool disk_resource_limit_mode =
_disk_resource_limit_mode.load(std::memory_order_relaxed);
Review Comment:
[P2] Keep one pressure policy for the whole reservation
This snapshots the flag to derive `eviction_size`, but every downstream
`is_overflow()` reloads the atomic, and the memory monitor stores its
transition after releasing `_mutex`. A request can therefore derive 50 MiB
while the flag is true and then use normal-capacity math after it flips false:
at 70/100 MiB, a real 10 MiB request with cold blocks removes 20 MiB, whereas
coherent normal mode removes 0 and coherent pressure mode targets 50. The
reverse flip can keep a 10 MiB target after pressure mode enters and
under-evict. Please pass one mode snapshot (plus separate actual and target
sizes) through every phase, and cover both flip directions with a barrier test.
##########
be/src/io/cache/file_block.cpp:
##########
@@ -346,6 +346,15 @@ FileBlocksHolder::~FileBlocksHolder() {
file_block->state_unlock(block_lock) ==
FileBlock::State::EMPTY)) {
should_remove = true;
}
+ // Serialize the holder release with deletion marks. Otherwise
RESET can mark the
Review Comment:
[P1] Serialize every deletion producer with this reference drop
This closes the handoff only for RESET, whose marker now takes the block
mutex. `clear_file_cache_impl()`, `try_release()`, and
`remove_if_cached{,_async}()` still call `set_deleting()` while holding only
the cache mutex. If the last holder pauses here after reading `false`, CLEAR
can mark and skip the busy cell, then the holder erases its final external
reference without rechecking; CLEAR returns while the cache-only cell, memory
payload, and `_cur_cache_size` remain. Route every deletion mark through the
same cache-then-block recheck helper and add a barrier test for CLEAR/targeted
removal.
--
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]
