Uh oh!
There was an error while loading. Please reload this page.
[fix](packed-file) Fix packed file cache cleanup issue - #59892
Conversation
hello-stephen
commented
Jan 14, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
liaoxin01
commented
Jan 14, 2026
run buildall |
There was a problem hiding this comment.
Pull request overview
This PR fixes a packed file cache cleanup issue by moving the cache layer from the inner reader to the PackedFileReader wrapper level. Previously, file cache used the packed file path as the cache key, which caused cache entries to become orphaned when stale rowsets were cleaned up (cleanup uses segment path as key).
Changes:
- Modified cache handling in
PackedFileSystem::open_file_impl()to disable caching at the inner reader level and wrapPackedFileReaderwithCachedRemoteFileReaderusing segment path and size - Added asynchronous cache write functions (
do_write_to_file_cacheandwrite_small_file_to_cache_async) to write small files to cache during packed file creation - Changed packed file writer options to disable
write_file_cacheflag, as individual small files are now cached separately - Updated logging from
LOG(INFO)toVLOG_DEBUGfor less verbose output
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| be/src/io/fs/packed_file_system.cpp | Moved cache layer to wrap PackedFileReader instead of inner reader; ensures cache key uses segment path and cache size uses segment size |
| be/src/io/fs/packed_file_manager.cpp | Added async cache writing for small files during packed file creation; disabled write_file_cache for packed files themselves |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
20954cd to
96a95e4Compareliaoxin01
commented
Jan 14, 2026
run buildall |
doris-robot
commented
Jan 14, 2026
TPC-H: Total hot run time: 30806 ms |
doris-robot
commented
Jan 14, 2026
TPC-DS: Total hot run time: 172662 ms |
doris-robot
commented
Jan 14, 2026
ClickBench: Total hot run time: 26.64 s |
dataroaring
commented
Jan 14, 2026
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
Previously, file cache used packed file path as cache key, which caused cache entries to be orphaned when stale rowsets were cleaned up (cleanup uses segment path as key). This fix moves the cache layer from inner reader to PackedFileReader wrapper level, ensuring: 1. Cache key = hash(segment_path.filename()) - matches cleanup key 2. Cache size = segment size - correct boundary 3. Each segment has independent cache entry - no interference
96a95e4 to
2cdcfe2Comparedataroaring
commented
Jan 15, 2026
run buildall |
doris-robot
commented
Jan 15, 2026
TPC-H: Total hot run time: 31129 ms |
doris-robot
commented
Jan 15, 2026
TPC-DS: Total hot run time: 174229 ms |
doris-robot
commented
Jan 15, 2026
ClickBench: Total hot run time: 26.97 s |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
liaoxin01
commented
Jan 16, 2026
run buildall |
doris-robot
commented
Jan 16, 2026
TPC-H: Total hot run time: 32236 ms |
doris-robot
commented
Jan 16, 2026
TPC-DS: Total hot run time: 178301 ms |
doris-robot
commented
Jan 16, 2026
ClickBench: Total hot run time: 27.3 s |
liaoxin01
commented
Jan 16, 2026
run buildall |
liaoxin01
commented
Jan 16, 2026
run buildall |
doris-robot
commented
Jan 16, 2026
TPC-H: Total hot run time: 31795 ms |
doris-robot
commented
Jan 16, 2026
TPC-DS: Total hot run time: 174472 ms |
doris-robot
commented
Jan 16, 2026
ClickBench: Total hot run time: 26.67 s |
hello-stephen
commented
Jan 16, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Jan 16, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
Uh oh!
There was an error while loading. Please reload this page.
Previously, file cache used packed file path as cache key, which caused cache entries to be orphaned when stale rowsets were cleaned up (cleanup uses segment path as key). This fix moves the cache layer from inner reader to PackedFileReader wrapper level, ensuring: 1. Cache key = hash(segment_path.filename()) - matches cleanup key 2. Cache size = segment size - correct boundary 3. Each segment has independent cache entry - no interference
…pache#59892) Previously, file cache used packed file path as cache key, which caused cache entries to be orphaned when stale rowsets were cleaned up (cleanup uses segment path as key). This fix moves the cache layer from inner reader to PackedFileReader wrapper level, ensuring: 1. Cache key = hash(segment_path.filename()) - matches cleanup key 2. Cache size = segment size - correct boundary 3. Each segment has independent cache entry - no interference
…ting small file to file cache (#65701) ### What problem does this PR solve? Issue Number: DORIS-27148 When a packed small file is written to the file cache, `do_write_to_file_cache()` did not set `ctx.tablet_id`, so the cache block meta was persisted with `tablet_id=0`. As a result: 1. `information_schema.file_cache_info` shows these blocks under `tablet_id=0`, and they cannot be queried by the real tablet id. 2. TTL blocks are not registered to `BlockFileCacheTtlMgr` (registration is skipped when `context.tablet_id == 0`), so the TTL expiration check cannot evict them by tablet. This was introduced when picking #59892 to this branch: the pick dropped the `ctx.tablet_id = tablet_id;` line that exists on master. This PR restores it, aligning with master. Note: existing cache meta already persisted with `tablet_id=0` will not be rewritten automatically; it is corrected only when the block is evicted and cached again. ### Release note Fix that file cache blocks written via the packed-file path were recorded with tablet_id=0 in information_schema.file_cache_info. ### Check List (For Author) - Test - [x] Manual test (add detailed scripts or steps below) - Verified per DORIS-27148: write small segments that go through the packed-file path, then query `information_schema.file_cache_info` by the real tablet ids and confirm blocks are no longer recorded under tablet_id=0. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Previously, file cache used packed file path as cache key, which caused cache entries to be orphaned when stale rowsets were cleaned up (cleanup uses segment path as key).
This fix moves the cache layer from inner reader to PackedFileReader wrapper level, ensuring:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)