github-actions[bot] commented on code in PR #62842:
URL: https://github.com/apache/doris/pull/62842#discussion_r3502735323
##########
be/src/common/status.h:
##########
@@ -527,7 +527,7 @@ class [[nodiscard]] Status {
void set_code(int code) { _code = code; }
- bool ok() const { return _code == ErrorCode::OK || _code ==
ErrorCode::FINISHED; }
+ bool ok() const { return _code == ErrorCode::OK; }
Review Comment:
`FINISHED` is used by FE as a normal teardown signal after the coordinator
is done (`AbstractJobProcessor.tryFinishSchedule()` calls
`cancelExecute(Status.FINISHED)`, and the RPC sends it as `cancel_status`).
After this change `QueryContext::is_cancelled()` becomes true for that status,
so any task that wakes up from the cancel path enters
`TaskScheduler::_do_work()`'s canceled branch with `exec_status == FINISHED`.
`close_task()` still treats every non-OK status as a failure, calls
`ctx->cancel(exec_status)`, and logs `Pipeline task failed... reason:
[FINISHED]...`. The per-instance warning in `PipelineFragmentContext::cancel()`
was special-cased for FINISHED, but this scheduler warning remains, so normal
query completion can still be reported in BE logs as task failures. Please
handle `ErrorCode::FINISHED` explicitly in the scheduler close/failure
predicate, or split "should stop work" from "task failed", and cover the FE
FINISHED cancellation path in the focused status-report t
ests.
--
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]
