github-actions[bot] commented on code in PR #68348:
URL: https://github.com/apache/doris/pull/68348#discussion_r4272900136
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -990,6 +994,63 @@ bool BlockFileCache::is_block_deleting(const
FileBlockSPtr& block) const {
return block->_is_deleting;
}
+template <class T>
+ requires IsXLock<T>
+void BlockFileCache::mark_cell_deleting(FileBlockCell& cell, T& /* cache_lock
*/) {
+ cell.file_block->set_deleting();
+ // The block stays in _files until its last reference is dropped. Remember
it so that
+ // run_background_gc() can finish the job if that reference is not
released through
+ // FileBlock::release_cache_reference(). remove() forgets it again when it
erases the cell.
+ _deleting_blocks.emplace(cell.file_block->get_hash_value(),
cell.file_block->offset());
+}
+
+size_t BlockFileCache::recycle_deleting_blocks(size_t batch_limit) {
+ if (batch_limit == 0) {
+ return 0;
+ }
+ size_t recycled = 0;
+ size_t pending = 0;
+ {
+ SCOPED_CACHE_LOCK(_mutex, this);
+ std::vector<FileBlockSPtr> to_remove;
+ for (const auto& [hash, offset] : _deleting_blocks) {
+ if (to_remove.size() >= batch_limit) {
+ break;
+ }
+ auto* cell = get_cell(hash, offset, cache_lock);
+ if (cell == nullptr || !cell->file_block->is_deleting())
[[unlikely]] {
+ // Cannot happen: remove() erases the key together with the
cell.
+ DCHECK(false) << "stale deleting block, hash=" <<
hash.to_string()
+ << " offset=" << offset;
+ continue;
+ }
+ if (!cell->releasable() ||
+ cell->file_block->state_unsafe() ==
FileBlock::State::DOWNLOADING) {
Review Comment:
[P2] Synchronize the sweep's download-state read
`releasable()` only checks `shared_ptr::use_count()`; it does not publish
writes made under `FileBlock::_mutex`. For example, with `enable_peer_s3_race`,
the losing peer task retains `empty_blocks` after S3 wins. If the local cache
append fails and a concurrent clear marked the block deleting, holder
destruction resets `_download_state` to `EMPTY` under the block mutex while
those peer references keep the count above two, so it skips taking the cache
mutex. The peer later drops its vector normally, and this sweep sees count one
and reads the state without a happens-before edge to that write. This is a C++
data race even after the holder released its reference ([shared_ptr observer
synchronization](https://eel.is/c++draft/util.smartptr.shared.obs)). Use
`state()` here, or take the block mutex before reading, preserving
cache-then-block order.
##########
be/src/io/cache/block_file_cache.cpp:
##########
@@ -990,6 +994,63 @@ bool BlockFileCache::is_block_deleting(const
FileBlockSPtr& block) const {
return block->_is_deleting;
}
+template <class T>
+ requires IsXLock<T>
+void BlockFileCache::mark_cell_deleting(FileBlockCell& cell, T& /* cache_lock
*/) {
+ cell.file_block->set_deleting();
+ // The block stays in _files until its last reference is dropped. Remember
it so that
+ // run_background_gc() can finish the job if that reference is not
released through
+ // FileBlock::release_cache_reference(). remove() forgets it again when it
erases the cell.
+ _deleting_blocks.emplace(cell.file_block->get_hash_value(),
cell.file_block->offset());
+}
+
+size_t BlockFileCache::recycle_deleting_blocks(size_t batch_limit) {
+ if (batch_limit == 0) {
+ return 0;
Review Comment:
[P3] Refresh the deleting-block gauge when GC removal is paused
Setting `file_cache_remove_block_qps_limit=0` makes every GC round return
here, before the only update of `file_cache_deleting_block_count`.
`mark_cell_deleting()` and holder cleanup still change `_deleting_blocks`, so
pausing removals and clearing eight retained blocks leaves the gauge at zero
indefinitely despite eight pending keys; an earlier nonzero sample can likewise
remain after holders remove them. This hides the backlog while removal is
paused. Sample the set size under the cache lock even with a zero budget, or
update the gauge when keys are inserted and erased; retain the zero-removal
behavior.
--
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]
