Repository navigation
[fix](executor) Fix rare self-deadlock that can cause the time-sharing task executor to hang. - #58273
Merged
morningman merged 2 commits intoJan 13, 2026
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run nonConcurrent |
kaka11chen
force-pushed
the
time_sharing_task_executor_timeout_test
branch
from
November 24, 2025 03:30
90c5711 to
4fab545
Compare
Contributor
Author
|
run nonConcurrent |
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 34076 ms |
ClickBench: Total hot run time: 27.91 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run nonConcurrent |
7 similar comments
Contributor
Author
|
run nonConcurrent |
Contributor
Author
|
run nonConcurrent |
Contributor
Author
|
run nonConcurrent |
Contributor
Author
|
run nonConcurrent |
Contributor
Author
|
run nonConcurrent |
Contributor
Author
|
run nonConcurrent |
Contributor
Author
|
run nonConcurrent |
kaka11chen
force-pushed
the
time_sharing_task_executor_timeout_test
branch
from
December 8, 2025 11:17
4fab545 to
d3e7247
Compare
Contributor
Author
|
run buildall |
kaka11chen
force-pushed
the
time_sharing_task_executor_timeout_test
branch
from
December 8, 2025 11:51
d3e7247 to
f3cc40b
Compare
Contributor
Author
|
run buildall |
TPC-H: Total hot run time: 35954 ms |
TPC-DS: Total hot run time: 186512 ms |
ClickBench: Total hot run time: 27.2 s |
Contributor
Author
|
run nonConcurrent |
kaka11chen
force-pushed
the
time_sharing_task_executor_timeout_test
branch
from
December 9, 2025 04:26
f3cc40b to
8651d73
Compare
Contributor
Author
|
run buildall |
kaka11chen
force-pushed
the
time_sharing_task_executor_timeout_test
branch
from
December 9, 2025 06:33
8651d73 to
79a2525
Compare
Contributor
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run nonConcurrent |
suxiaogang223
approved these changes
Jan 13, 2026
kaka11chen
changed the title
github-actions Bot
pushed a commit
that referenced
this pull request
Jan 13, 2026
…g task executor to hang. (#58273) ### What problem does this PR solve? ### Release note #### Summary 1. Fix a self-deadlock that can occur in `_dispatch_thread` when a destructor path attempts to re-acquire the same mutex already held by the thread. The root cause is destructors (triggered while holding `_lock`) performing operations that try to re-acquire the same mutex. A safe fix should ensure destructors that may call `remove_task()` run outside the `_lock` scope or avoid re-locking the same mutex inside destructor paths. 2. Call `_task_executor->wait()` in `TaskExecutorSimplifiedScanScheduler::stop()`. --- #### Details / Reproduction steps 1. `std::shared_ptr<PrioritizedSplitRunner> split = _tokenless->_entries->take();` 3. `l.lock();` — `_dispatch_thread` acquires `_lock`. 4. After the `while` loop finishes, `split` goes out of scope and the `shared_ptr` is destroyed. 5. `PrioritizedSplitRunner` destructor runs → destroys `_split_runner` (`ScannerSplitRunner`). 6. `ScannerSplitRunner::_scan_func` destructor runs → destroys captured `ctx` (`std::shared_ptr<ScannerContext>`). 7. `ScannerContext::~ScannerContext()` calls `remove_task()`. 8. `remove_task()` attempts to acquire `_lock`. 9. Result: **self-deadlock** because `_lock` is already held by `_dispatch_thread`. --- #### Solution We must explicitly release 'split' BEFORE acquiring _lock to avoid self-deadlock. The destructor chain (`PrioritizedSplitRunner` -> `ScannerSplitRunner`-> `_scan_func` lambda -> captured `ScannerContext`) may call `remove_task()` which tries to acquire `_lock`. Since `_lock` is not a recursive mutex, this would deadlock.
zzzxl1993
pushed a commit
to zzzxl1993/doris
that referenced
this pull request
Jan 13, 2026
…g task executor to hang. (apache#58273) ### What problem does this PR solve? ### Release note #### Summary 1. Fix a self-deadlock that can occur in `_dispatch_thread` when a destructor path attempts to re-acquire the same mutex already held by the thread. The root cause is destructors (triggered while holding `_lock`) performing operations that try to re-acquire the same mutex. A safe fix should ensure destructors that may call `remove_task()` run outside the `_lock` scope or avoid re-locking the same mutex inside destructor paths. 2. Call `_task_executor->wait()` in `TaskExecutorSimplifiedScanScheduler::stop()`. --- #### Details / Reproduction steps 1. `std::shared_ptr<PrioritizedSplitRunner> split = _tokenless->_entries->take();` 3. `l.lock();` — `_dispatch_thread` acquires `_lock`. 4. After the `while` loop finishes, `split` goes out of scope and the `shared_ptr` is destroyed. 5. `PrioritizedSplitRunner` destructor runs → destroys `_split_runner` (`ScannerSplitRunner`). 6. `ScannerSplitRunner::_scan_func` destructor runs → destroys captured `ctx` (`std::shared_ptr<ScannerContext>`). 7. `ScannerContext::~ScannerContext()` calls `remove_task()`. 8. `remove_task()` attempts to acquire `_lock`. 9. Result: **self-deadlock** because `_lock` is already held by `_dispatch_thread`. --- #### Solution We must explicitly release 'split' BEFORE acquiring _lock to avoid self-deadlock. The destructor chain (`PrioritizedSplitRunner` -> `ScannerSplitRunner`-> `_scan_func` lambda -> captured `ScannerContext`) may call `remove_task()` which tries to acquire `_lock`. Since `_lock` is not a recursive mutex, this would deadlock.
kaka11chen
added a commit
to kaka11chen/doris
that referenced
this pull request
Jan 21, 2026
…g task executor to hang. (apache#58273) 1. Fix a self-deadlock that can occur in `_dispatch_thread` when a destructor path attempts to re-acquire the same mutex already held by the thread. The root cause is destructors (triggered while holding `_lock`) performing operations that try to re-acquire the same mutex. A safe fix should ensure destructors that may call `remove_task()` run outside the `_lock` scope or avoid re-locking the same mutex inside destructor paths. 2. Call `_task_executor->wait()` in `TaskExecutorSimplifiedScanScheduler::stop()`. --- 1. `std::shared_ptr<PrioritizedSplitRunner> split = _tokenless->_entries->take();` 3. `l.lock();` — `_dispatch_thread` acquires `_lock`. 4. After the `while` loop finishes, `split` goes out of scope and the `shared_ptr` is destroyed. 5. `PrioritizedSplitRunner` destructor runs → destroys `_split_runner` (`ScannerSplitRunner`). 6. `ScannerSplitRunner::_scan_func` destructor runs → destroys captured `ctx` (`std::shared_ptr<ScannerContext>`). 7. `ScannerContext::~ScannerContext()` calls `remove_task()`. 8. `remove_task()` attempts to acquire `_lock`. 9. Result: **self-deadlock** because `_lock` is already held by `_dispatch_thread`. --- We must explicitly release 'split' BEFORE acquiring _lock to avoid self-deadlock. The destructor chain (`PrioritizedSplitRunner` -> `ScannerSplitRunner`-> `_scan_func` lambda -> captured `ScannerContext`) may call `remove_task()` which tries to acquire `_lock`. Since `_lock` is not a recursive mutex, this would deadlock.
ybtsdst
pushed a commit
to ybtsdst/doris
that referenced
this pull request
Feb 27, 2026
… time-sharing task executor to hang. (apache#60089) bp apache#58273
mrhhsg
added a commit
to mrhhsg/doris
that referenced
this pull request
Oct 9, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#51690, apache#58144, apache#58273, apache#65814, apache#67070 Problem Summary: Workload groups pick the scan scheduler for internal and external table scans from `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table`. Both configs currently default to `true` on master, so every workload group creates `TaskExecutorSimplifiedScanScheduler` (the time-sharing task executor) for local (`ls_*`) and remote (`rs_*`) scans. The time-sharing executor is still maturing (for example the self-deadlock fixed in apache#58273; the internal-table default was already reverted once in apache#58144), so the default should be the `ThreadPoolSimplifiedScanScheduler` for both internal and external tables. This change sets both configs' default values to `false`. The task executor can still be enabled explicitly through `be.conf`. Making the thread pool path the default exposed one scheduling gap between the two admission paths. apache#65814 stops the TaskExecutor path (`ScannerContext::_pull_next_scan_task`) from submitting pending scanners once the shared scan LIMIT is exhausted while a completed or in-flight task can still wake the operator. The thread pool admission rewritten in apache#67070 (`ScannerContext::can_admit_scan_task`) does not apply this rule, so for a `SELECT ... LIMIT n` over many tablets every remaining pending scanner would still be admitted and run `prepare()`/`open()` (tablet reader init) only to report EOS right away. The same rule is now applied in `can_admit_scan_task`, including the existing escape that admits one pending scanner when nothing else can wake the pipeline task. ### Release note The BE configs `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table` now default to `false`; internal and external table scans use the thread pool scan scheduler by default. ### Check List (For Author) - Test: - Unit Test: added `WorkloadGroupManagerTest.DefaultScanSchedulersUseThreadPool` (registered defaults are `false` and the workload group creates thread pool schedulers for local and remote scans) and `ScannerContextTest.thread_pool_admission_stops_after_shared_limit` (fails without the admission fix); ran `WorkloadGroupManagerTest.*`, `ScannerContextTest.*` and other scanner/scheduler/task-executor UTs - Regression test: ran existing suites on a local cluster with the new default: correctness_p0/test_shared_scan_limit_pending_tasks, query_p0 limit pushdown suites, query_p0/scan_range, and stream load CSV suites - Behavior changed: Yes (default scan scheduler for internal and external table scans changes from the task executor to the thread pool; it can be switched back via the two BE configs) - Does this need documentation: No
mrhhsg
added a commit
to mrhhsg/doris
that referenced
this pull request
Oct 9, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#51690, apache#58144, apache#58273, apache#65814, apache#67070 Problem Summary: Workload groups pick the scan scheduler for internal and external table scans from `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table`. Both configs currently default to `true` on master, so every workload group creates `TaskExecutorSimplifiedScanScheduler` (the time-sharing task executor) for local (`ls_*`) and remote (`rs_*`) scans. The time-sharing executor is still maturing (for example the self-deadlock fixed in apache#58273; the internal-table default was already reverted once in apache#58144), so the default should be the `ThreadPoolSimplifiedScanScheduler` for both internal and external tables. This change sets both configs' default values to `false`. The task executor can still be enabled explicitly through `be.conf`. Making the thread pool path the default exposed one scheduling gap between the two admission paths. apache#65814 stops the TaskExecutor path (`ScannerContext::_pull_next_scan_task`) from submitting pending scanners once the shared scan LIMIT is exhausted while a completed or in-flight task can still wake the operator. The thread pool admission rewritten in apache#67070 (`ScannerContext::can_admit_scan_task`) does not apply this rule, so for a `SELECT ... LIMIT n` over many tablets every remaining pending scanner would still be admitted and run `prepare()`/`open()` (tablet reader init) only to report EOS right away. The same rule is now applied in `can_admit_scan_task`, including the existing escape that admits one pending scanner when nothing else can wake the pipeline task. ### Release note The BE configs `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table` now default to `false`; internal and external table scans use the thread pool scan scheduler by default. ### Check List (For Author) - Test: - Unit Test: added `WorkloadGroupManagerTest.DefaultScanSchedulersUseThreadPool` (registered defaults are `false` and the workload group creates thread pool schedulers for local and remote scans) and `ScannerContextTest.thread_pool_admission_stops_after_shared_limit` (fails without the admission fix); ran `WorkloadGroupManagerTest.*`, `ScannerContextTest.*` and other scanner/scheduler/task-executor UTs - Regression test: ran existing suites on a local cluster with the new default: correctness_p0/test_shared_scan_limit_pending_tasks, query_p0 limit pushdown suites, query_p0/scan_range, and stream load CSV suites - Behavior changed: Yes (default scan scheduler for internal and external table scans changes from the task executor to the thread pool; it can be switched back via the two BE configs) - Does this need documentation: No
mrhhsg
added a commit
to mrhhsg/doris
that referenced
this pull request
Oct 9, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#51690, apache#58144, apache#58273, apache#65814, apache#67070 Problem Summary: Workload groups pick the scan scheduler for internal and external table scans from `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table`. Both configs currently default to `true` on master, so every workload group creates `TaskExecutorSimplifiedScanScheduler` (the time-sharing task executor) for local (`ls_*`) and remote (`rs_*`) scans. The time-sharing executor is still maturing (for example the self-deadlock fixed in apache#58273; the internal-table default was already reverted once in apache#58144), so the default should be the `ThreadPoolSimplifiedScanScheduler` for both internal and external tables. This change sets both configs' default values to `false`. The task executor can still be enabled explicitly through `be.conf`. Making the thread pool path the default exposed one scheduling gap between the two admission paths. apache#65814 stops the TaskExecutor path (`ScannerContext::_pull_next_scan_task`) from submitting pending scanners once the shared scan LIMIT is exhausted while a completed or in-flight task can still wake the operator. The thread pool admission rewritten in apache#67070 (`ScannerContext::can_admit_scan_task`) does not apply this rule, so for a `SELECT ... LIMIT n` over many tablets every remaining pending scanner would still be admitted and run `prepare()`/`open()` (tablet reader init) only to report EOS right away. The same rule is now applied in `can_admit_scan_task`, including the existing escape that admits one pending scanner when nothing else can wake the pipeline task. ### Release note The BE configs `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table` now default to `false`; internal and external table scans use the thread pool scan scheduler by default. ### Check List (For Author) - Test: - Unit Test: added `WorkloadGroupManagerTest.DefaultScanSchedulersUseThreadPool` (registered defaults are `false` and the workload group creates thread pool schedulers for local and remote scans) and `ScannerContextTest.thread_pool_admission_stops_after_shared_limit` (fails without the admission fix); ran `WorkloadGroupManagerTest.*`, `ScannerContextTest.*` and other scanner/scheduler/task-executor UTs - Regression test: ran existing suites on a local cluster with the new default: correctness_p0/test_shared_scan_limit_pending_tasks, query_p0 limit pushdown suites, query_p0/scan_range, and stream load CSV suites - Behavior changed: Yes (default scan scheduler for internal and external table scans changes from the task executor to the thread pool; it can be switched back via the two BE configs) - Does this need documentation: No
mrhhsg
added a commit
to mrhhsg/doris
that referenced
this pull request
Oct 9, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#51690, apache#58144, apache#58273, apache#65814, apache#67070 Problem Summary: Workload groups pick the scan scheduler for internal and external table scans from `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table`. Both configs currently default to `true` on master, so every workload group creates `TaskExecutorSimplifiedScanScheduler` (the time-sharing task executor) for local (`ls_*`) and remote (`rs_*`) scans. The time-sharing executor is still maturing (for example the self-deadlock fixed in apache#58273; the internal-table default was already reverted once in apache#58144), so the default should be the `ThreadPoolSimplifiedScanScheduler` for both internal and external tables. This change sets both configs' default values to `false`. The task executor can still be enabled explicitly through `be.conf`. Making the thread pool path the default exposed one scheduling gap between the two admission paths. apache#65814 stops the TaskExecutor path (`ScannerContext::_pull_next_scan_task`) from submitting pending scanners once the shared scan LIMIT is exhausted while a completed or in-flight task can still wake the operator. The thread pool admission rewritten in apache#67070 (`ScannerContext::can_admit_scan_task`) does not apply this rule, so for a `SELECT ... LIMIT n` over many tablets every remaining pending scanner would still be admitted and run `prepare()`/`open()` (tablet reader init) only to report EOS right away. The same rule is now applied in `can_admit_scan_task`, including the existing escape that admits one pending scanner when nothing else can wake the pipeline task. ### Release note The BE configs `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table` now default to `false`; internal and external table scans use the thread pool scan scheduler by default. ### Check List (For Author) - Test: - Unit Test: added `WorkloadGroupManagerTest.DefaultScanSchedulersUseThreadPool` (registered defaults are `false` and the workload group creates thread pool schedulers for local and remote scans) and `ScannerContextTest.thread_pool_admission_stops_after_shared_limit` (fails without the admission fix); ran `WorkloadGroupManagerTest.*`, `ScannerContextTest.*` and other scanner/scheduler/task-executor UTs - Regression test: ran existing suites on a local cluster with the new default: correctness_p0/test_shared_scan_limit_pending_tasks, query_p0 limit pushdown suites, query_p0/scan_range, and stream load CSV suites - Behavior changed: Yes (default scan scheduler for internal and external table scans changes from the task executor to the thread pool; it can be switched back via the two BE configs) - Does this need documentation: No
mrhhsg
added a commit
to mrhhsg/doris
that referenced
this pull request
Oct 9, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#51690, apache#58144, apache#58273, apache#65814, apache#67070 Problem Summary: Workload groups pick the scan scheduler for internal and external table scans from `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table`. Both configs currently default to `true` on master, so every workload group creates `TaskExecutorSimplifiedScanScheduler` (the time-sharing task executor) for local (`ls_*`) and remote (`rs_*`) scans. The time-sharing executor is still maturing (for example the self-deadlock fixed in apache#58273; the internal-table default was already reverted once in apache#58144), so the default should be the `ThreadPoolSimplifiedScanScheduler` for both internal and external tables. This change sets both configs' default values to `false`. The task executor can still be enabled explicitly through `be.conf`. Making the thread pool path the default exposed one scheduling gap between the two admission paths. apache#65814 stops the TaskExecutor path (`ScannerContext::_pull_next_scan_task`) from submitting pending scanners once the shared scan LIMIT is exhausted while a completed or in-flight task can still wake the operator. The thread pool admission rewritten in apache#67070 (`ScannerContext::can_admit_scan_task`) does not apply this rule, so for a `SELECT ... LIMIT n` over many tablets every remaining pending scanner would still be admitted and run `prepare()`/`open()` (tablet reader init) only to report EOS right away. The same rule is now applied in `can_admit_scan_task`, including the existing escape that admits one pending scanner when nothing else can wake the pipeline task. ### Release note The BE configs `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table` now default to `false`; internal and external table scans use the thread pool scan scheduler by default. ### Check List (For Author) - Test: - Unit Test: added `WorkloadGroupManagerTest.DefaultScanSchedulersUseThreadPool` (registered defaults are `false` and the workload group creates thread pool schedulers for local and remote scans) and `ScannerContextTest.thread_pool_admission_stops_after_shared_limit` (fails without the admission fix); ran `WorkloadGroupManagerTest.*`, `ScannerContextTest.*` and other scanner/scheduler/task-executor UTs - Regression test: ran existing suites on a local cluster with the new default: correctness_p0/test_shared_scan_limit_pending_tasks, query_p0 limit pushdown suites, query_p0/scan_range, and stream load CSV suites - Behavior changed: Yes (default scan scheduler for internal and external table scans changes from the task executor to the thread pool; it can be switched back via the two BE configs) - Does this need documentation: No
mrhhsg
added a commit
to mrhhsg/doris
that referenced
this pull request
Oct 14, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#51690, apache#58144, apache#58273, apache#65814, apache#67070 Problem Summary: Workload groups pick the scan scheduler for internal and external table scans from `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table`. Both configs currently default to `true` on master, so every workload group creates `TaskExecutorSimplifiedScanScheduler` (the time-sharing task executor) for local (`ls_*`) and remote (`rs_*`) scans. The time-sharing executor is still maturing (for example the self-deadlock fixed in apache#58273; the internal-table default was already reverted once in apache#58144), so the default should be the `ThreadPoolSimplifiedScanScheduler` for both internal and external tables. This change sets both configs' default values to `false`. The task executor can still be enabled explicitly through `be.conf`. With the thread pool path as the default, a `SELECT ... LIMIT n` over many tablets keeps admitting its remaining pending scanners after peer scanners have exhausted the shared scan LIMIT (`ScannerContext::can_admit_scan_task`, rewritten in apache#67070, only applies the concurrency limit). Admission is kept that way on purpose: holding pending scanners back based on the in-flight or completed task counters, as apache#65814 does on the TaskExecutor path, ties the schedule to bookkeeping that is easy to get wrong and can leave a context with nothing to wake the operator. Instead, `Scanner::get_block_after_projects()` now checks the shared LIMIT before it reads: once the counter is exhausted, the scanner returns an empty block with EOS, so an admitted scanner never reads its range for rows the operator will cut anyway. ### Release note The BE configs `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table` now default to `false`; internal and external table scans use the thread pool scan scheduler by default. ### Check List (For Author) - Test: - Unit Test: added `WorkloadGroupManagerTest.DefaultScanSchedulersUseThreadPool` (registered defaults are `false` and the workload group creates thread pool schedulers for local and remote scans), `ScannerContextTest.thread_pool_admission_ignores_shared_limit` (a pending scanner is still admitted after the shared LIMIT is exhausted while tasks are in flight or completed) and `ScannerProjectionTest.shared_limit_exhausted_before_call_reports_eos_without_reading` (the admitted scanner reports EOS with an empty block without reading); ran `WorkloadGroupManagerTest.*`, `ScannerContextTest.*`, `ScannerProjectionTest.*` and other scanner/scheduler/task-executor UTs - Regression test: ran existing suites on a local cluster with the new default: correctness_p0/test_shared_scan_limit_pending_tasks, query_p0 limit pushdown suites, query_p0/scan_range, and stream load CSV suites - Behavior changed: Yes (default scan scheduler for internal and external table scans changes from the task executor to the thread pool; it can be switched back via the two BE configs) - Does this need documentation: No
mrhhsg
added a commit
to mrhhsg/doris
that referenced
this pull request
Oct 14, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#51690, apache#58144, apache#58273, apache#65814, apache#67070 Problem Summary: Workload groups pick the scan scheduler for internal and external table scans from `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table`. Both configs currently default to `true` on master, so every workload group creates `TaskExecutorSimplifiedScanScheduler` (the time-sharing task executor) for local (`ls_*`) and remote (`rs_*`) scans. The time-sharing executor is still maturing (for example the self-deadlock fixed in apache#58273; the internal-table default was already reverted once in apache#58144), so the default should be the `ThreadPoolSimplifiedScanScheduler` for both internal and external tables. This change sets both configs' default values to `false`. The task executor can still be enabled explicitly through `be.conf`. With the thread pool path as the default, a `SELECT ... LIMIT n` over many tablets keeps admitting its remaining pending scanners after peer scanners have exhausted the shared scan LIMIT (`ScannerContext::can_admit_scan_task`, rewritten in apache#67070, only applies the concurrency limit). Admission is kept that way on purpose: holding pending scanners back based on the in-flight or completed task counters, as apache#65814 does on the TaskExecutor path, ties the schedule to bookkeeping that is easy to get wrong and can leave a context with nothing to wake the operator. Instead, `Scanner::get_block_after_projects()` now checks the shared LIMIT before it reads: once the counter is exhausted, the scanner returns an empty block with EOS, so an admitted scanner never reads its range for rows the operator will cut anyway. ### Release note The BE configs `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table` now default to `false`; internal and external table scans use the thread pool scan scheduler by default. ### Check List (For Author) - Test: - Unit Test: added `WorkloadGroupManagerTest.DefaultScanSchedulersUseThreadPool` (registered defaults are `false` and the workload group creates thread pool schedulers for local and remote scans), `ScannerContextTest.thread_pool_admission_ignores_shared_limit` (a pending scanner is still admitted after the shared LIMIT is exhausted while tasks are in flight or completed) and `ScannerProjectionTest.shared_limit_exhausted_before_call_reports_eos_without_reading` (the admitted scanner reports EOS with an empty block without reading); ran `WorkloadGroupManagerTest.*`, `ScannerContextTest.*`, `ScannerProjectionTest.*` and other scanner/scheduler/task-executor UTs - Regression test: ran existing suites on a local cluster with the new default: correctness_p0/test_shared_scan_limit_pending_tasks, query_p0 limit pushdown suites, query_p0/scan_range, and stream load CSV suites - Behavior changed: Yes (default scan scheduler for internal and external table scans changes from the task executor to the thread pool; it can be switched back via the two BE configs) - Does this need documentation: No
mrhhsg
added a commit
to mrhhsg/doris
that referenced
this pull request
Oct 15, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#51690, apache#58144, apache#58273, apache#65814, apache#67070 Problem Summary: Workload groups pick the scan scheduler for internal and external table scans from `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table`. Both configs currently default to `true` on master, so every workload group creates `TaskExecutorSimplifiedScanScheduler` (the time-sharing task executor) for local (`ls_*`) and remote (`rs_*`) scans. The time-sharing executor is still maturing (for example the self-deadlock fixed in apache#58273; the internal-table default was already reverted once in apache#58144), so the default should be the `ThreadPoolSimplifiedScanScheduler` for both internal and external tables. This change sets both configs' default values to `false`. The task executor can still be enabled explicitly through `be.conf`. With the thread pool path as the default, a `SELECT ... LIMIT n` over many tablets keeps admitting its remaining pending scanners after peer scanners have exhausted the shared scan LIMIT (`ScannerContext::can_admit_scan_task`, rewritten in apache#67070, only applies the concurrency limit). Admission is kept that way on purpose: holding pending scanners back based on the in-flight or completed task counters, as apache#65814 does on the TaskExecutor path, ties the schedule to bookkeeping that is easy to get wrong and can leave a context with nothing to wake the operator. Instead, `Scanner::get_block_after_projects()` now checks the shared LIMIT before it reads: once the counter is exhausted, the scanner returns an empty block with EOS, so an admitted scanner never reads its range for rows the operator will cut anyway. ### Release note The BE configs `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table` now default to `false`; internal and external table scans use the thread pool scan scheduler by default. ### Check List (For Author) - Test: - Unit Test: added `WorkloadGroupManagerTest.DefaultScanSchedulersUseThreadPool` (registered defaults are `false` and the workload group creates thread pool schedulers for local and remote scans), `ScannerContextTest.thread_pool_admission_ignores_shared_limit` (a pending scanner is still admitted after the shared LIMIT is exhausted while tasks are in flight or completed) and `ScannerProjectionTest.shared_limit_exhausted_before_call_reports_eos_without_reading` (the admitted scanner reports EOS with an empty block without reading); ran `WorkloadGroupManagerTest.*`, `ScannerContextTest.*`, `ScannerProjectionTest.*` and other scanner/scheduler/task-executor UTs - Regression test: ran existing suites on a local cluster with the new default: correctness_p0/test_shared_scan_limit_pending_tasks, query_p0 limit pushdown suites, query_p0/scan_range, and stream load CSV suites - Behavior changed: Yes (default scan scheduler for internal and external table scans changes from the task executor to the thread pool; it can be switched back via the two BE configs) - Does this need documentation: No
mrhhsg
added a commit
that referenced
this pull request
Oct 19, 2026
### What problem does this PR solve? Issue Number: None Related PR: #51690, #58144, #58273, #65814, #67070 Problem Summary: Workload groups pick the scan scheduler for internal and external table scans from `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table`. Both configs currently default to `true` on master, so every workload group creates `TaskExecutorSimplifiedScanScheduler` (the time-sharing task executor) for local (`ls_*`) and remote (`rs_*`) scans. The time-sharing executor is still maturing (for example the self-deadlock fixed in #58273; the internal-table default was already reverted once in #58144), so the default should be the `ThreadPoolSimplifiedScanScheduler` for both internal and external tables. This change sets both configs' default values to `false`. The task executor can still be enabled explicitly through `be.conf`. Making the thread pool path the default exposed four gaps. 1. Pending scanners after an exhausted shared LIMIT. For a `SELECT ... LIMIT n` over many tablets, the thread pool admission (`ScannerContext::can_admit_scan_task`, rewritten in #67070) keeps admitting the remaining pending scanners after peer scanners have exhausted the shared scan LIMIT. Admission is kept that way on purpose: holding pending scanners back based on the in-flight or completed task counters, as #65814 does on the TaskExecutor path, ties the schedule to bookkeeping that is easy to get wrong and can leave a context with nothing to wake the operator. Instead, `Scanner::get_block_after_projects()` now checks the shared LIMIT before it reads: once the counter is exhausted, the scanner returns an empty block, so an admitted scanner never reads its range for rows the operator will cut anyway. It does not report EOS for that: EOS means the underlying read is exhausted, while the LIMIT is applied by the scan operator, which stops the scanners after `reached_limit()`; `ScannerContext::get_block_from_queue()` also finishes the context once the counter is exhausted with nothing in flight. The two pre-existing checks in `Scanner::get_block()` that set `eof` from the shared counter are removed for the same reason, so the shared LIMIT is handled in one place. 2. Rows held in scanner padding under a shared LIMIT. `Scanner::get_block()` charges the shared LIMIT counter as soon as rows pass the filters, but with projections `get_block_after_projects()` may keep a small block in `_padding_block` and emit a later block first. Once peer scanners exhaust the counter, the context reaches EOS without running that scanner again, and the charged rows it holds are lost; for example, a parallel projected `LIMIT 4096` could return 4086 rows. The counter is now decremented only by the rows a call actually returns, so held padding is never charged and the counter equals "LIMIT minus returned rows" between calls. The padding batch target is capped by the remaining LIMIT when a call starts, so a scanner does not read on just to fill half a batch when fewer rows are needed, and the read loop stops as soon as peers exhaust the LIMIT, returning the rows it already holds. Projected scans with selective filters keep coalescing small blocks. The scan does not cap its output at the LIMIT (the operator cuts extra rows), so no further accounting is needed. 3. Exceptions in the row-ID fetch RPC callback. `multiget_data_v2` submits its callback to the remote scan scheduler; on a `ThreadPool` worker there is no exception boundary, and `RowIdStorageReader::read_by_rowids()` constructs `SlotDescriptor(pslot)` before its local `try`, so an unknown `PTypeNode.type` would throw `doris::Exception` and terminate the BE. The callback now calls `RowIdStorageReader::read_by_rowids_for_rpc()`, which converts a thrown exception into an error status on the response before the closure completes the RPC. `ExecEnv::set_id_manager()` is added under `BE_TEST` for the unit test. 4. The shared LIMIT pointer of a scanner initialized while the counter is negative. `ScanLocalState::shared_scan_limit_ptr()` returned null whenever the live counter was negative, and `Scanner::init()` caches that pointer once. Concurrent scanners can return more than the LIMIT before the operator makes its final cut, leaving the counter negative, so a scanner whose local state opened at that moment never checked or charged the shared LIMIT and could read a long selective tail after peers had satisfied the query. The pointer is now returned whenever the operator has a SQL LIMIT, which is how `ScannerContext` already decides it. ### Release note The BE configs `enable_task_executor_in_internal_table` and `enable_task_executor_in_external_table` now default to `false`; internal and external table scans use the thread pool scan scheduler by default. ### Check List (For Author) - Test: - Unit Test: added `WorkloadGroupManagerTest.DefaultScanSchedulersUseThreadPool`, `ScannerContextTest.thread_pool_admission_ignores_shared_limit`, `ScannerProjectionTest.shared_limit_exhausted_before_call_returns_empty_block_without_eos`, `ScannerProjectionTest.shared_limit_counts_only_emitted_rows`, `ScannerProjectionTest.shared_limit_drops_padding_after_peer_satisfies_limit`, `ScannerProjectionTest.shared_limit_flushes_padding_before_reading_more`, `ScannerProjectionTest.shared_limit_flushes_merged_padding_at_limit`, `ScannerProjectionTest.shared_limit_counts_returned_rows_without_projection`, `ScannerProjectionTest.shared_limit_counts_returned_rows_without_padding`, `ScannerProjectionTest.shared_limit_keeps_padding_small_blocks`, `ScannerProjectionTest.shared_limit_exhausted_by_own_rows_stops_reading_without_eos`, `ScannerProjectionTest.shared_limit_exhausted_by_peer_mid_read_returns_held_rows_without_eos`, `ScannerProjectionTest.shared_limit_observed_by_scanner_initialized_under_negative_counter`, `ScannerProjectionTest.no_shared_limit_without_sql_limit` and `RowIdStorageReaderTest.RpcHelperReportsThrownExceptionOnThreadPoolPath` (runs the RPC helper on a real `ThreadPoolSimplifiedScanScheduler`; the test process aborts without the fix); ran `ScannerProjectionTest.*`, `ScannerContextTest.*`, `ScannerLateArrivalRfTest.*`, `WorkloadGroupManagerTest.*`, `RowIdStorageReaderTest.*`, `SubmitExternalScanTasksTest.*` - Regression test: ran on a local cluster with the new default: query_p0/limit (19 suites) and correctness_p0/test_shared_scan_limit_pending_tasks - Behavior changed: Yes (default scan scheduler for internal and external table scans changes from the task executor to the thread pool; it can be switched back via the two BE configs) - Does this need documentation: No
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Release note
Summary
_dispatch_threadwhen a destructor path attempts to re-acquire the same mutex already held by the thread. The root cause is destructors (triggered while holding_lock) performing operations that try to re-acquire the same mutex. A safe fix should ensure destructors that may callremove_task()run outside the_lockscope or avoid re-locking the same mutex inside destructor paths._task_executor->wait()inTaskExecutorSimplifiedScanScheduler::stop().Details / Reproduction steps
std::shared_ptr<PrioritizedSplitRunner> split = _tokenless->_entries->take();l.lock();—_dispatch_threadacquires_lock.whileloop finishes,splitgoes out of scope and theshared_ptris destroyed.PrioritizedSplitRunnerdestructor runs → destroys_split_runner(ScannerSplitRunner).ScannerSplitRunner::_scan_funcdestructor runs → destroys capturedctx(std::shared_ptr<ScannerContext>).ScannerContext::~ScannerContext()callsremove_task().remove_task()attempts to acquire_lock._lockis already held by_dispatch_thread.Solution
We must explicitly release 'split' BEFORE acquiring _lock to avoid self-deadlock. The destructor chain (
PrioritizedSplitRunner->ScannerSplitRunner->_scan_funclambda -> capturedScannerContext) may callremove_task()which tries to acquire_lock. Since_lockis not a recursive mutex, this would deadlock.Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)