[improvement](be) Add queue time for delete bitmap tasks#65248
Open
bobhan1 wants to merge 1 commit into
Open
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: MOW memtable flush calculates delete bitmap asynchronously, but its log did not show how long the async task waited in the executor queue. The load delete-bitmap executor also used the same thread-pool metric name as the normal delete-bitmap executor, making their metrics hard to distinguish. This change records queue time for flush, segment-level, between-segment, and cloud tablet-level async delete-bitmap tasks, and gives the load delete-bitmap executor a distinct thread-pool name.
### Release note
None
### Check List (For Author)
- Test:
- Build: `./build.sh --be -j 100`
- Code style: `build-support/check-format.sh`
- Behavior changed: No
- Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
There was a problem hiding this comment.
Reviewed PR 65248 against the Doris code-review checkpoints and the BE cloud/storage guides. I did not find a PR-scoped correctness issue that needs an inline comment.
Critical checkpoint conclusions:
- Goal/test proof: the patch achieves the stated observability goal by adding queue-time logging to delete-bitmap async work and separating the load delete-bitmap thread-pool name. The author reports
./build.sh --be -j 100andbuild-support/check-format.sh; I could not rerun build/tests in this runner because.worktree_initialized,thirdparty/installed, andthirdparty/installed/bin/protocare absent. - Scope/focus: the change is small and limited to nine BE storage/cloud files. No additional user-provided review focus points were supplied.
- Concurrency/lifecycle: async task behavior is unchanged apart from capturing
MonotonicMicros()before enqueue and reading the elapsed value inside the worker. Existing token/thread-pool and lock behavior is preserved. - Compatibility/config/session: no storage format, thrift/protocol, config, session variable, or persistence compatibility change was introduced.
- Data correctness and parallel paths: cloud and non-cloud engines both pass distinct executor names; token-based segment and between-segment delete-bitmap paths pass measured queue time, while synchronous/default callers keep
queue_time_us = 0. Delete-bitmap version, sentinel, and update logic is unchanged. - Observability/performance: the added work is a small monotonic timestamp read per submitted task and extra logging fields/log-threshold checks; I did not find a hot-path correctness or lifecycle issue.
- Tests/style: scoped clang-format 16 wrapper check passed for all nine changed files, and scoped
git diff --checkagainst the fetched base/head was clean.
Subagent conclusions:
optimizer-rewrite: no optimizer/Nereids or semantic-equivalence candidate found; final convergence round ended withNO_NEW_VALUABLE_FINDINGS.tests-session-config: no regression-output, session/config, compatibility, or basic CI/style candidate found; final convergence round ended withNO_NEW_VALUABLE_FINDINGS.
Contributor
TPC-H: Total hot run time: 29951 ms |
Contributor
TPC-DS: Total hot run time: 173746 ms |
Contributor
ClickBench: Total hot run time: 25.15 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
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
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.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: MOW memtable flush calculates delete bitmap asynchronously, but its log did not show how long the async task waited in the executor queue. The load delete-bitmap executor also used the same thread-pool metric name as the normal delete-bitmap executor, making their metrics hard to distinguish. This change records queue time for flush, segment-level, between-segment, and cloud tablet-level async delete-bitmap tasks, and gives the load delete-bitmap executor a distinct thread-pool name.
Release note
None
Check List (For Author)
./build.sh --be -j 100build-support/check-format.sh