Uh oh!
There was an error while loading. Please reload this page.
[fix](load) correct quorum participants for incremental streams - #66016
Merged
Conversation
hello-stephen
commented
Jul 24, 2026
Contributor
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
deardeng
commented
Jul 24, 2026
ContributorAuthor
run buildall |
deardeng
commented
Jul 24, 2026
ContributorAuthor
/review |
Contributor
Codex automated review failed and did not complete. Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Jul 30th, 2026 7:58 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
hello-stephen
commented
Jul 24, 2026
Contributor
TPC-H: Total hot run time: 22848 ms |
hello-stephen
commented
Jul 24, 2026
Contributor
TPC-DS: Total hot run time: 117086 ms |
hello-stephen
commented
Jul 24, 2026
Contributor
ClickBench: Total hot run time: 18.42 s |
hello-stephen
commented
Jul 24, 2026
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Jul 24, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
PR approved by at least one committer and no changes requested. |
Contributor
PR approved by anyone and no changes requested. |
sollhui added a commit
to sollhui/doris
that referenced
this pull request
Jul 27, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#66016 Problem Summary: The first close stage waits only for non-incremental streams, but quorum calculation treated incremental streams outside that stage as already finished. In auto-partition loads this could satisfy quorum before the shared dummy stream closed, allowing an early source to close incremental streams before late sources opened them. Snapshot the streams participating in each close stage and count a destination only after its participating streams finish. ### Release note None ### Check List (For Author) - Test: Unit test added; local execution skipped after dependency initialization was interrupted - Behavior changed: No - Does this need documentation: No
5 tasks
deardeng
commented
Jul 27, 2026
ContributorAuthor
run buildall |
deardeng
commented
Jul 27, 2026
ContributorAuthor
/review |
hello-stephen
commented
Jul 27, 2026
Contributor
TPC-H: Total hot run time: 29721 ms |
Contributor
Codex automated review failed and did not complete. Error: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Aug 2nd, 2026 1:27 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
hello-stephen
commented
Jul 27, 2026
Contributor
TPC-DS: Total hot run time: 177147 ms |
hello-stephen
commented
Jul 27, 2026
Contributor
ClickBench: Total hot run time: 25.08 s |
hello-stephen
commented
Jul 27, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
sollhui
reviewed
Jul 28, 2026
Uh oh!
There was an error while loading. Please reload this page.
hello-stephen
commented
Jul 28, 2026
Contributor
TPC-H: Total hot run time: 29208 ms |
hello-stephen
commented
Jul 28, 2026
Contributor
TPC-DS: Total hot run time: 177131 ms |
hello-stephen
commented
Jul 28, 2026
Contributor
ClickBench: Total hot run time: 24.97 s |
hello-stephen
commented
Jul 28, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Codex automated review failed and did not complete. Error: Your access token could not be refreshed because your refresh token was revoked. Please log out and sign in again. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
deardeng
commented
Jul 28, 2026
ContributorAuthor
run p0 |
shuke987
commented
Jul 28, 2026
Contributor
/review |
Contributor
Codex automated review failed and did not complete. Error: Your access token could not be refreshed because your refresh token was revoked. Please log out and sign in again. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
hello-stephen
commented
Jul 28, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
PR approved by at least one committer and no changes requested. |
shuke987
commented
Jul 28, 2026
Contributor
/review |
Contributor
There was a problem hiding this comment.
Request changes. The local is_closing guard correctly prevents not-yet-closing incremental streams from supplying free quorum votes in normal ordered executions, but the PR still has two blocking gaps.
Critical checkpoint conclusions:
- Goal and data correctness: the sender-side predicate is directionally correct, but EOS plus
StreamCloseis not a reliable all-original-sources fence. The receiver can either strand a deferred stream under concurrentCLOSE_LOADcallbacks or let an abnormal reset look like a successful fence, so the intended lifecycle race can become a timeout or still advance early. - Concurrency, lifecycle, and error handling: multiple accepted streams share one
LoadStreamhandler while running separate callback queues. The splitall_closed/deferred-id transition is not atomic. The new_is_closingatomic itself is C++20-initialized and sequentially consistent; send failure and explicit cancellation remain excluded. The unresolved problem is the receiver acknowledgement and the reasonless abnormal-close path. - Tests: the BE unit test is a useful focused pre-fix-fails/post-fix-passes check for quorum membership, but it does not exercise receiver concurrency or reset behavior. The new Docker regression deterministically fails on its first
SETbecause all three scheduler variable names are unknown in this branch, so it never creates a table or opens a stream. - Scope, parallel paths, compatibility, and performance: the four-file patch is small and focused; the V1 parallel path was reviewed. No new protocol field, configuration, persisted state, storage format, static-lifecycle, memory-accounting, observability, or material hot-path performance issue was found.
- User focus: no additional review focus was supplied, so the whole PR and all applicable checkpoints were reviewed.
Validation was static because the review bundle forbids local builds and tests. Current PR checks were green apart from the in-progress automated review, but the Docker setup failure is directly established by the current variable registry and VariableMgr error path.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
github-actionsBot
pushed a commit
that referenced
this pull request
Jul 28, 2026
### What problem does this PR solve?
Problem Summary:
Auto partition loads may create incremental load streams after another
source has already reached write quorum.
In `VTabletWriterV2::_quorum_success()`, a stream that had not entered
the current close stage was implicitly counted as finished — the check
only looked at `unfinished_streams` membership and cancellation.
Incremental streams never enter the first-stage `close_wait` participant
set (`_non_incremental_streams()`), so they contributed free "finished"
votes.
As a result the first-stage `close_wait` could return before every
source had sent CLOSE_LOAD. The early source then sent CLOSE_LOAD on its
incremental streams while the late sources had not opened theirs yet. A
destination's incremental `LoadStream` counts senders dynamically
(`add_source()` bumps `_total_streams`), so it concluded that all
senders had closed and committed the tablet early, which surfaces as
double close and segment number mismatch errors.
This PR excludes streams that have not entered the current close stage
from V2 quorum accounting (`!stream->is_closing()`), matching the V1
participant semantics while preserving the two-stage close algorithm. V1
already has the equivalent guard: `IndexChannel::_quorum_success()`
skips a node channel when `check_status()` fails, and `check_status()`
is `none_of({_cancelled, !_eos_is_produced})`, i.e. it also requires the
channel to have been `mark_close()`d for the current stage.
Also documents why leaving the first-stage `close_wait` on quorum
success still preserves the cross-source fence.
Tests:
-
`TestVTabletWriterV2.quorum_excludes_streams_not_closing_in_current_stage`
- `auto_partition_quorum_race_docker` (20 attempts, 1 FE + 3 BE,
`enable_quorum_success_write=true`)github-actionsBot
pushed a commit
that referenced
this pull request
Jul 28, 2026
### What problem does this PR solve?
Problem Summary:
Auto partition loads may create incremental load streams after another
source has already reached write quorum.
In `VTabletWriterV2::_quorum_success()`, a stream that had not entered
the current close stage was implicitly counted as finished — the check
only looked at `unfinished_streams` membership and cancellation.
Incremental streams never enter the first-stage `close_wait` participant
set (`_non_incremental_streams()`), so they contributed free "finished"
votes.
As a result the first-stage `close_wait` could return before every
source had sent CLOSE_LOAD. The early source then sent CLOSE_LOAD on its
incremental streams while the late sources had not opened theirs yet. A
destination's incremental `LoadStream` counts senders dynamically
(`add_source()` bumps `_total_streams`), so it concluded that all
senders had closed and committed the tablet early, which surfaces as
double close and segment number mismatch errors.
This PR excludes streams that have not entered the current close stage
from V2 quorum accounting (`!stream->is_closing()`), matching the V1
participant semantics while preserving the two-stage close algorithm. V1
already has the equivalent guard: `IndexChannel::_quorum_success()`
skips a node channel when `check_status()` fails, and `check_status()`
is `none_of({_cancelled, !_eos_is_produced})`, i.e. it also requires the
channel to have been `mark_close()`d for the current stage.
Also documents why leaving the first-stage `close_wait` on quorum
success still preserves the cross-source fence.
Tests:
-
`TestVTabletWriterV2.quorum_excludes_streams_not_closing_in_current_stage`
- `auto_partition_quorum_race_docker` (20 attempts, 1 FE + 3 BE,
`enable_quorum_success_write=true`)wyxxxcat pushed a commit
to wyxxxcat/doris
that referenced
this pull request
Aug 17, 2026
…he#66016) ### What problem does this PR solve? Problem Summary: Auto partition loads may create incremental load streams after another source has already reached write quorum. In `VTabletWriterV2::_quorum_success()`, a stream that had not entered the current close stage was implicitly counted as finished — the check only looked at `unfinished_streams` membership and cancellation. Incremental streams never enter the first-stage `close_wait` participant set (`_non_incremental_streams()`), so they contributed free "finished" votes. As a result the first-stage `close_wait` could return before every source had sent CLOSE_LOAD. The early source then sent CLOSE_LOAD on its incremental streams while the late sources had not opened theirs yet. A destination's incremental `LoadStream` counts senders dynamically (`add_source()` bumps `_total_streams`), so it concluded that all senders had closed and committed the tablet early, which surfaces as double close and segment number mismatch errors. This PR excludes streams that have not entered the current close stage from V2 quorum accounting (`!stream->is_closing()`), matching the V1 participant semantics while preserving the two-stage close algorithm. V1 already has the equivalent guard: `IndexChannel::_quorum_success()` skips a node channel when `check_status()` fails, and `check_status()` is `none_of({_cancelled, !_eos_is_produced})`, i.e. it also requires the channel to have been `mark_close()`d for the current stage. Also documents why leaving the first-stage `close_wait` on quorum success still preserves the cross-source fence. Tests: - `TestVTabletWriterV2.quorum_excludes_streams_not_closing_in_current_stage` - `auto_partition_quorum_race_docker` (20 attempts, 1 FE + 3 BE, `enable_quorum_success_write=true`)
16 tasks
morrySnow pushed a commit
that referenced
this pull request
Aug 31, 2026
### What problem does this PR solve?
Problem Summary:
Auto partition loads may create incremental load streams after another
source has already reached write quorum.
In `VTabletWriterV2::_quorum_success()`, a stream that had not entered
the current close stage was implicitly counted as finished — the check
only looked at `unfinished_streams` membership and cancellation.
Incremental streams never enter the first-stage `close_wait` participant
set (`_non_incremental_streams()`), so they contributed free "finished"
votes.
As a result the first-stage `close_wait` could return before every
source had sent CLOSE_LOAD. The early source then sent CLOSE_LOAD on its
incremental streams while the late sources had not opened theirs yet. A
destination's incremental `LoadStream` counts senders dynamically
(`add_source()` bumps `_total_streams`), so it concluded that all
senders had closed and committed the tablet early, which surfaces as
double close and segment number mismatch errors.
This PR excludes streams that have not entered the current close stage
from V2 quorum accounting (`!stream->is_closing()`), matching the V1
participant semantics while preserving the two-stage close algorithm. V1
already has the equivalent guard: `IndexChannel::_quorum_success()`
skips a node channel when `check_status()` fails, and `check_status()`
is `none_of({_cancelled, !_eos_is_produced})`, i.e. it also requires the
channel to have been `mark_close()`d for the current stage.
Also documents why leaving the first-stage `close_wait` on quorum
success still preserves the cross-source fence.
Tests:
-
`TestVTabletWriterV2.quorum_excludes_streams_not_closing_in_current_stage`
- `auto_partition_quorum_race_docker` (20 attempts, 1 FE + 3 BE,
`enable_quorum_success_write=true`)
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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?
Problem Summary:
Auto partition loads may create incremental load streams after another source has already reached write quorum.
In
VTabletWriterV2::_quorum_success(), a stream that had not entered the current close stage was implicitly counted as finished — the check only looked atunfinished_streamsmembership and cancellation. Incremental streams never enter the first-stageclose_waitparticipant set (_non_incremental_streams()), so they contributed free "finished" votes.As a result the first-stage
close_waitcould return before every source had sent CLOSE_LOAD. The early source then sent CLOSE_LOAD on its incremental streams while the late sources had not opened theirs yet. A destination's incrementalLoadStreamcounts senders dynamically (add_source()bumps_total_streams), so it concluded that all senders had closed and committed the tablet early, which surfaces as double close and segment number mismatch errors.This PR excludes streams that have not entered the current close stage from V2 quorum accounting (
!stream->is_closing()), matching the V1 participant semantics while preserving the two-stage close algorithm. V1 already has the equivalent guard:IndexChannel::_quorum_success()skips a node channel whencheck_status()fails, andcheck_status()isnone_of({_cancelled, !_eos_is_produced}), i.e. it also requires the channel to have beenmark_close()d for the current stage.Also documents why leaving the first-stage
close_waiton quorum success still preserves the cross-source fence.Tests:
TestVTabletWriterV2.quorum_excludes_streams_not_closing_in_current_stageauto_partition_quorum_race_docker(20 attempts, 1 FE + 3 BE,enable_quorum_success_write=true)Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)