zhuxiangyi opened a new pull request, #67059:
URL: https://github.com/apache/doris/pull/67059
### What problem does this PR solve?
Issue Number: close #67040
Related PR: #67041
Problem Summary:
Cherry-picked from #67041. The automatic pick failed with conflicts in the
three test files, so this is a manual backport.
`SpillReadDeserializeBlockTime` is always `0` in query profiles on
branch-4.1 — the time spent deserializing spilled blocks is never recorded.
`PipelineXSpillLocalState` registers the counter as
`"SpillReadDeserializeBlockTime"` (`be/src/exec/operator/operator.h`), but
`SpillFileReader` looked it up with a literal that is missing an `s`:
```cpp
// be/src/exec/spill/spill_file_reader.cpp
_deserialize_timer =
custom_profile->get_counter("SpillReadDerializeBlockTime");
// ^^^ Derialize
```
`get_counter()` returns `nullptr` for the unknown name and `ScopedTimer`
returns early on a null counter, so `SCOPED_TIMER(_deserialize_timer)` in
`SpillFileReader::read()` silently measures nothing.
**Difference from the master PR:** #67041 resolves every spill read counter
through the shared `profile::` name constants, but
`be/src/runtime/runtime_profile_counter_names.h` does not exist on branch-4.1.
This backport therefore keeps the minimal spelling fix rather than porting the
constant refactor — that is also what caused the automatic cherry-pick conflict.
Changes:
1. `SpillFileReader` looks the counter up under the name the operator
actually registers.
2. `spill_file_test.cpp` and `spill_repartitioner_test.cpp` registered the
same misspelling by hand, which is why the gap went unnoticed: the reader's
lookup matched in tests while returning null in a real query. They now register
the canonical name.
3. `spillable_operator_test_helper` registered `SpillReadFileTime` and
`SpillReadDeserializeBlockTime` as `TUnit::UNIT`. That stayed harmless only
while the deserialize lookup resolved to null; once it resolves,
`SCOPED_TIMER`'s `DCHECK_EQ(counter->type(), TUnit::TIME_NS)` would fire, so
both are registered as timers.
### Release note
Fix `SpillReadDeserializeBlockTime` always being 0 in query profiles.
### Check List (For Author)
- Test
- [x] Unit Test
Ported `SpillFileTest.ReadDeserializeTimerIsRecorded` from #67041. It
fails before
this change: with the test registering the canonical name, the
reader's misspelled
lookup yields a null counter and the timer stays at 0 after a real
read. The test
also asserts the misspelled name is absent, so it cannot be
reintroduced.
- Behavior changed:
- [x] No.
Profile-only fix. The counter already existed and was already
reported; it was
simply never updated. No counter is added, removed, or renamed.
- Does this need documentation?
- [x] No.
--
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]
