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/01 05:23:23 UTC
[PR] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
BiteTheDDDDt opened a new pull request, #61989:
URL: https://github.com/apache/doris/pull/61989
This pull request updates the pipeline task state transition logic to allow certain tasks to skip intermediate states under specific conditions, and adds corresponding test coverage. The main focus is on permitting a task in the BLOCKED state to transition directly to FINISHED if the `_wake_up_early` flag is set, bypassing the RUNNABLE state. Documentation and tests are updated to reflect this new behavior.
### Pipeline task state transition logic:
* Modified `_state_transition` in `pipeline_task.cpp` to allow a task in the BLOCKED state to transition directly to FINISHED when `_wake_up_early` is true, even if this transition is not in the standard legal state transitions table.
### Documentation:
* Updated the state transition diagram in the `PipelineTask` class docstring to document the new possible BLOCKED → FINISHED transition when `_wake_up_early` is set.
### Testing:
* Added tests to verify that BLOCKED → FINISHED is allowed when `_wake_up_early` is true, and that other illegal transitions remain disallowed even with this flag set, in `pipeline_task_test.cpp`. [[1]](diffhunk://#diff-262afd1bf43b83333335fec0b00b65ab0b0241315fd3ceb98c5b3d568971052fR482-R483) [[2]](diffhunk://#diff-262afd1bf43b83333335fec0b00b65ab0b0241315fd3ceb98c5b3d568971052fR492-R502)
* Minor formatting improvement in test case for `_should_trigger_revoking` for consistency.
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4167544475
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4175071471
# BE Regression && UT Coverage Report
Increment line coverage `100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/61989_752e46cc8222c31d117d1b2370542af0f5439a1a_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/61989_752e46cc8222c31d117d1b2370542af0f5439a1a_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.79% (26624/37088) |
| Line Coverage | 54.75% (281219/513686) |
| Region Coverage | 51.86% (233216/449742) |
| Branch Coverage | 53.26% (100689/189036) |
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4174374271
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4167544710
/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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4176136410
# BE Regression && UT Coverage Report
Increment line coverage `100.00% (7/7)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/61989_0c252c4f1644da931f3a1a23ce981f8c3af9f57b_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/61989_0c252c4f1644da931f3a1a23ce981f8c3af9f57b_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.62% (27307/37092) |
| Line Coverage | 57.22% (293973/513737) |
| Region Coverage | 54.35% (244443/449772) |
| Branch Coverage | 56.13% (106117/189068) |
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4175112550
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4175112702
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4175100408
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4168383099
No issues found after reviewing the pipeline task state-transition change. The BLOCKED->FINISHED exception is narrowly scoped to the _wake_up_early race, the delayed dependency wake-up path now safely ignores stale wake-ups after task completion, and the added unit coverage exercises both the special transition and the stale wake-up case.\n\nCheckpoint summary:\n- Goal / correctness: The change addresses the intended wake_up_early race and the updated tests cover the new behavior.\n- Scope: The modification is small and focused to PipelineTask state handling plus targeted tests.\n- Concurrency: The review traced Pipeline::make_all_runnable(), Dependency::set_ready(), PipelineTask::execute(), wake_up(), close(), and finalize(); the new logic is consistent with the existing locking and atomic usage.\n- Lifecycle: No new special lifecycle or static-init risk introduced.\n- Compatibility / config: No compatibility or configuration impact.\n- Parallel paths: The relevant wake-up and cl
ose paths are covered by the change.\n- Conditional checks: The new special-case transition is documented in code and constrained to _wake_up_early.\n- Test coverage: Targeted BE unit tests were added for the new transition and stale wake-up handling.\n- Observability: Existing debug_string/profile info remains sufficient for this change.\n- Transaction / persistence / data writes / FE-BE protocol: Not applicable for this PR.\n- Performance: No material performance concern found in the modified hot path.
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4182287011
# BE Regression && UT Coverage Report
Increment line coverage `100.00% (8/8)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/61989_946a24b4207166e3ce29fb6cf99245c157d05ae0_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/61989_946a24b4207166e3ce29fb6cf99245c157d05ae0_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.78% (26642/37114) |
| Line Coverage | 54.74% (281555/514373) |
| Region Coverage | 51.69% (232900/450536) |
| Branch Coverage | 53.22% (100814/189432) |
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4174251718
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4174315982
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on code in PR #61989:
URL: https://github.com/apache/doris/pull/61989#discussion_r3031450252
##########
be/src/exec/pipeline/pipeline_task.cpp:
##########
@@ -1063,11 +1063,19 @@ Status PipelineTask::_state_transition(State new_state) {
}
_task_profile->add_info_string("TaskState", _to_string(new_state));
_task_profile->add_info_string("BlockedByDependency", _blocked_dep ? _blocked_dep->name() : "");
- if (!LEGAL_STATE_TRANSITION[(int)new_state].contains(_exec_state)) {
+ const auto& table =
+ _wake_up_early ? WAKE_UP_EARLY_LEGAL_STATE_TRANSITION : LEGAL_STATE_TRANSITION;
Review Comment:
就两个
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4174926182
# BE Regression && UT Coverage Report
Increment line coverage `100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/61989_752e46cc8222c31d117d1b2370542af0f5439a1a_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/61989_752e46cc8222c31d117d1b2370542af0f5439a1a_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.80% (26628/37088) |
| Line Coverage | 54.75% (281257/513686) |
| Region Coverage | 51.86% (233258/449742) |
| Branch Coverage | 53.27% (100701/189036) |
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4168298877
/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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #61989:
URL: https://github.com/apache/doris/pull/61989#discussion_r3020501515
##########
be/src/exec/pipeline/pipeline_task.cpp:
##########
@@ -474,6 +474,15 @@ Status PipelineTask::execute(bool* done) {
// If task is woke up early, we should terminate all operators, and this task could be closed immediately.
if (_wake_up_early) {
+ // If the task is in BLOCKED state (e.g., _is_blocked() returned true right before
+ // another thread called make_all_runnable), we must transition back to RUNNABLE first.
+ // This ensures close() sees a legal RUNNABLE→FINISHED transition, and also races with
+ // Dependency::set_ready()'s delayed wake_up() call: wake_up() will see the task is
+ // already RUNNABLE and be handled safely (see wake_up() below).
+ if (_exec_state == State::BLOCKED) {
+ _blocked_dep = nullptr;
Review Comment:
`execute()` now clears `_blocked_dep` and flips `_exec_state` back to `RUNNABLE` without taking the dependency's `_task_lock`, but `Dependency::set_ready()` still invokes `wake_up()` while holding that lock. A concrete interleaving is: (1) the task is blocked on dependency `D`, (2) `set_ready()` swaps `_blocked_task` and enters `wake_up()`, (3) the worker thread executes this block and stores `_blocked_dep = nullptr`, and then (4) the readiness thread still continues down the old `BLOCKED` path and reads `_blocked_dep` in `DCHECK_EQ`. That is a data race on a plain pointer, and in debug/ASAN builds it can still trip the DCHECK. I think this transition needs to stay synchronized with the existing dependency lock protocol instead of mutating `_blocked_dep` here unsafely.
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4168239011
# BE Regression && UT Coverage Report
Increment line coverage `100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/61989_250cf9d665591a090432ba282ba6c469e7eade7c_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/61989_250cf9d665591a090432ba282ba6c469e7eade7c_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.57% (27218/36994) |
| Line Coverage | 57.15% (293011/512723) |
| Region Coverage | 54.37% (243926/448633) |
| Branch Coverage | 56.09% (105854/188726) |
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4168365970
/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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4169593960
# BE Regression && UT Coverage Report
Increment line coverage `100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/61989_4528f261aca144f8de5c38810b2ec44d049c37ad_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/61989_4528f261aca144f8de5c38810b2ec44d049c37ad_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.51% (27262/37088) |
| Line Coverage | 57.13% (293496/513691) |
| Region Coverage | 54.33% (244349/449742) |
| Branch Coverage | 56.02% (105891/189034) |
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4182165938
# BE UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/946a24b4207166e3ce29fb6cf99245c157d05ae0_946a24b4207166e3ce29fb6cf99245c157d05ae0/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/946a24b4207166e3ce29fb6cf99245c157d05ae0_946a24b4207166e3ce29fb6cf99245c157d05ae0/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.96% (20068/37895) |
| Line Coverage | 36.54% (188508/515944) |
| Region Coverage | 32.78% (146345/446398) |
| Branch Coverage | 33.95% (64107/188850) |
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4168298302
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #61989:
URL: https://github.com/apache/doris/pull/61989#discussion_r3019842635
##########
be/src/exec/pipeline/pipeline_task.cpp:
##########
@@ -1063,7 +1063,12 @@ Status PipelineTask::_state_transition(State new_state) {
}
_task_profile->add_info_string("TaskState", _to_string(new_state));
_task_profile->add_info_string("BlockedByDependency", _blocked_dep ? _blocked_dep->name() : "");
- if (!LEGAL_STATE_TRANSITION[(int)new_state].contains(_exec_state)) {
+ bool legal = LEGAL_STATE_TRANSITION[(int)new_state].contains(_exec_state);
+ // When _wake_up_early is true, a BLOCKED task can skip RUNNABLE and go directly to FINISHED.
+ if (!legal && _wake_up_early && _exec_state == State::BLOCKED && new_state == State::FINISHED) {
Review Comment:
This new exception looks unsafe because it legalizes `BLOCKED -> FINISHED` without also making the dependency wake-up path tolerate an already-closed task.
A concrete race is:
1. The worker thread calls `_is_blocked()` / `_wait_to_start()`, so `Dependency::is_blocked_by()` stores the task in `_blocked_task` and sets `_exec_state = BLOCKED`.
2. Another thread calls `set_wake_up_early()` + `unblock_all_dependencies()`. `Dependency::set_ready()` swaps `_blocked_task` into a local list, but has not necessarily called `t->wake_up()` yet.
3. The worker thread returns from `execute()`, sees `_wake_up_early`, sets `done = true`, and `close_task()` now closes/finalizes the task while its state is still `BLOCKED`.
4. The delayed `wake_up()` still runs from `Dependency::set_ready()` and tries to transition the same task back to `RUNNABLE` and resubmit it.
Before this patch, the illegal `BLOCKED -> FINISHED` transition exposed that race immediately. After this patch, the race is only hidden until the later wake-up path. I think the fix needs to coordinate `close()` with pending dependency wake-ups (or make `wake_up()` explicitly ignore finished/finalized tasks), rather than broadening the legal transition table here.
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4181906107
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "HappenLee (via GitHub)" <gi...@apache.org>.
HappenLee commented on code in PR #61989:
URL: https://github.com/apache/doris/pull/61989#discussion_r3031350672
##########
be/src/exec/pipeline/pipeline_task.cpp:
##########
@@ -1063,11 +1063,19 @@ Status PipelineTask::_state_transition(State new_state) {
}
_task_profile->add_info_string("TaskState", _to_string(new_state));
_task_profile->add_info_string("BlockedByDependency", _blocked_dep ? _blocked_dep->name() : "");
- if (!LEGAL_STATE_TRANSITION[(int)new_state].contains(_exec_state)) {
+ const auto& table =
+ _wake_up_early ? WAKE_UP_EARLY_LEGAL_STATE_TRANSITION : LEGAL_STATE_TRANSITION;
+ if (!table[(int)new_state].contains(_exec_state)) {
return Status::InternalError(
"Task state transition from {} to {} is not allowed! Task info: {}",
_to_string(_exec_state), _to_string(new_state), debug_string());
}
+ // FINISHED/FINALIZED → RUNNABLE is legal under wake_up_early (delayed wake_up() arriving
+ // after the task already terminated), but we must not actually move the state backwards.
+ if ((_exec_state == State::FINISHED || _exec_state == State::FINALIZED) &&
+ new_state == State::RUNNABLE) {
+ return Status::OK();
+ }
Review Comment:
这里应该加个else，等于统一用最后的return来返回啊，代码格式得讲究一下
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "HappenLee (via GitHub)" <gi...@apache.org>.
HappenLee commented on code in PR #61989:
URL: https://github.com/apache/doris/pull/61989#discussion_r3031347165
##########
be/src/exec/pipeline/pipeline_task.cpp:
##########
@@ -1063,11 +1063,19 @@ Status PipelineTask::_state_transition(State new_state) {
}
_task_profile->add_info_string("TaskState", _to_string(new_state));
_task_profile->add_info_string("BlockedByDependency", _blocked_dep ? _blocked_dep->name() : "");
- if (!LEGAL_STATE_TRANSITION[(int)new_state].contains(_exec_state)) {
+ const auto& table =
+ _wake_up_early ? WAKE_UP_EARLY_LEGAL_STATE_TRANSITION : LEGAL_STATE_TRANSITION;
Review Comment:
这块有几组啊？能不能后面搞成一个二维数组
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #61989:
URL: https://github.com/apache/doris/pull/61989#discussion_r3019861432
##########
be/src/exec/pipeline/pipeline_task.cpp:
##########
@@ -1063,7 +1063,12 @@ Status PipelineTask::_state_transition(State new_state) {
}
_task_profile->add_info_string("TaskState", _to_string(new_state));
_task_profile->add_info_string("BlockedByDependency", _blocked_dep ? _blocked_dep->name() : "");
- if (!LEGAL_STATE_TRANSITION[(int)new_state].contains(_exec_state)) {
+ bool legal = LEGAL_STATE_TRANSITION[(int)new_state].contains(_exec_state);
+ // When _wake_up_early is true, a BLOCKED task can skip RUNNABLE and go directly to FINISHED.
+ if (!legal && _wake_up_early && _exec_state == State::BLOCKED && new_state == State::FINISHED) {
+ legal = true;
+ }
+ if (!legal) {
Review Comment:
Allowing BLOCKED -> FINISHED in _state_transition means a task can be closed/finalized while it is still registered in Dependency::_blocked_task (because Dependency::set_ready() is what drains that list). If unblock_all_dependencies()/Dependency::set_ready() races and runs after close_task has already transitioned the task to FINISHED/FINALIZED, Dependency::set_ready() will call PipelineTask::wake_up(), which will attempt FINISHED/FINALIZED -> RUNNABLE and throw; since set_ready() doesn’t catch per-task errors, one failing wake_up can abort waking the remaining blocked tasks for that dependency. Consider making wake_up() a no-op when the task is already FINISHED/FINALIZED (especially when _wake_up_early is set), or make Dependency::set_ready() robust to wake_up failures so one finished task can’t prevent waking others.
##########
be/test/exec/pipeline/pipeline_task_test.cpp:
##########
@@ -479,6 +479,8 @@ TEST_F(PipelineTaskTest, TEST_STATE_TRANSITION) {
EXPECT_EQ(task->_exec_state, PipelineTask::State::RUNNABLE);
EXPECT_GT(task->_execution_dependencies.size(), 1);
}
+ // Test static LEGAL_STATE_TRANSITION table (with _wake_up_early = false)
Review Comment:
Minor: the comment says “static LEGAL_STATE_TRANSITION table”, but LEGAL_STATE_TRANSITION is a per-instance const member (not static). Consider rewording to avoid confusion about where the transitions are defined.
```suggestion
// Test LEGAL_STATE_TRANSITION table for this task instance (with _wake_up_early = false)
```
--
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4167543740
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4168361689
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #61989:
URL: https://github.com/apache/doris/pull/61989#issuecomment-4182093302
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] [Bug](pipeline) do not treat BLOCKED->FINISHED as error when wake_up_early is true [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt merged PR #61989:
URL: https://github.com/apache/doris/pull/61989
--
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