github-actions[bot] commented on code in PR #64392:
URL: https://github.com/apache/doris/pull/64392#discussion_r3410665593
##########
fe/fe-core/src/main/java/org/apache/doris/common/profile/ExecutionProfile.java:
##########
@@ -156,6 +161,28 @@ protected void setMultiBeProfile(int fragmentId,
TNetworkAddress backendHBAddres
}
}
+ private void markBackendProfileDone(int fragmentId, TNetworkAddress
backendHBAddress) {
+ multiBeProfileLock.writeLock().lock();
+ try {
+ fragmentIdDoneBackends.get(fragmentId).add(backendHBAddress);
+ } finally {
+ multiBeProfileLock.writeLock().unlock();
+ }
Review Comment:
`expectedBackendNum` is never populated for Nereids coordinator profiles, so
this branch makes every Nereids execution profile stay `COLLECTING` and spill
as `INCOMPLETE`. The classic `Coordinator` increments `fragmentIdBeNum` from
`PipelineExecContext`, but the Nereids path builds
`SingleFragmentPipelineTask`s in
`PipelineExecutionTaskBuilder.buildSingleFragmentPipelineTask()` without
calling `ExecutionProfile.addFragmentBackend()`. Those queries still report
done profiles through `QeProcessorImpl.processQueryProfile()`, but
`fragmentIdBeNum` remains 0 for each fragment and `areAllBackendProfilesDone()`
always returns false. The new REST column and RF pruning pollers therefore
never see `COMPLETE` for the default Nereids path. Please populate the expected
backend count from the Nereids worker/fragment assignment as well, and add a
test through that path.
##########
fe/fe-core/src/main/java/org/apache/doris/common/profile/ExecutionProfile.java:
##########
@@ -156,6 +161,28 @@ protected void setMultiBeProfile(int fragmentId,
TNetworkAddress backendHBAddres
}
}
+ private void markBackendProfileDone(int fragmentId, TNetworkAddress
backendHBAddress) {
+ multiBeProfileLock.writeLock().lock();
+ try {
+ fragmentIdDoneBackends.get(fragmentId).add(backendHBAddress);
+ } finally {
+ multiBeProfileLock.writeLock().unlock();
+ }
+ }
+
+ private boolean areAllBackendProfilesDone(int fragmentId, int
expectedBackendNum) {
+ if (expectedBackendNum == 0) {
+ return false;
+ }
+
+ multiBeProfileLock.readLock().lock();
+ try {
+ return fragmentIdDoneBackends.get(fragmentId).size() >=
expectedBackendNum;
+ } finally {
+ multiBeProfileLock.readLock().unlock();
+ }
+ }
+
protected RuntimeProfile getPipelineAggregatedProfile(Map<Integer, String>
planNodeMap) {
RuntimeProfile fragmentsProfile = new RuntimeProfile("Fragments");
for (int i = 0; i < fragmentProfiles.size(); ++i) {
Review Comment:
This marks a backend/fragment as done, but later non-final real-time profile
fetches can still overwrite the final `multiBeProfile` entry above.
`ProfileManager.getProfile()` fetches real-time profiles and feeds them back
with `is_done=false`; the Groovy pollers in this PR now call it even while the
list state is `COLLECTING`. Because profile writes run asynchronously on
`profile-write-pool`, a stale real-time update can run after the final done
update: `fragmentIdDoneBackends` still says the fragment is complete, but
`setMultiBeProfile()` has replaced the final task profiles used by
`getPipelineAggregatedProfile()`. Then `/query_profile` can report `COMPLETE`
while the detail view is built from older profiles and still misses RF pruning
counters. Once a backend/fragment is done, non-done updates for that key should
not replace the stored final profile, or the completion state and merged
profile source need to be updated atomically.
--
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]
