Skip to content

[branch-4.1][fix](profile) guard execution profile access#65442

Open
shuke987 wants to merge 1 commit into
apache:branch-4.1from
shuke987:backport-65353-branch-4.1
Open

[branch-4.1][fix](profile) guard execution profile access#65442
shuke987 wants to merge 1 commit into
apache:branch-4.1from
shuke987:backport-65353-branch-4.1

Conversation

@shuke987

Copy link
Copy Markdown
Collaborator

What

Backport the ProfileManager.getExecutionProfile() read-lock fix from
#65353 to branch-4.1.

Why

queryIdToExecutionProfiles is updated and removed under the profile manager's
write lock. Reads should use the corresponding read lock to keep access
consistent with the map's lock discipline.

Backport Notes

The master PR also marks query_profile/dml_profile_safe.groovy as
nonConcurrent, because that master-only suite runs clean all profile.
branch-4.1 does not contain dml_profile_safe.groovy, and no query_profile
suite on this branch uses clean all profile, so that part is intentionally not
backported.

Validation

  • git diff --check

Local FE build/regression was not run in this manual pick pass.

@shuke987 shuke987 requested a review from yiguolei as a code owner July 10, 2026 03:44
@hello-stephen

Copy link
Copy Markdown
Contributor

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:

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

@shuke987

Copy link
Copy Markdown
Collaborator Author

run buildall

@shuke987

Copy link
Copy Markdown
Collaborator Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review summary for PR 65442 at head 6d50b354a55c53e35fc51eb5c66a9c80343f4687.

No blocking issues or inline findings were identified.

Critical checkpoints:

  • PR scope/head verified with GitHub metadata: one modified file, fe/fe-core/src/main/java/org/apache/doris/common/profile/ProfileManager.java.
  • Existing review context checked: no prior inline review threads or review comments were present, so there were no duplicates to suppress.
  • Lock-discipline review: queryIdToExecutionProfiles is a regular map, and the observed mutation/removal paths use the profile manager write lock. The new getExecutionProfile(TUniqueId) read lock is consistent with those paths and with nearby read-locked access in markExecutionProfileFinished.
  • Caller/lifetime review: QeProcessorImpl.processQueryProfile only looks up the ExecutionProfile and schedules the existing async update. I did not find a new lock-order, reentrancy, or semantic issue introduced by the getter synchronization.
  • Backport/test-scope review: the upstream master PR also changed query_profile/dml_profile_safe.groovy, but that suite is not present on this branch. The branch query_profile suites I checked do not contain clean all profile, matching the PR's backport note.
  • Validation: after fetching the exact base SHA, git diff --check 515030f65abb5fdfd0a66605a29f5f12f42f9b13 6d50b354a55c53e35fc51eb5c66a9c80343f4687 -- fe/fe-core/src/main/java/org/apache/doris/common/profile/ProfileManager.java passed. I did not run FE unit tests or regression suites in this runner.

User focus: no additional user-provided review focus was supplied.

Subagent conclusions:

  • optimizer-rewrite: no candidate findings; no optimizer/rewrite, semantic-equivalence, parallel join, or aggregate-path issue found.
  • tests-session-config: no candidate findings; no regression-test, expected-output, session/config propagation, compatibility, or style issue found.
  • Convergence round 1 ended with both live subagents returning NO_NEW_VALUABLE_FINDINGS for the same final ledger and proposed comment set.

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 100.00% (3/3) 🎉
Increment coverage report
Complete coverage report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants