Uh oh!
There was an error while loading. Please reload this page.
[fix](filecache) add async lru update machanism and fix partial hit in cache reader - #61083
Conversation
Thearas
commented
Mar 5, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
freemandealer
commented
Mar 5, 2026
run buildall |
doris-robot
commented
Mar 5, 2026
TPC-H: Total hot run time: 27941 ms |
doris-robot
commented
Mar 5, 2026
TPC-DS: Total hot run time: 152871 ms |
doris-robot
commented
Mar 5, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 5, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
freemandealer
commented
Mar 11, 2026
run p0 |
freemandealer
commented
Mar 27, 2026
run buildall |
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
doris-robot
commented
Mar 27, 2026
TPC-H: Total hot run time: 26787 ms |
doris-robot
commented
Mar 27, 2026
TPC-DS: Total hot run time: 168987 ms |
doris-robot
commented
Mar 27, 2026
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 27, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…artial hit in cache reader (pick #61083) (#61812) - CachedRemoteFileReader::read_at_impl has incorrect initialization of subsequent traversal start point and count after direct partial hit (causes incorrect fallback / extra overhead) - Proposed LRU ordering async update solution: Decoupled LRU update from query read operations, slightly reducing read lock latency and laying groundwork for subsequent lock splitting - Established performance unit tests for cache lock Signed-off-by: zhengyu <zhangzhengyu@selectdb.com> ### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into --> --------- Signed-off-by: zhengyu <zhangzhengyu@selectdb.com>
freemandealer
commented
Apr 23, 2026
run buildall |
hello-stephen
commented
May 27, 2026
TPC-H: Total hot run time: 32010 ms |
hello-stephen
commented
May 27, 2026
TPC-DS: Total hot run time: 172076 ms |
freemandealer
commented
May 27, 2026
run p0 |
hello-stephen
commented
May 27, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
May 27, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
PR approved by at least one committer and no changes requested. |
freemandealer
commented
May 28, 2026
/review |
There was a problem hiding this comment.
I found two test reliability issues that should be fixed before merging. The direct partial-hit fix itself is covered by a focused unit test, and the async touch implementation is guarded by a new mutable config, but the new tests currently leave global state polluted and one async-LRU assertion is still scheduler dependent.
Critical checkpoint conclusions:
- Goal/test proof: the PR fixes direct partial-hit continuation and adds async LRU touch coverage, but the new tests are not fully deterministic yet.
- Scope: implementation changes are mostly focused, aside from enabling direct cache-file reads by default.
- Concurrency: the changed paths involve cache mutex/background LRU update concurrency; no production deadlock issue was confirmed, but the test reads outcomes via wall-clock scheduling.
- Lifecycle/static initialization: no new cross-TU static initialization hazard found.
- Configuration: a new async-touch config is added and the direct-read default changes; test config restoration is incomplete.
- Compatibility/storage format: no storage/protocol compatibility issue found.
- Parallel paths: direct and indirect cache-read paths were reviewed; no additional missed production path was confirmed.
- Test coverage/results: tests exist, but two reliability issues below need correction.
- Observability: existing metrics/logging appear sufficient for this change.
- Transaction/persistence/data-write correctness: not applicable to this cache-reader/test-focused PR.
- FE/BE variable passing: not applicable.
- Performance: async touch reduces work under the cache mutex when enabled; no blocking performance regression was confirmed.
User focus: no additional user-provided review focus was specified.
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.
…n cache reader (#61083) - CachedRemoteFileReader::read_at_impl has incorrect initialization of subsequent traversal start point and count after direct partial hit (causes incorrect fallback / extra overhead) - Proposed LRU ordering async update solution: Decoupled LRU update from query read operations, slightly reducing read lock latency and laying groundwork for subsequent lock splitting - Established performance unit tests for cache lock Signed-off-by: zhengyu <zhangzhengyu@selectdb.com>
…n cache reader (apache#61083) - CachedRemoteFileReader::read_at_impl has incorrect initialization of subsequent traversal start point and count after direct partial hit (causes incorrect fallback / extra overhead) - Proposed LRU ordering async update solution: Decoupled LRU update from query read operations, slightly reducing read lock latency and laying groundwork for subsequent lock splitting - Established performance unit tests for cache lock Signed-off-by: zhengyu <zhangzhengyu@selectdb.com>
…artial hit in cache reader(pick#61083) (#64234) Original PR: #61083 Picked to: branch-4.1 Pick branch: freemandealer:pick-branch-4.1-pr-61083 Validation: - git diff --check - build-support/check-format.sh with clang-format 16 Notes: The first two original commits were already present in branch-4.1 and were skipped as empty; picked the remaining commits.
### What problem does this PR solve? Issue Number: None Related PR: #61083 Problem Summary: The branch-4.0 pick of #61083 added an mtime() override to the mock file reader used by cached_remote_file_reader_lock_wait_test.cpp. branch-4.0 FileReader does not define mtime(), so BE UT compilation fails with only virtual member functions can be marked override. Remove the branch-incompatible mock method so the test matches the branch-4.0 FileReader interface and the BE UT target compiles. ### Release note None ### Check List (For Author) - Test: Unit Test - DORIS_TOOLCHAIN=clang DISABLE_BE_JAVA_EXTENSIONS=ON ENABLE_INJECTION_POINT=ON ENABLE_CACHE_LOCK_DEBUG=0 ENABLE_PCH=0 sh run-be-ut.sh --run --filter=CachedRemoteFileReaderLockWaitTest.AsyncTouchOnGetOrSetDefersLruUpdate - build-support/check-format.sh with clang-format 16 - git diff --check - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: #61083 Problem Summary: The branch-4.0 pick of #61083 added an mtime() override to the mock file reader used by cached_remote_file_reader_lock_wait_test.cpp. branch-4.0 FileReader does not define mtime(), so BE UT compilation fails with only virtual member functions can be marked override. Remove the branch-incompatible mock method so the test matches the branch-4.0 FileReader interface and the BE UT target compiles. ### Release note None ### Check List (For Author) - Test: Unit Test - DORIS_TOOLCHAIN=clang DISABLE_BE_JAVA_EXTENSIONS=ON ENABLE_INJECTION_POINT=ON ENABLE_CACHE_LOCK_DEBUG=0 ENABLE_PCH=0 sh run-be-ut.sh --run --filter=CachedRemoteFileReaderLockWaitTest.AsyncTouchOnGetOrSetDefersLruUpdate - build-support/check-format.sh with clang-format 16 - git diff --check - Behavior changed: No - Does this need documentation: No
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)