github-actions[bot] commented on code in PR #65722:
URL: https://github.com/apache/doris/pull/65722#discussion_r3642875193
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -1681,7 +1692,9 @@ void BlockFileCache::remove(FileBlockSPtr file_block, T&
cache_lock, U& block_lo
key.meta.type = type;
key.meta.expiration_time = expiration_time;
key.meta.tablet_id = tablet_id;
- if (sync) {
+ // Memory payloads are replaced in place by key, so a stale key-only
async delete could
+ // erase a newer payload downloaded after this block's metadata is
removed.
+ if (sync || is_memory_storage()) {
Review Comment:
[P2] Keep explicitly asynchronous memory removals asynchronous
This storage-wide condition also overrides `sync == false`, so
`clear_file_cache_async()` and `remove_if_cached_async()` now free every memory
payload inline while their callers hold the global cache mutex. Those APIs are
explicitly used for HTTP CLEAR and background reader/rowset/compaction cleanup;
a large memory-cache purge can therefore block the request or worker and all
concurrent cache admissions for the full deletion pass. Please preserve
identity safety while keeping these APIs asynchronous—for example, queue an
identity/generation-tagged payload for their removals—and retain an
async-return test.
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -1998,14 +2039,14 @@ std::string BlockFileCache::reset_capacity(size_t
new_capacity) {
if (need_remove_size <= 0) {
break;
}
- need_remove_size -= entry_size;
- space_released += entry_size;
- queue_released += entry_size;
auto* cell = get_cell(entry_key, entry_offset, cache_lock);
if (!cell->releasable()) {
cell->file_block->set_deleting();
Review Comment:
[P1] Close the RESET/last-holder deletion handoff
This mark can race the last holder's destructor: the destructor can read
`is_deleting() == false` under the block lock and release it, then RESET sees
the cache+holder reference count and sets this plain bool while holding only
the cache lock. The destructor follows its already-false decision and drops the
final non-cache reference without calling `remove()`, leaving the cell, memory
payload, and `_cur_cache_size` resident above the new capacity. Please
coordinate the mark and final-holder recheck under the established cache->block
lock order (or an equivalent atomic handoff), and add a barrier test for this
interleaving.
--
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]
