liaoxin01 opened a new pull request, #68348:
URL: https://github.com/apache/doris/pull/68348
### What problem does this PR solve?
Related Jira: CORE-6174
When `clear_file_cache_sync/async`, `remove_if_cached[_async]`,
`try_release` or `reset_capacity` run into a block that is still referenced,
they do not remove it. They only call `FileBlock::set_deleting()` and leave the
block in `_files`, expecting the last reference holder to remove it when it
releases.
The only code that honours that expectation is
`FileBlock::release_cache_reference()`, which runs from `~FileBlocksHolder` and
`~FileBlocksProbeResult`. Every other reference holder simply drops its
`shared_ptr`:
- the `batch` of `run_background_block_lru_update()`, which is declared
outside the loop and therefore keeps its references alive across a whole sleep
interval (5s by default);
- `clear_need_update_lru_blocks()`, which `clear_file_cache_impl()` called
*after* the scan, so a block queued there was first counted as busy and then
had its reference dropped;
- the `_cache_file_readers` map of `CachedRemoteFileReader`;
- the block vector of `BlockFileCacheTtlMgr::reconcile_tablet_blocks()`.
If one of those held the last extra reference, the block ended up stranded:
marked deleting, no longer referenced, but still in `_files`, still in its LRU
queue and still on disk. Nothing ever removed it, because no background thread
looks for deleting blocks — `run_background_gc()` only drains `_recycle_keys`,
which is filled by `remove()`. The block only disappeared if it happened to be
read again (`get_impl()` does not filter deleting cells, so a later holder
release cleans it up), or if eviction or `try_release()` picked it up.
So a synchronous clear could return `status=OK` with
`num_cells_wait_recycle=1` and leave that block in `file_cache_info` and on
disk indefinitely. That is what a regression test caught: no query touched the
table after the clear, and the same 2599-byte TTL block was still there more
than 10 minutes later.
### Release note
[fix](file-cache) Fix file cache blocks that were busy during a clear never
being reclaimed
### What is changed and how it works?
Make the reclaim independent of which code path drops the last reference:
- New `_deleting_blocks` set records every block that is marked deleting
while it is busy, through a new `mark_cell_deleting()` helper used by all such
call sites.
- New `recycle_deleting_blocks()` rechecks those candidates and removes the
ones that became releasable. `run_background_gc()` calls it every round,
bounded by the existing `file_cache_remove_block_qps_limit`, so a stranded
block is now reclaimed within one GC interval (100ms by default) instead of
never. Removal is asynchronous, so the storage is not touched while the cache
lock is held.
- Entries whose cell is already gone (removed through the holder path) are
dropped from the set lazily by the same sweep, so it does not grow.
Two related cleanups, both of which reduce how often a block is treated as
busy in the first place:
- `clear_file_cache_impl()` now drops the pending LRU updates *before* the
scan rather than after it.
- `run_background_block_lru_update()` clears its batch right after applying
it instead of at the top of the next round.
And, since the existing log line made this impossible to diagnose from a BE
log, `clear_file_cache_impl()` now logs the hash (it was missing entirely), use
count and state of the blocks it has to wait for.
Two unit tests cover the sweep: blocks are dropped the way a non-holder
reference holder drops them, and the test asserts that they are then reclaimed,
that still-referenced blocks survive, and that the batch limit is honoured.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [x] No.
- [ ] Yes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [x] Confirm the release note
- [x] Confirm test cases
- [x] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
