[fix](be) Release workload group schedulers on shutdown#65112
Open
jacktengg wants to merge 1 commit into
Open
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
yiguolei
previously approved these changes
Jul 1, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
approved
Contributor
|
PR approved by anyone and no changes requested. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review result: request changes.
Critical checkpoint conclusions:
- Goal/test proof: the PR targets graceful-exit retention of workload-group schedulers and queued resources, but the pipeline queue drain can still miss a shutdown-time enqueue, so the fix is incomplete.
- Scope: the patch is small and focused on shutdown cleanup.
- Concurrency/lifecycle: one blocking issue remains in
PriorityTaskQueue::close()versuspush()serialization. Shutdown ordering for the workload-group destructive stop was reviewed. - Config/compatibility/persistence: no new config, protocol, storage format, or persistence compatibility concern was found.
- Parallel paths: pipeline scheduler, time-sharing scan scheduler, memtable flush pool, workload-group drop, and final ExecEnv destroy paths were checked.
- Tests/style: no test or expected-output files are changed. I did not run BE tests locally in this review. Current PR checks show Clang Formatter passing; BE UT (macOS) is failing in GitHub checks.
- User focus: no additional user-provided focus points.
Subagent conclusions: optimizer-rewrite proposed OPT-001, accepted as MAIN-001 and submitted inline. tests-session-config proposed TSC-001; I dismissed it with shutdown-order evidence because the known dynamic config entry points are stopped and joined before WorkloadGroupMgr::stop() performs the new destructive reset. Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the final ledger/comment set.
Contributor
TPC-H: Total hot run time: 29537 ms |
Contributor
TPC-DS: Total hot run time: 172926 ms |
Contributor
ClickBench: Total hot run time: 25.21 s |
Contributor
Author
|
run nonConcurrent 10 |
Graceful shutdown with enable_graceful_exit_check could report ASAN/LSan leaks after exec env destroy. The leak stacks showed pending pipeline tasks in MultiCoreTaskQueue, scanner splits in TimeSharingTaskExecutor, memtable flush tasks in workload group ThreadPool, and QueryContext/ResourceContext retaining WorkloadGroup. ExecEnv stopped workload group schedulers before fragment and load resources were released, and WorkloadGroupMgr::stop only stopped scheduler threads without destroying scheduler objects or their queued resources. Pending queues and executor task maps could therefore retain query/load resources until process exit. This change clears pending pipeline task queues on close, makes queue close and submit mutually exclusive, only marks tasks runnable after a successful enqueue, releases scheduler-owned queues after fragment/load resources have stopped.
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
github-actions
Bot
removed
the
approved
Contributor
There was a problem hiding this comment.
Automated review summary for PR #65112 at e111072f.
Opinion: no additional blocking issue found in the current head. I did not submit a duplicate for the existing PriorityTaskQueue::close()/push() thread; the current diff now checks _closed under _work_size_mutex before enqueueing.
Critical checkpoint conclusions:
- Goal/test: The PR targets shutdown-time ASAN/LSan retention by draining pipeline queues, stopping workload group schedulers before FragmentMgr cleanup, and delaying scheduler object destruction until fragment/load resources stop submitting cleanup work. The current code matches that goal. No new test was added.
- Scope: The changed files are focused on BE workload group scheduler shutdown, queue draining, and idempotent scheduler stop behavior.
- Concurrency/lifecycle:
PriorityTaskQueue::push()andclose()now serialize submit/close with_work_size_mutex; queue-held tasks are released after dropping the queue lock. Scan scheduler andTimeSharingTaskExecutorstops are idempotent, and scheduler object reset is delayed until after FragmentMgr/load resources stop. - Parallel paths: Pipeline queues, local and remote scan scheduler variants, time-sharing scan executor cleanup, and memtable flush pool shutdown/reset were checked.
- Config/compat/persistence: No new config, protocol, storage format, or EditLog compatibility impact.
- Tests/CI: Local build/test was not runnable in this checkout because
.worktree_initializedandthirdparty/installedare absent.git diff --checkon the PR changed files passed. GitHub formatter/checkstyle passed. The macOS BE UT job failed before running tests because the runner had Java 25 while the job requires JDK 17; larger compile/BE UT contexts were still pending at review time. - User focus: No additional user-provided review focus.
Subagent conclusions: Initial optimizer-rewrite and tests-session-config subagent passes both reported NO_NEW_VALUABLE_FINDINGS; convergence round 1 ended with both live subagents again replying NO_NEW_VALUABLE_FINDINGS for this same ledger and proposed final comment set.
Contributor
TPC-H: Total hot run time: 29302 ms |
Contributor
TPC-DS: Total hot run time: 173310 ms |
Contributor
ClickBench: Total hot run time: 25.34 s |
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
3 participants
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?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
There are smart pointer cycle references:
and
Graceful shutdown with enable_graceful_exit_check could report ASAN/LSan leaks after exec env destroy. The leak stacks showed pending pipeline tasks in MultiCoreTaskQueue, scanner splits in TimeSharingTaskExecutor, memtable flush tasks in workload group ThreadPool, and QueryContext/ResourceContext retaining WorkloadGroup.
ExecEnv stopped workload group schedulers before fragment and load resources were released, and WorkloadGroupMgr::stop only stopped scheduler threads without destroying scheduler objects or their queued resources. Pending queues and executor task maps could therefore retain query/load resources until process exit. This change clears pending pipeline task queues on close, makes queue close and submit mutually exclusive, only marks tasks runnable after a successful enqueue, releases scheduler-owned queues after fragment/load resources have stopped.
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)