[branch-4.0][fix](file-cache) Make sync clear use safe removal #64578 by gavinchou · Pull Request #64770 · apache/doris

gavinchou · 2026-06-24T04:07:19Z

Backport #64578 to branch-4.0.

Changes:

Preserve the safe removal path for sync file-cache clear.
Adapt the HTTP action path and branch-4.0 file-cache test layout.
Keep branch-4.0 cache storage removal behavior while adding the sync-point hook from the original PR.

Validation:

sh format_code.sh on changed C++ files
git diff --check HEAD~1..HEAD
sh run-be-ut.sh --run --filter "BlockFileCacheTest.clear_file_cache_sync*"

(cherry picked from commit 874a1c4)

hello-stephen · 2026-06-24T04:07:25Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

gavinchou · 2026-06-24T04:07:27Z

run buildall

gavinchou · 2026-06-24T04:07:33Z

/review

github-actions

Requesting changes for cache-clear regressions introduced by the new safe sync-clear path.

Critical checkpoints:

Existing inline review threads: none were present, so these are not duplicates.
User focus: no additional user-provided focus was supplied.
Changed-file sweep: reviewed all seven changed files and relevant cache removal, TTL bookkeeping, storage loading, and HTTP clear paths.
Subagent conclusions: TEST-1 was accepted as M-1; OPT-1 was merged into M-2; OPT-2 was accepted as M-3; OPT-3 was accepted as M-4. Convergence round 2 ended with both live subagents reporting NO_NEW_VALUABLE_FINDINGS for this same final comment set.

github-actions · 2026-06-24T04:22:48Z

 io::UInt128Wrapper hash = io::BlockFileCache::hash(segment_path);
 io::BlockFileCache* cache = io::FileCacheFactory::instance()->get_by_path(hash);
- cache->remove_if_cached(hash);
+ cache->remove_if_cached_async(hash);


The value-scoped clear path still accepts sync=true, but this branch now ignores it and always queues remove_if_cached_async. Before this change, clearing a single value used remove_if_cached, so callers such as op=clear&sync=true&value=... could rely on releasable blocks being physically removed before the HTTP request returned. With the async call the in-memory entry disappears immediately, but the cache file can remain in storage until background GC runs, which is a compatibility regression for the existing sync/value callers. Please preserve the sync flag here, e.g. use remove_if_cached(hash) when sync_clear is true and the async variant otherwise.

github-actions · 2026-06-24T04:22:49Z

 }
 }

- // we cannot delete the element in the loop above, because it will break the iterator


The new sync clear is only walking _files, so it no longer clears cache data that exists on disk but is not currently represented in memory. The old clear_file_cache_directly() called _storage->clear(), and FSFileCacheStorage::clear() sweeps the cache base path and deletes every cache subdirectory. With this replacement, any block that has not been loaded into _files yet, or any leftover cache/tmp directory that is not tracked in _files, survives a successful clear_file_caches(true) response. Please keep a storage-level sweep for the synchronous full clear path, or otherwise delete untracked files before reporting that the cache was cleared.

gavinchou · 2026-06-24T12:26:14Z

run buildall

dismiss

gavinchou · 2026-06-24T12:45:32Z

run buildall

gavinchou · 2026-06-24T13:13:17Z

run buildall

gavinchou · 2026-06-25T08:32:00Z

run buildall

gavinchou · 2026-06-25T08:32:02Z

/review

github-actions

Automated review completed for PR 64770.

Critical checkpoints:

Scope reviewed: all eight changed files plus the related full/value clear, factory, storage remove, holder cleanup, TTL bookkeeping, and HTTP response paths.
Duplicate suppression: existing inline threads already cover the value-scoped sync regression, missing storage-level sweep for untracked files, stale TTL maps, and busy deleting blocks remaining discoverable.
User focus: no additional review focus was provided.
Tests: I did not run BE test binaries in this runner; the local checkout is shallow and does not contain a usable base/parent revision for a full local diff-check range. Review was based on the GitHub PR patch, current head files, and targeted code inspection.

Subagent conclusions:

optimizer-rewrite proposed OR-1; I verified it independently, accepted it as M-1, and submitted it inline.
tests-session-config reported no new valuable findings after reviewing tests, config/session compatibility, HTTP behavior, and style scope.
Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same ledger and proposed final comment set.

gavinchou · 2026-06-25T11:45:46Z

run buildall

…8-branch-4.0

gavinchou · 2026-06-25T12:32:38Z

run buildall

gavinchou · 2026-06-25T12:33:11Z

/review

github-actions

Automated re-review completed for PR 64770 at head 261cc88b6921fbf0a34abddc94477873f5338c1a.

Critical checkpoints:

Scope reviewed: all eight changed files plus the related FileCacheAction, FileCacheFactory, BlockFileCache, FileBlocksHolder, storage removal, TTL bookkeeping, cache lookup, and test paths touched by the sync/async clear changes.
Existing review context: all existing inline threads were treated as already-known issues. I did not submit duplicate comments for the already-raised storage sweep, busy/deleting block visibility, TTL map cleanup, sync remove failure propagation, or value-scoped sync=true compatibility regressions.
New findings from this pass: none distinct enough to submit as a new inline comment.
User focus: .code-review.DkRA02/review_focus.txt says there is no additional user-provided review focus.
Repository instructions: .code-review.DkRA02/review_prompt.txt was followed; no required AGENTS.md files were listed. The repo-local code-review skill path was not present in this checkout after targeted search, so the review followed the available review bundle workflow.
Verification: git diff --check was clean for the PR range. No BE unit-test binaries were present in this checkout, so I did not run the new gtests locally.

Subagent conclusions:

optimizer-rewrite: no new valuable finding beyond existing inline threads.
tests-session-config: no new valuable finding beyond existing inline threads.
Convergence round 1: both live subagents reviewed the same current ledger and zero-inline proposed final comment set and replied NO_NEW_VALUABLE_FINDINGS after the last main-ledger update.

Final opinion: this pass adds no new inline comments. The PR still has blocking cache-clear issues already represented by the existing CHANGES_REQUESTED review threads; this review is submitted as a summary-only comment to avoid duplicating them.

hello-stephen · 2026-06-25T14:58:53Z

BE Regression && UT Coverage Report

Increment line coverage 92.59% (50/54) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	72.97% (26092/35757)
Line Coverage	56.37% (281080/498614)
Region Coverage	54.07% (233965/432710)
Branch Coverage	55.75% (101080/181315)

hello-stephen · 2026-06-25T15:16:29Z

BE Regression && UT Coverage Report

Increment line coverage 92.59% (50/54) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	72.97% (26092/35757)
Line Coverage	56.37% (281080/498614)
Region Coverage	54.07% (233965/432710)
Branch Coverage	55.75% (101080/181315)

hello-stephen · 2026-06-25T16:03:28Z

BE Regression && UT Coverage Report

Increment line coverage 92.59% (50/54) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	72.97% (26093/35757)
Line Coverage	56.38% (281112/498614)
Region Coverage	54.07% (233980/432710)
Branch Coverage	55.76% (101094/181315)

a

[fix](file-cache) Make sync clear use safe removal
c63dca0
(cherry picked from commit 874a1c4)

gavinchou requested a review from morningman as a code owner June 24, 2026 04:07

gavinchou changed the title ~~[branch-4.0][fix](file-cache) Make sync clear use safe removal~~[branch-4.0][fix](file-cache) Make sync clear use safe removal #64578Jun 24, 2026

github-actionsBot previously requested changes Jun 24, 2026
View reviewed changes

Merge branch 'branch-4.0' into gavin-pick-64578-branch-4.0
50ae1c6

Update file_cache_action.cpp
33832db

[fix](file-cache)Fixapache#64578sync clear test on branch-4.0
e1834a7

github-actionsBot previously requested changes Jun 25, 2026
View reviewed changes

Comment threadbe/src/io/cache/block_file_cache.cpp

gavinchou added 2 commits June 25, 2026 20:05

Merge remote-tracking branch 'origin/branch-4.0' into gavin-pick-6457…
34cb17a
…8-branch-4.0

[fix](file-cache)Fixapache#64578after branch-4.0 merge
261cc88

github-actionsBot reviewed Jun 25, 2026
View reviewed changes

morningman approved these changes Jun 26, 2026
View reviewed changes

morningman merged commit 96cb108 into apache:branch-4.0Jun 26, 2026
30 of 32 checks passed

Uh oh!

Conversation

gavinchou commented Jun 24, 2026

Uh oh!

hello-stephen commented Jun 24, 2026

Uh oh!

gavinchou commented Jun 24, 2026

Uh oh!

gavinchou commented Jun 24, 2026

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actionsBotJun 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actionsBotJun 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gavinchou commented Jun 24, 2026

Uh oh!

gavinchou commented Jun 24, 2026

Uh oh!

gavinchou commented Jun 24, 2026

Uh oh!

gavinchou commented Jun 25, 2026

Uh oh!

gavinchou commented Jun 25, 2026

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gavinchou commented Jun 25, 2026

Uh oh!

gavinchou commented Jun 25, 2026

Uh oh!

gavinchou commented Jun 25, 2026

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

hello-stephen commented Jun 25, 2026

BE Regression && UT Coverage Report

Uh oh!

hello-stephen commented Jun 25, 2026

BE Regression && UT Coverage Report

Uh oh!

hello-stephen commented Jun 25, 2026

BE Regression && UT Coverage Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants