[fix](be) Fix exchange receiver dependency race#62777
Merged
yiguolei merged 1 commit intoapache:masterfrom Apr 28, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No blocking findings in the reviewed changes.
Critical Checkpoints
- Goal and correctness: The PR fixes the lost ready-notification window between receiver registration and
set_dependency().SenderQueue::set_dependency()now rechecks queue and EOS state under_lock, and the two added unit tests cover both "data before dependency" and "EOS before dependency". I found no remaining correctness issue in the modified flow. - Scope: The change is small and focused to the receiver readiness race, the sink buffer mutex protection, and targeted tests.
- Concurrency: The modified receiver path now uses
SenderQueue::_lockconsistently for_source_dependency,_block_queue,_num_remaining_senders, and_is_cancelledwhen deciding readiness. The sink buffer now readsrpc_channel_is_turn_offunderinstance_data.mutex, removing an unsynchronized access. I found no new lock-order or deadlock issue. - Lifecycle: No new special lifecycle or static-initialization risk. The existing create-and-register ordering window in
VDataStreamMgris the race being fixed. - Config and compatibility: No new config, protocol, symbol, or storage compatibility concern.
- Parallel paths: I checked analogous dependency-based queue code and did not find another path with the same externally visible registration race in this PR scope.
- Conditions and comments: The new recheck in
set_dependency()is documented and matches the actual race window. - Test coverage: The new BE unit tests are appropriate for the receiver race. There is no dedicated test for the sink-buffer mutex move, but I did not find a behavioral regression there. I attempted to run
./run-be-ut.sh --run --filter=DataStreamRecvrTest.TestEosBeforeSetDependency:DataStreamRecvrTest.TestDataBeforeSetDependencyWithRemainingSendersin this runner, but the build failed before compilation becausethirdparty/installed/bin/protocis missing. - Observability: The added teardown log is acceptable and does not hide failures.
- Transactions, persistence, data writes, and FE-to-BE variable passing: Not applicable.
- Performance: No meaningful regression; the added locking work is negligible.
User Focus
- No additional user-provided review focus was supplied.
Member
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Issue Number: None
Related PR: None
Problem Summary: Data or EOS could arrive after the receiver is registered but before its source dependency is installed. In that window, ready notification was lost and the exchange source could remain blocked. Recheck the queue state when setting the dependency and protect channel turn-off checks with the instance mutex.
Fix a potential query hang caused by an exchange receiver dependency ready-notification race.
- Test: Unit Test / Manual test
- Unit Test: ./run-be-ut.sh --run --filter=DataStreamRecvrTest.TestEosBeforeSetDependency:DataStreamRecvrTest.TestDataBeforeSetDependencyWithRemainingSenders
- Manual test: build-support/check-format.sh
- Static analysis attempted: build-support/run-clang-tidy.sh --build-dir be/ut_build_ASAN failed because clang-tidy could not analyze the files due to environment/pre-existing diagnostics, including missing stddef.h from system/libstdc++ headers and an existing unmatched NOLINTEND in be/src/core/types.h
- Behavior changed: Yes. Exchange source dependencies are now marked ready if queued data or EOS arrived before set_dependency().
- Does this need documentation: No
Mryange
approved these changes
Apr 27, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Member
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
yiguolei
approved these changes
Apr 28, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Apr 28, 2026
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Data or EOS could arrive after the receiver is
registered but before its source dependency is installed. In that
window, ready notification was lost and the exchange source could remain
blocked. Recheck the queue state when setting the dependency and protect
channel turn-off checks with the instance mutex.
### Release note
Fix a potential query hang caused by an exchange receiver dependency
ready-notification race.
### Check List (For Author)
- Test: Unit Test / Manual test
- Unit Test: ./run-be-ut.sh --run
--filter=DataStreamRecvrTest.TestEosBeforeSetDependency:DataStreamRecvrTest.TestDataBeforeSetDependencyWithRemainingSenders
- Manual test: build-support/check-format.sh
- Static analysis attempted: build-support/run-clang-tidy.sh --build-dir
be/ut_build_ASAN failed because clang-tidy could not analyze the files
due to environment/pre-existing diagnostics, including missing stddef.h
from system/libstdc++ headers and an existing unmatched NOLINTEND in
be/src/core/types.h
- Behavior changed: Yes. Exchange source dependencies are now marked
ready if queued data or EOS arrived before set_dependency().
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] 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.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#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 -->
yiguolei
pushed a commit
that referenced
this pull request
Apr 29, 2026
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
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.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Data or EOS could arrive after the receiver is registered but before its source dependency is installed. In that window, ready notification was lost and the exchange source could remain blocked. Recheck the queue state when setting the dependency and protect channel turn-off checks with the instance mutex.
Release note
Fix a potential query hang caused by an exchange receiver dependency ready-notification race.
Check List (For Author)
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)