(doris) branch branch-4.0 updated: [fix](profile) fix missing update_scanner_profile on non-eos scanner yield path (#62647)

[yi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

yiguolei pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/branch-4.0 by this push:
     new 69e3104abe2 [fix](profile) fix missing update_scanner_profile on non-eos scanner yield path (#62647)
69e3104abe2 is described below

commit 69e3104abe20c7763eb989f503b318f36f39e506
Author: Pxl <xl...@selectdb.com>
AuthorDate: Thu May 7 15:20:22 2026 +0800

    [fix](profile) fix missing update_scanner_profile on non-eos scanner yield path (#62647)
    
    ## Proposed changes
    
    `update_scanner_profile()` was only called when `eos==true`. On non-eos
    yield paths (e.g. scanner reaches `max_run_time_ms`, memory pressure,
    etc.), the CPU timer and realtime counters were never updated for that
    execution round.
    
    ### Bug 1: CPU timer permanently undercounted
    Since `start_scan_cpu_timer()` resets `_cpu_watch` at the start of each
    round, CPU time from all non-final rounds was permanently lost from
    `_scan_cpu_timer` and `resource_ctx cpu_cost_ms`.
    
    ### Bug 2: Realtime counters update delayed
    `update_realtime_counters()` was not called on yield, delaying workload
    group policy enforcement that depends on scan bytes/rows.
    
    ### Root cause
    This bug was introduced by #61064, which removed
    `update_scanner_profile()` from the Defer lambda (it was previously
    called unconditionally in Defer). The removal was intended to fix the
    wait worker timer counting, but inadvertently broke CPU timer
    accumulation and realtime counter updates on non-eos paths.
    
    ### Master status
    Master has already been fixed by #62258 which restructured the code to
    call `update_scanner_profile()` unconditionally before the eos check.
    This PR applies an equivalent fix.
    
    ### Fix
    Split profile-update responsibility between two mutually exclusive exit
    paths:
    
    - **non-eos yield with `status.ok()`**: the `Defer` lambda calls
    `update_scanner_profile()` inside the same branch that calls
    `start_wait_worker_timer()`, so the yielding round's CPU / realtime
    counters are flushed before the scanner is re-scheduled.
    - **eos or error path**: the existing explicit
    `update_scanner_profile()` call in the eos block at the end of the
    function flushes the counters before `mark_to_need_to_close()`.
    
    The two paths cannot both fire (`eos==true` short-circuits the Defer
    branch), so no dedup guard is needed.
    
    The procedural comment block above the timers is updated to document
    where `update_scan_cpu_timer` lives on each exit path.
    
    ## Related PRs
    - Bug introduced by: #61064 (cherry-picked as #61691 to branch-4.0)
    - Master fix: #62258
    - branch-4.1 fix: #62646
    - branch-4.0 fix: #62647
    
    Co-authored-by: Copilot <22...@users.noreply.github.com>
---
 be/src/vec/exec/scan/scanner_scheduler.cpp | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/be/src/vec/exec/scan/scanner_scheduler.cpp b/be/src/vec/exec/scan/scanner_scheduler.cpp
index 8582089b102..7ba373c592e 100644
--- a/be/src/vec/exec/scan/scanner_scheduler.cpp
+++ b/be/src/vec/exec/scan/scanner_scheduler.cpp
@@ -157,7 +157,9 @@ void ScannerScheduler::_scanner_scan(std::shared_ptr<ScannerContext> ctx,
     // we set and get counter according below order, to make sure the counter is updated before get_block, and the time of get_block is recorded in the counter.
     // 1. update_wait_worker_timer to make sure the time of waiting for worker thread is recorded in the timer
     // 2. start_scan_cpu_timer to make sure the cpu timer include the time of open and get_block, which is the real cpu time of scanner
-    // 3. update_scan_cpu_timer when defer, to make sure the cpu timer include the time of open and get_block, which is the real cpu time of scanner
+    // 3. update_scan_cpu_timer at exit (in the Defer below for the non-eos yield path,
+    //    in the explicit eos block at the end of this function for the eos / error path),
+    //    to make sure the cpu timer includes the time of open and get_block, which is the real cpu time of scanner
     // 4. start_wait_worker_timer when defer, to make sure the time of waiting for worker thread is recorded in the timer
 
     MonotonicStopWatch max_run_time_watch;
@@ -165,21 +167,20 @@ void ScannerScheduler::_scanner_scan(std::shared_ptr<ScannerContext> ctx,
     scanner->update_wait_worker_timer();
     scanner->start_scan_cpu_timer();
 
-    bool need_update_profile = true;
     auto update_scanner_profile = [&]() {
-        if (need_update_profile) {
-            scanner->update_scan_cpu_timer();
-            scanner->update_realtime_counters();
-            need_update_profile = false;
-        }
+        scanner->update_scan_cpu_timer();
+        scanner->update_realtime_counters();
     };
 
     Status status = Status::OK();
     bool eos = false;
     Defer defer_scanner([&] {
         if (status.ok() && !eos) {
-            // if status is not ok, it means the scanner is failed, and the counter may be not updated correctly, so no need to update counter again. if eos is true, it means the scanner is finished successfully, and the counter is updated correctly, so no need to update counter again.
+            // Only start wait worker timer when the scanner will be re-submitted to the thread
+            // pool. When status is not ok or eos is true, the scanner is done and won't be
+            // re-scheduled, so there is no "waiting for worker" phase to track.
             scanner->start_wait_worker_timer();
+            update_scanner_profile();
         }
     });
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org