github-actions[bot] commented on code in PR #64614:
URL: https://github.com/apache/doris/pull/64614#discussion_r3471533521


##########
be/src/exec/sink/load_stream_stub.cpp:
##########
@@ -143,6 +142,24 @@ LoadStreamStub::~LoadStreamStub() {
     }
 }
 
+void LoadStreamStub::append_load_stream_profile(const TRuntimeProfileTree& 
profile) {
+    std::lock_guard<bthread::Mutex> lock(_load_stream_profile_mutex);
+    _load_stream_profile.update(profile);

Review Comment:
   This stores the remote thrift profile by first importing it into a fresh 
`RuntimeProfile`, but `RuntimeProfile::update()` does not preserve the incoming 
`TCounter.level` when it creates a missing counter; it calls `new 
Counter(tcounter.type, tcounter.value)`, whose default level is 3. When 
`_close_wait()` later calls 
`collect_load_stream_profile(_state->profile_level())`, level-2 reporting 
prunes those imported counters, so the final profile can still contain the 
`DeltaWriterV2`/`MemTableWriter` child names while losing their actual timers 
such as `WriteMemTableTime` and `MemTableSortTime`. Please preserve 
`tcounter.level` when importing, or keep the received tree until the final 
merge without this level-dropping round trip; the tests should assert at least 
one concrete writer counter rather than only child names.



-- 
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]

Reply via email to