mrhhsg opened a new pull request, #68571:
URL: https://github.com/apache/doris/pull/68571
### What problem does this PR solve?
Issue Number: None
Related PR: #67070
Problem Summary:
This is a rewrite of #67070 for branch-4.2, not a cherry-pick. #67070 builds
on scheduler refactors that branch-4.2 does not have, and those are
intentionally **not** picked here:
- #61617 (ScanTask state machine, `_pending_tasks` / `_completed_tasks`) was
reverted on this release line by #62191 and stays reverted.
- #61271 (adaptive scanners and the scan memory limiter), #62222 (limit push
down to the segment iterator) and #65814 (shared scan limit) are large behavior
changes of their own.
The admission logic is therefore written against the branch-4.2 model
(`_pending_scanners`, `_tasks_queue`, `_num_scheduled_scanners`,
`cached_blocks`). The parts of #67070 that depend on the shared scan limit or
on adaptive scanners / the memory limiter have no counterpart here and are
dropped.
The problem is the same on branch-4.2: the ThreadPool scan scheduler submits
one runnable per scanner and serializes scheduling of every Context through a
scheduler-wide lock. With many scanners this inflates queue occupancy and
couples ThreadPool admission to the TaskExecutor scheduling logic.
With this change the ThreadPool scheduler:
- queues at most one runnable per `ScannerContext`; the runnable admits one
pending scanner under the Context transfer lock, submits its successor and then
scans without the lock;
- keeps submission failures local to the Context (`TOO_MANY_TASKS` or the
shutdown error make only that Context terminal);
- checks the terminal Context state in `get_block_from_queue()` before
rescheduling a drained scanner;
- fails the Context at its next scheduling attempt when the pool has been
stopped while its runnable was still queued, instead of treating the runnable
that shutdown dropped as still pending.
Admission keeps the limits branch-4.2 applies through `_get_margin()` and
`_pull_next_scan_task()`: a Context never exceeds `_max_scan_concurrency`
(cached results count as occupied slots), low memory mode caps running
scanners, and once the pool has no slack (`active + queued >=
min_active_scan_threads`) a Context is held at `_min_scan_concurrency`, or at
`_max_scan_concurrency` while the operator is starving. One scanner is always
admitted when nothing is progressing so the operator can be woken. A worker
admitting the task it runs itself does not count its own thread against the
pool budget, matching `_get_margin()`, which runs on the operator thread.
The Context runnable waits in the pool queue on behalf of the scanner it
admits, so that wait is credited to the scanner's wait-worker time
(`Scanner::add_wait_worker_time()`), keeping `ScannerWorkerWaitTime` /
`PerScannerWaitTime` meaningful on the ThreadPool path.
TaskExecutor remains the default scan scheduler and keeps its behavior.
Generic ThreadPool behavior is unchanged. `debug_string()` now reports
`_is_context_queued` and `_num_finished_scanners`, and the ThreadPool path logs
admission, refusal and runnable submission at `VLOG_DEBUG`.
Validation:
- `./run-be-ut.sh --run
--filter="ScannerContextTest.*:ThreadPoolTest.*:*ScanScheduler*:*Scanner*"`
(132 tests passed)
- The new ThreadPool tests repeated 50 times with `GTEST_REPEAT=50` without
failure.
- clang-format 16 check and `git diff --check`.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. The opt-in ThreadPool scan scheduler queues and admits work
per `ScannerContext`.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
