[fix](be) Fix file cache queue evict size metrics#64897
Open
deardeng wants to merge 1 commit into
Open
Conversation
deardeng
requested review from
Gabriel39,
gavinchou,
liaoxin01 and
morningman
as code owners
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Collaborator
Author
|
run buildall |
Collaborator
Author
|
/review |
Contributor
TPC-H: Total hot run time: 29730 ms |
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: File cache queue evict size metrics were constructed with literal array indexes, while increments use FileCacheType enum values. Because FileCacheType maps DISPOSABLE to 0 and INDEX to 2, file_cache_index_queue_evict_size actually counted disposable queue evictions, and file_cache_disposable_queue_evict_size counted index queue evictions. This change initializes the metrics array with explicit FileCacheType indexes so each bvar name matches the queue type that increments it.
### Release note
None
### Check List (For Author)
- Test: Manual test
- Ran Doris commit precheck.
- Ran git diff --check and git diff --cached --check.
- Attempted build-support/check-format.sh, but the available clang-format is version 20/18 and the script requires version 16.
- Attempted build-support/run-clang-tidy.sh --build-dir be/build_Release, but the current environment reports missing system header stddef.h and pre-existing clang-tidy errors in included/existing code.
- Behavior changed: Yes. File cache queue evict size bvar metrics now report the queue matching their metric names.
- Does this need documentation: No
Contributor
TPC-DS: Total hot run time: 171798 ms |
deardeng
force-pushed
the
fix-file-cache-metrics
branch
from
e2e5c86 to
c875352
Compare
Collaborator
Author
|
run buildall |
Contributor
ClickBench: Total hot run time: 25.21 s |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed the live PR diff for be/src/io/cache/block_file_cache.cpp against base 2893331370b0f4502acfbf83d71e8d11e13ce624 and head c87535241e61b26ce1f4ee3cf2521b369deed1cd.
The change is focused on file-cache eviction bvar observability: _queue_evict_size_metrics is now constructed with the same FileCacheType indexes used by the remove path, and the helper used at increment time is semantically the same enum-to-index mapping. The four enum values remain DISPOSABLE=0, NORMAL=1, INDEX=2, and TTL=3, so the metric names now line up with the queue whose evictions are counted. I also checked adjacent eviction-by-size/time, self-LRU, LRU recorder, and stats paths and did not find a remaining parallel metric-label issue.
Critical checkpoint conclusions: the PR accomplishes its stated observability fix; the change is small and localized; no new concurrency, lifecycle, config, compatibility, FE/BE protocol, persistence, data write, or optimizer/rewrite behavior is introduced; existing cache tests cover the relevant queue/eviction paths, though I could not run BE tests in this runner because thirdparty/installed is absent. git diff --check on the live PR range passed.
Subagent conclusions: optimizer-rewrite found no applicable optimizer/rewrite surface and no candidates. tests-session-config found no test, session/config, compatibility, or style candidates. Convergence round 1 ended with both live subagents returning NO_NEW_VALUABLE_FINDINGS for the same current ledger/comment set.
User focus: no additional user-provided review focus was supplied.
Contributor
|
Codex automated review failed and did not complete. Error: Codex completed, but no new pull request review was submitted for the current head SHA. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
TPC-H: Total hot run time: 28562 ms |
Contributor
TPC-DS: Total hot run time: 171473 ms |
Contributor
ClickBench: Total hot run time: 25.23 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem Summary: File cache queue evict size metrics were constructed with literal array indexes, while increments use FileCacheType enum values. Because FileCacheType maps DISPOSABLE to 0 and INDEX to 2, file_cache_index_queue_evict_size actually counted disposable queue evictions, and file_cache_disposable_queue_evict_size counted index queue evictions. This change initializes the metrics array with explicit FileCacheType indexes so each bvar name matches the queue type that increments it.
Release note
None
Check List (For Author)
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)