branch-4.0: [fix](load_stream) Fix flush token deadlock by ensuring wait_for_flush_tasks is called before destruction #60284 #60285
Conversation
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
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
There was a problem hiding this comment.
Pull request overview
This PR fixes a deadlock issue in load_stream by ensuring flush tasks are properly waited for before TabletStream destruction. The fix reverts the use of shared_from_this() and instead relies on explicit lifetime management through the IndexStream destructor.
Changes:
- Removed
std::enable_shared_from_thisinheritance from TabletStream and allshared_from_this()usages - Added
wait_for_flush_tasks()method to TabletStream to safely wait for all flush tasks before destruction - Added IndexStream destructor to ensure all TabletStreams wait for flush tasks before cleanup
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| be/src/runtime/load_stream.h | Removed shared_from_this inheritance, added wait_for_flush_tasks() declaration, added IndexStream destructor, added _flush_tasks_done flag |
| be/src/runtime/load_stream.cpp | Replaced shared_from_this with this in lambdas, implemented wait_for_flush_tasks() and IndexStream destructor, refactored pre_close() to use wait_for_flush_tasks() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
liaoxin01
force-pushed
the
fix-load-stream-flush-token-deadlock
branch
from
1529b65 to
ed0add4
Compare
Contributor
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
liaoxin01
force-pushed
the
fix-load-stream-flush-token-deadlock
branch
from
ed0add4 to
3e507a7
Compare
…h_tasks is called before destruction Problem: When TabletStream is destroyed without pre_close() being called (e.g., on_idle_timeout scenario), the _flush_token destructor calls shutdown() which triggers deadlock detection if called from the pool thread. Root cause: - on_idle_timeout() directly calls brpc::StreamClose() without calling LoadStream::close() - This triggers the destruction chain without calling pre_close() on TabletStreams - If flush tasks are still running, TabletStream may be destroyed in pool thread Solution: - Add IndexStream::~IndexStream() to ensure wait_for_flush_tasks() is called on all TabletStreams - Add TabletStream::wait_for_flush_tasks() to wait for all flush tasks to complete - This ensures _flush_token is properly handled before TabletStream destruction - Revert commit c18ef17 (shared_from_this) as it is no longer needed
liaoxin01
force-pushed
the
fix-load-stream-flush-token-deadlock
branch
from
3e507a7 to
2a1ef37
Compare
Contributor
Author
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
yiguolei
approved these changes
Jan 28, 2026
Contributor
|
skip buildall |
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
pick from：#60284