zhuxiangyi opened a new pull request, #67041:
URL: https://github.com/apache/doris/pull/67041
### What problem does this PR solve?
Issue Number: close #67040
Problem Summary:
`SpillReadDeserializeBlockTime` is always `0` in query profiles — the time
spent deserializing spilled blocks has never been recorded.
The counter is registered by `SpillReadCounters::init` under the shared
constant `profile::SPILL_READ_DESERIALIZE_BLOCK_TIME`, whose value is
`"SpillReadDeserializeBlockTime"`:
```cpp
// be/src/exec/operator/spill_counters.h
spill_read_deserialize_block_timer =
ADD_TIMER_WITH_LEVEL(profile,
profile::SPILL_READ_DESERIALIZE_BLOCK_TIME, 1);
```
But `SpillFileReader` looked it up with a hand-written 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.
**Why existing tests did not catch it:** `spill_file_test.cpp` and
`spill_repartitioner_test.cpp` registered the counters by hand and copied the
same misspelling, so the reader's lookup resolved in tests while returning null
in a real query.
Changes:
1. `SpillFileReader` now resolves every read counter through the `profile::`
name constants instead of string literals, and `DCHECK`s that each one
resolves, so a future rename fails loudly instead of silently dropping a
counter.
2. The unit tests reuse `SpillWriteCounters::init` /
`SpillReadCounters::init` rather than re-listing the names, so the registered
names and units cannot drift from production again.
3. `spillable_operator_test_helper` registered `SpillReadFileTime` and
`SpillReadDeserializeBlockTime` as `TUnit::UNIT` instead of timers. That was
harmless while the timer resolved to null, but trips the
`DCHECK_EQ(counter->type(), TUnit::TIME_NS)` inside `ScopedTimer` once it
resolves, so it is corrected by the same reuse.
### Release note
Fix `SpillReadDeserializeBlockTime` always being 0 in query profiles.
### Check List (For Author)
- Test
- [x] Unit Test
Added `SpillFileTest.ReadDeserializeTimerIsRecorded`, which 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]
