morningman opened a new pull request, #69044:
URL: https://github.com/apache/doris/pull/69044
### What problem does this PR solve?
Issue Number: None
Related PR: #66227, #64392
Problem Summary:
**In short.** A query profile shows a completion state (RUNNING, COLLECTING,
COMPLETE, INCOMPLETE), and regression tests wait for COMPLETE before they read
profile counters. On master the state can be wrong in three ways: a profile
whose BE reports were dropped after a failed spill says COMPLETE, a profile
rendered while it is being spilled can keep COLLECTING for good, and a profile
can say COMPLETE before the last BE report is visible in its per-pipeline
section. This PR ports the fixes for all three from the profile part of
branch-4.1 #66227.
**Background.** #64392 added the completion state on master.
`ExecutionProfile` counts the BEs that sent a final report for each fragment,
and `Profile` derives the state from that: RUNNING until the query finishes,
then COLLECTING until every BE has sent its final report, then COMPLETE. When
`ProfileManager` spills a profile to disk it writes a terminal state instead
(COMPLETE, or INCOMPLETE when reports are still missing), and after the spill
it releases the in-memory execution profiles. The state is printed in the
profile summary (`Profile Completion State: ...`) and in the query profile
list. branch-4.1 got the same mechanism inside #66227, together with the three
fixes below; master never got the fixes.
**The problem, and what it cost.**
| Case | master | with this PR |
|---|---|---|
| The spill fails (for example the profile directory is not writable).
`writeProfileToStorage` still releases the reports, and the next spill writes
the profile again. | The state is computed over an empty report list, so it
shows COMPLETE, and the next spill stores COMPLETE for a profile whose BE
reports are gone. | INCOMPLETE, in memory and in the stored profile. |
| The profile is rendered (`SHOW QUERY PROFILE`, the web UI) while it is
being spilled. Rendering writes the display state into the same summary field
the spill has just set to the terminal state. | Rendered before serialization:
the stored file says COLLECTING. Rendered after serialization but before the
storage path is set: the file says INCOMPLETE, but the profile in memory says
COLLECTING from then on. Either way a poller waiting for a terminal state never
gets one. | The terminal state is frozen before serialization and returned from
then on; a failed write drops the frozen state so a retry recomputes it. |
| A BE sends its final report. `updateProfile` marks the BE done under the
lock and attaches its pipeline nodes after releasing the lock. | A reader that
sees COMPLETE in between can render, or spill, a profile whose per-pipeline
section lacks that report while the merged section already has it. | The nodes
are attached under the same lock, before the BE is marked done. |
**How this PR fixes it.**
- `Profile.releaseMemory()` records that the reports were released; from
then on both the display state and the storage state are INCOMPLETE.
- `Profile.writeToStorage()` freezes the storage state before it serializes
the summary, and `getProfileCompletionState()` returns the frozen state first.
The two state updates are `synchronized`, and a failed write clears the frozen
state.
- `ExecutionProfile.updateMultiBeProfile()` attaches the report's pipeline
nodes to the fragment profile inside the write lock, before it adds the BE to
the done set.
The code is the same as on branch-4.1 and branch-4.2.
**Results.** `ProfileCompletionStateTest` (new) and
`ProfileManagerTest#testFailedSpillRemainsIncompleteAfterMemoryRelease` (new)
cover the failed spill, a failed write followed by a successful one, rendering
paused before and after serialization, the state after reports are released,
multiple pipeline nodes from one BE, and a stale non-final report after the
final one. With the fix, all 35 tests in `ProfileCompletionStateTest` and
`ProfileManagerTest` pass. With master's `Profile` and `ExecutionProfile`,
`testFailedSpillRemainsIncompleteAfterMemoryRelease` gets COMPLETE and
`renderingDuringStoragePreservesTerminalState` gets COLLECTING where both
expect INCOMPLETE; removing only the reset after a failed write makes
`failedStorageDoesNotFreezeCompletionState` get INCOMPLETE where it expects
COLLECTING. The order in which `ExecutionProfile` publishes the nodes is a race
and has no deterministic test.
Original author: @pzhdfy (#66227)
### Release note
A query profile whose BE reports were dropped after a failed spill now shows
`Profile Completion State: INCOMPLETE` instead of COMPLETE.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- `ProfileCompletionStateTest` (new, 4 cases) and
`ProfileManagerTest` (31 cases, 1 new) pass. Against master's `Profile` /
`ExecutionProfile` the failed-spill case and the render-during-spill case fail;
without the reset after a failed write, the failed-write case fails.
- [ ] 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.
- [x] Yes. <!-- Explain the behavior change -->
- A profile whose reports were released after a failed spill reports
INCOMPLETE instead of COMPLETE; a stored profile keeps the terminal state it
was written with.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/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 -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
