github-actions[bot] commented on code in PR #62842:
URL: https://github.com/apache/doris/pull/62842#discussion_r3473972635
##########
be/src/exec/pipeline/pipeline_fragment_context.cpp:
##########
@@ -2562,26 +2559,17 @@ void
PipelineFragmentContext::_coordinator_callback(const ReportStatusRequest& r
Status PipelineFragmentContext::send_report(bool done) {
Status exec_status = _query_ctx->exec_status();
- // If plan is done successfully, but _is_report_success is false,
- // no need to send report.
- // Load will set _is_report_success to true because load wants to know
- // the process.
- if (!_is_report_success && done && exec_status.ok()) {
- return Status::OK();
- }
- // If both _is_report_success and _is_report_on_cancel are false,
- // which means no matter query is success or failed, no report is needed.
- // This may happen when the query limit reached and
- // a internal cancellation being processed
- // When limit is reached the fragment is also cancelled, but
_is_report_on_cancel will
- // be set to false, to avoid sending fault report to FE.
- if (!_is_report_success && !_is_report_on_cancel) {
- if (done) {
- // if done is true, which means the query is finished
successfully, we can safely close the fragment instance without sending report
to FE, and just return OK status here.
+ if (!_is_report_success) {
+ // _is_report_success means this is not a load job, do not need to
report to fe periodically.
+ if (exec_status.is<ErrorCode::LIMIT_REACH>() ||
exec_status.is<ErrorCode::FINISHED>() ||
Review Comment:
This change needs a targeted test for the status-reporting matrix it is
fixing. The branch now suppresses non-load reports for
`LIMIT_REACH`/`FINISHED`/OK and lets ordinary non-OK statuses fall through to
`_coordinator_callback()`, but the PR only changes the implementation/header
and there is no BE or regression coverage for `send_report()` after a
`LIMIT_REACH` cancel versus after a normal task failure. That distinction
already regressed once in this PR in the earlier `done` fast path, so please
add focused coverage that proves `LIMIT_REACH` is suppressed while real
execution errors on final close are still reported to FE.
--
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]
