You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "BiteTheDDDDt (via GitHub)" <gi...@apache.org> on 2026/04/08 02:02:36 UTC
[PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
BiteTheDDDDt opened a new pull request, #62190:
URL: https://github.com/apache/doris/pull/62190
This pull request improves the robustness of the pipeline dependency task wake-up logic by enhancing error handling in the `Dependency::set_ready` method. The main focus is on ensuring that failures during task wake-up do not leave tasks stuck, and that any unexpected exceptions are handled gracefully to prevent potential hangs.
**Error handling and robustness improvements:**
* The `Dependency::set_ready` method is now marked as `noexcept` to indicate it should not throw exceptions. [[1]](diffhunk://#diff-35b4c339dbfd76aad6528e7f75ca7cc6804341bd8d8e2a45779e7695c71a9023L65-R65) [[2]](diffhunk://#diff-76c3907a8cf89f34ce95fe64799c25abee3f4c17676ec7923db3210c5dda0f82L125-R125)
* Added comprehensive try/catch blocks around the logic that wakes up blocked tasks. This ensures:
- Doris-specific exceptions are handled by checking if the task is finalized; if not, the associated query is cancelled to prevent hangs.
- Standard exceptions (e.g., `std::bad_alloc`) also trigger query cancellation and are logged.
- Any other unexpected exceptions are caught to prevent further failures, with a best-effort approach for recovery.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4205543449
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4203452312
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4211540598
# BE Regression && UT Coverage Report
Increment line coverage `88.57% (62/70)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62190_fcc09b0de65662d2c9aa3372bcbb9e349bbc217f_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62190_fcc09b0de65662d2c9aa3372bcbb9e349bbc217f_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.68% (27400/37186) |
| Line Coverage | 57.29% (295633/516029) |
| Region Coverage | 54.44% (245939/451740) |
| Branch Coverage | 56.15% (106691/189997) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4203482065
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4205756336
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #62190:
URL: https://github.com/apache/doris/pull/62190#discussion_r3048817292
##########
be/src/exec/pipeline/dependency.cpp:
##########
@@ -77,9 +77,39 @@ void Dependency::set_ready() {
local_block_task.swap(_blocked_task);
}
for (auto task : local_block_task) {
Review Comment:
In the loop `for (auto task : local_block_task)`, `task` is copied (atomic ref-count adjustments on `weak_ptr`). Iterating by const reference (`for (const auto& task : local_block_task)`) avoids that overhead and is sufficient here since the loop body doesn’t modify the `weak_ptr` itself.
```suggestion
for (const auto& task : local_block_task) {
```
##########
be/src/exec/pipeline/dependency.cpp:
##########
@@ -77,9 +77,39 @@ void Dependency::set_ready() {
local_block_task.swap(_blocked_task);
}
Review Comment:
`set_ready()` is now `noexcept`, but the initial critical section (the `std::unique_lock<std::mutex> lc(_task_lock);` block and the code inside it) is not inside any try/catch. If `std::unique_lock` throws `std::system_error` (or any other unexpected exception escapes from this section), the process will `std::terminate` due to `noexcept`. Consider wrapping the entire function body in a top-level try/catch (similar to the per-task wake-up handling) or otherwise ensuring all potentially-throwing operations are contained so `noexcept` is actually honored.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4211221463
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4211037190
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62190:
URL: https://github.com/apache/doris/pull/62190#discussion_r3048925372
##########
be/src/exec/pipeline/dependency.cpp:
##########
@@ -62,25 +62,42 @@ void Dependency::_add_block_task(std::shared_ptr<PipelineTask> task) {
_blocked_task.push_back(task);
}
-void Dependency::set_ready() {
+void Dependency::set_ready() noexcept {
if (_ready) {
return;
}
- std::vector<std::weak_ptr<PipelineTask>> local_block_task {};
- {
- std::unique_lock<std::mutex> lc(_task_lock);
- if (_ready) {
- return;
- }
- _watcher.stop();
- _ready = true;
- local_block_task.swap(_blocked_task);
- }
- for (auto task : local_block_task) {
- if (auto t = task.lock()) {
+ try {
+ std::vector<std::weak_ptr<PipelineTask>> local_block_task {};
+ {
std::unique_lock<std::mutex> lc(_task_lock);
- THROW_IF_ERROR(t->wake_up(this, lc));
+ if (_ready) {
+ return;
+ }
+ _watcher.stop();
+ _ready = true;
+ local_block_task.swap(_blocked_task);
+ }
+ for (const auto& task : local_block_task) {
+ if (auto t = task.lock()) {
+ std::unique_lock<std::mutex> lc(_task_lock);
+ auto st = t->wake_up(this, lc);
+ if (!st.ok()) {
Review Comment:
`PipelineTask::wake_up()` is not an atomic "enqueue me" operation. It first clears `_blocked_dep` and transitions the task to `RUNNABLE`, and only then calls `submit()` (`pipeline_task.cpp:1047-1055`). If `submit()` returns non-OK here, this new branch only cancels the fragment and returns. The task has already been removed from `_blocked_task`, is no longer blocked on this dependency, and was never inserted into the scheduler queue, so no worker thread will ever call `close_task()` / `decrement_running_task()` for it. `PipelineFragmentContext::cancel()` only calls `unblock_all_dependencies()`, which does not repair that stranded-task state. This turns a scheduler failure into a potential permanent hang/leaked running-task count.
##########
be/src/exec/pipeline/dependency.cpp:
##########
@@ -62,25 +62,42 @@ void Dependency::_add_block_task(std::shared_ptr<PipelineTask> task) {
_blocked_task.push_back(task);
}
-void Dependency::set_ready() {
+void Dependency::set_ready() noexcept {
if (_ready) {
return;
}
- std::vector<std::weak_ptr<PipelineTask>> local_block_task {};
- {
- std::unique_lock<std::mutex> lc(_task_lock);
- if (_ready) {
- return;
- }
- _watcher.stop();
- _ready = true;
- local_block_task.swap(_blocked_task);
- }
- for (auto task : local_block_task) {
- if (auto t = task.lock()) {
+ try {
+ std::vector<std::weak_ptr<PipelineTask>> local_block_task {};
+ {
std::unique_lock<std::mutex> lc(_task_lock);
- THROW_IF_ERROR(t->wake_up(this, lc));
+ if (_ready) {
+ return;
+ }
+ _watcher.stop();
+ _ready = true;
+ local_block_task.swap(_blocked_task);
+ }
+ for (const auto& task : local_block_task) {
+ if (auto t = task.lock()) {
+ std::unique_lock<std::mutex> lc(_task_lock);
+ auto st = t->wake_up(this, lc);
+ if (!st.ok()) {
+ LOG(WARNING) << "Dependency::set_ready(): failed to wake_up task, cancelling "
+ "query. dep="
+ << _name << ", task=" << t->debug_string() << ", status=" << st;
+ if (auto frag = t->fragment_context().lock()) {
+ frag->cancel(Status::InternalError(
+ "wake_up failed in Dependency::set_ready: {}", st.to_string()));
+ }
+ }
+ }
}
+ } catch (const std::exception& e) {
+ // Recovery itself threw (e.g. OOM during logging/cancel). Best effort: nothing more we can do for this task.
Review Comment:
This still aborts the BE process on the exact `std::exception` path the PR description says should be handled gracefully. In particular, `std::bad_alloc` during logging or `frag->cancel()` will now hit `CHECK(false)` and crash the process instead of degrading to a query-local failure. If the intent is truly `noexcept` + best-effort recovery, this branch needs to avoid process termination; otherwise the PR description should not claim graceful handling for standard exceptions.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4204055904
# BE Regression && UT Coverage Report
Increment line coverage `64.29% (18/28)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62190_d0920a8fae2df80180dff2a7e787a952472aff8c_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62190_d0920a8fae2df80180dff2a7e787a952472aff8c_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.52% (27323/37162) |
| Line Coverage | 57.14% (294623/515636) |
| Region Coverage | 54.23% (244736/451276) |
| Branch Coverage | 56.00% (106294/189817) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4207387549
# BE Regression && UT Coverage Report
Increment line coverage `60.61% (40/66)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62190_43853183517966f9b0349b66284a650db8c5d556_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62190_43853183517966f9b0349b66284a650db8c5d556_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.73% (27401/37166) |
| Line Coverage | 57.34% (295690/515677) |
| Region Coverage | 54.49% (245857/451230) |
| Branch Coverage | 56.22% (106714/189828) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62190:
URL: https://github.com/apache/doris/pull/62190#discussion_r3048819492
##########
be/src/exec/pipeline/dependency.cpp:
##########
@@ -77,9 +77,39 @@ void Dependency::set_ready() {
local_block_task.swap(_blocked_task);
}
for (auto task : local_block_task) {
- if (auto t = task.lock()) {
Review Comment:
`t->is_finalized()` is too narrow for the new "benign late wake-up" check. `PipelineTask::close()` sets the state to `FINISHED` before `finalize()` flips it to `FINALIZED` (`pipeline_task.cpp`), so a delayed dependency notification can legitimately hit a task that is already completed but not yet finalized. In that window `wake_up()` returns `Task state transition from FINISHED to RUNNABLE is not allowed`, and this code now cancels the whole fragment even though the task has already finished.
Please treat the pre-finalize `FINISHED` state as completed too, or key off the specific state-transition failure instead of only `is_finalized()`.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt merged PR #62190:
URL: https://github.com/apache/doris/pull/62190
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4203434933
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4203451811
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62190:
URL: https://github.com/apache/doris/pull/62190#discussion_r3050749502
##########
be/src/exec/pipeline/pipeline_task.h:
##########
@@ -118,7 +118,7 @@ class PipelineTask : public std::enable_shared_from_this<PipelineTask> {
return _op_shared_states[id].get();
}
- Status wake_up(Dependency* dep, std::unique_lock<std::mutex>& /* dep_lock */);
+ void wake_up(Dependency* dep, std::unique_lock<std::mutex>& /* dep_lock */);
Review Comment:
This signature change is not fully propagated yet. `be/test/exec/pipeline/pipeline_task_test.cpp:557` still does `EXPECT_TRUE(task->wake_up(dep.get(), lc).ok());`, so the PR no longer compiles because `wake_up()` now returns `void`. Please update that remaining test call site together with the two lines already changed above.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4211221580
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4203314639
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62190:
URL: https://github.com/apache/doris/pull/62190#discussion_r3048903520
##########
be/src/exec/pipeline/dependency.cpp:
##########
@@ -62,25 +62,44 @@ void Dependency::_add_block_task(std::shared_ptr<PipelineTask> task) {
_blocked_task.push_back(task);
}
-void Dependency::set_ready() {
+void Dependency::set_ready() noexcept {
if (_ready) {
return;
}
- std::vector<std::weak_ptr<PipelineTask>> local_block_task {};
- {
- std::unique_lock<std::mutex> lc(_task_lock);
- if (_ready) {
- return;
- }
- _watcher.stop();
- _ready = true;
- local_block_task.swap(_blocked_task);
- }
- for (auto task : local_block_task) {
- if (auto t = task.lock()) {
+ try {
+ std::vector<std::weak_ptr<PipelineTask>> local_block_task {};
+ {
std::unique_lock<std::mutex> lc(_task_lock);
- THROW_IF_ERROR(t->wake_up(this, lc));
+ if (_ready) {
+ return;
+ }
+ _watcher.stop();
+ _ready = true;
+ local_block_task.swap(_blocked_task);
+ }
+ for (const auto& task : local_block_task) {
+ if (auto t = task.lock()) {
+ std::unique_lock<std::mutex> lc(_task_lock);
+ auto st = t->wake_up(this, lc);
+ if (!st.ok()) {
+ LOG(WARNING) << "Dependency::set_ready(): failed to wake_up task, cancelling "
+ "query. dep="
+ << _name << ", task=" << t->debug_string() << ", status=" << st;
+ if (auto frag = t->fragment_context().lock()) {
+ frag->cancel(Status::InternalError(
+ "wake_up failed in Dependency::set_ready: {}", st.to_string()));
+ }
+ }
+ }
}
+ } catch (const std::exception& e) {
+ // Non-Doris exceptions (e.g. std::bad_alloc from scheduler submit path).
+ LOG(WARNING) << "Dependency::set_ready(): unexpected exception during wake_up, "
+ "cancelling query. dep="
Review Comment:
Once `_ready` is set and `_blocked_task` is swapped out, any exception that escapes one iteration aborts the rest of `local_block_task`. The outer catch then only logs and returns. A concrete case is `std::bad_alloc` thrown from `t->wake_up()` or from the logging/cancel code in this loop: the current task is never resubmitted, the dependency is already marked ready, and no later `set_ready()` call will revisit the dropped waiters. That leaves the fragment hanging instead of cancelling it. The exception handling needs to be per task, with fragment cancellation or equivalent unblock logic before continuing to the next waiter.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4203778676
# BE UT Coverage Report
Increment line coverage `57.14% (16/28)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/d0920a8fae2df80180dff2a7e787a952472aff8c_d0920a8fae2df80180dff2a7e787a952472aff8c/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/d0920a8fae2df80180dff2a7e787a952472aff8c_d0920a8fae2df80180dff2a7e787a952472aff8c/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.98% (20105/37947) |
| Line Coverage | 36.53% (188935/517218) |
| Region Coverage | 32.80% (146674/447143) |
| Branch Coverage | 33.93% (64215/189235) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "Thearas (via GitHub)" <gi...@apache.org>.
Thearas commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4203314281
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4203314290
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make Dependency::set_ready() noexcept [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4203433581
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4205513517
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Chore](pipeline) make wake up do not return error [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62190:
URL: https://github.com/apache/doris/pull/62190#issuecomment-4211263637
# BE UT Coverage Report
Increment line coverage `66.67% (8/12)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/fcc09b0de65662d2c9aa3372bcbb9e349bbc217f_fcc09b0de65662d2c9aa3372bcbb9e349bbc217f/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/fcc09b0de65662d2c9aa3372bcbb9e349bbc217f_fcc09b0de65662d2c9aa3372bcbb9e349bbc217f/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.99% (20122/37971) |
| Line Coverage | 36.55% (189162/517611) |
| Region Coverage | 32.80% (146829/447607) |
| Branch Coverage | 33.92% (64253/189415) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org