github-actions[bot] commented on code in PR #65814:
URL: https://github.com/apache/doris/pull/65814#discussion_r3638709984
##########
be/src/exec/scan/scanner_context.cpp:
##########
@@ -619,6 +623,15 @@ Status
ScannerContext::schedule_scan_task(std::shared_ptr<ScanTask> current_scan
throw doris::Exception(ErrorCode::INTERNAL_ERROR, "Scanner scheduler
logical error.");
}
+ // Once the shared LIMIT is exhausted and no completed or in-flight task
can make progress,
+ // finish the context instead of opening pending scanners only to let them
report EOS.
+ if (_is_shared_scan_limit_exhausted() && _completed_tasks.empty() &&
Review Comment:
[P2] Preserve close accounting on the terminal branch
A scanner can return a non-EOS block while the shared counter is still
positive, then have a peer exhaust the counter before this context consumes
that block. After moving the block, `get_block_from_queue()` calls
`schedule_scan_task(current_scan_task)` with no other work, so this branch
drops a previously used task without another worker EOS.
Worker EOS is the only path that calls `mark_to_need_to_close()` and its
virtual `_collect_profile_before_close()`. `stop_scanners()` only calls
`try_stop()`, and delegate `close()` does not recover that hook. The successful
query can therefore omit final scan CPU/cumulative rows-read/load counters and
scanner-specific reader/cache/condition-profile finalization. Please preserve
the once-only close-accounting transition for the current task and previously
used pending tasks before terminal completion, without opening untouched
pending scanners.
##########
be/src/exec/scan/scanner_context.cpp:
##########
@@ -545,6 +545,10 @@ void ScannerContext::_set_scanner_done() {
_dependency->set_always_ready();
}
+bool ScannerContext::_is_shared_scan_limit_exhausted() const {
+ return limit >= 0 && _shared_scan_limit->load(std::memory_order_acquire)
<= 0;
Review Comment:
[P1] Update the finite-limit unit-test fixture
`ScannerContextTest` initializes its sole `shared_limit` to `-1` at
`be/test/exec/scan/scanner_context_test.cpp:123`, but constructs these contexts
with `limit = 100` and never writes the counter. This predicate therefore
treats every such context as exhausted. For example, the first
`schedule_scan_task()` at test lines 556-558 now returns through the new
terminal branch with `_in_flight_tasks_num == 0`, contradicting the expected
value `1`; the same fixture is reused throughout the file.
Please initialize the shared counter consistently for the existing
finite-limit cases (and add separate explicit zero/negative cases), or
otherwise preserve the fixture's `-1` sentinel contract.
--
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]
