[fix](transaction) Fail fast when a backend restarts during insert#65525
Merged
Conversation
### What problem does this PR solve? Issue Number: None Problem Summary: Nereids INSERT waits for fragment completion through LoadProcessor. The backend health check only considered the alive flag, so a quickly restarted backend could look healthy even though the old process could no longer report fragment completion. Capture the backend process epoch when fragment tasks are created, compare it while joining, and propagate a precise restart error through the existing coordinator cancellation path. ### Release note Nereids INSERT now fails promptly with a clear error when an executing backend restarts. ### Check List (For Author) - Test: Unit test and docker regression test added; not run per request - Behavior changed: Yes. An INSERT fails fast instead of waiting until insert timeout after its backend restarts - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 29750 ms |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Requesting changes based on three correctness/testability issues found in static review.
Checkpoint conclusions:
- Goal: The PR aims to fail fast when a BE restarts during insert, but the implementation is incomplete because the new health loop checks only top-fragment tasks while insert join waits all fragment tasks.
- Scope: The code change is small and focused, but the checked task set is too narrow for multi-fragment insert plans.
- Concurrency/lifecycle: Backend heartbeat epoch updates and synchronous cancellation paths were reviewed. Cancellation status propagation is OK for ordinary insert and TVF, but epoch comparison has an upgrade/replay compatibility edge when the captured epoch is 0.
- Compatibility: Existing Doris logic treats zero process epochs as upgrade/old-metadata compatibility; this PR handles only current zero epochs, not captured zero epochs.
- Parallel paths: Ordinary insert, TVF insert, LoadProcessor, task construction, and legacy Coordinator epoch logic were checked. The top-fragment-only check is the remaining parallel-path gap.
- Tests: Added unit and docker regression coverage are useful, but the docker regression uses a fixed sleep instead of synchronizing on the writer-close debug point. No tests/builds were run in this review environment; review was static only.
- Config/persistence/FE-BE protocol: No new config, storage format, or FE-BE protocol fields were introduced.
- User focus: No additional user-provided review focus was supplied.
Round 2 subagent convergence returned NO_NEW_VALUABLE_FINDINGS for this final three-comment set, and no existing inline review comments were present.
Contributor
TPC-DS: Total hot run time: 180927 ms |
Contributor
ClickBench: Total hot run time: 25.08 s |
### What problem does this PR solve? Issue Number: None Problem Summary: The insert health check only inspected top-fragment tasks, an unknown captured backend epoch could be misclassified as a restart, and the docker regression test restarted the backend after a fixed delay. Check every unfinished fragment task, compare only positive epochs, and wait for an observable writer-close debug-point hit before restarting the backend. ### Release note INSERT now fails fast when any unfinished fragment backend restarts, while unknown backend process epochs remain compatible. ### Check List (For Author) - Test: Not run per request; unit and docker regression coverage were updated. - Behavior changed: Yes, backend restart detection covers every unfinished load fragment and ignores unknown epochs. - Does this need documentation: No.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
sollhui
changed the title
Contributor
TPC-H: Total hot run time: 29835 ms |
Contributor
There was a problem hiding this comment.
I found three remaining issues in the follow-up patch.
The FE-side latch coverage and zero-epoch compatibility concerns from the existing threads look addressed: the health check now walks all unfinished load fragment tasks, and epoch comparison only runs when both captured/current epochs are positive. The tokenized debug-point wait also addresses the original fixed-sleep race, but the regression still has a separate slow-restart window noted inline.
Validation: static review only in this GitHub Actions checkout. I did not run FE/BE builds or regression tests; .worktree_initialized, thirdparty/installed, and thirdparty/installed/bin/protoc are absent.
Contributor
TPC-DS: Total hot run time: 180008 ms |
### What problem does this PR solve? Issue Number: None Problem Summary: Debug-point parameter lookups could consume an execute-limited point more than once, and the docker regression test could observe the existing backend-down path instead of proving process-epoch restart detection. Reuse the matched debug point parameters, keep the BE logically alive during restart, wait until FE observes a changed start time, and align table cleanup and naming with regression conventions. ### Release note No user-visible behavior change beyond the backend-restart INSERT fix already described by this PR. ### Check List (For Author) - Test: Not run per request; git diff --check passed. - Behavior changed: No additional production behavior change. - Does this need documentation: No.
Contributor
ClickBench: Total hot run time: 24.99 s |
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
I reviewed the updated diff after the prior comments. The load join health check now scans the full unfinished backend-fragment task set rather than only top fragments, and the process-epoch comparison is guarded so zero captured/current epochs remain compatible with upgrade/replay cases. The writer debug-point parameter reads now use the macro-provided dp, avoiding the execute-limited double lookup, and the docker regression test now synchronizes on the writer-close debug marker plus FE-observed alive restarted backend before asserting the epoch error.
I did not find a distinct new issue to raise.
Review focus: no additional user-provided focus was supplied.
Critical checkpoint conclusions:
- Goal/test coverage: the PR addresses fail-fast detection for load fragments after BE restart; FE unit coverage and a docker regression are present.
- Scope: the current change is focused on load join health checking, process-epoch compatibility, debug-point test support, and the regression.
- Concurrency/lifecycle: load report/latch behavior and heartbeat epoch updates were reviewed; the full unfinished-task scan and coordinator status update appear consistent with the existing lifecycle.
- Compatibility: zero process epochs are ignored on either side for upgrade/replay compatibility; no storage format or FE-BE protocol compatibility issue found.
- Parallel paths: both VTabletWriter implementations use the same debug-point parameter pattern, and the load health check covers all backend-fragment tasks represented in the full latch.
- Observability/performance: the new failure status includes backend and epoch values, and the debug log is gated by debug-point execution; no unresolved performance concern found.
- Tests: test intent and regression standards look satisfied in the current diff.
Validation here was static review only; I did not run builds or tests in this review runner.
Contributor
TPC-H: Total hot run time: 29535 ms |
Contributor
TPC-DS: Total hot run time: 180133 ms |
Contributor
ClickBench: Total hot run time: 25.03 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 14, 2026
…65525) ### What problem does this PR solve? Introduced by #41730 Nereids INSERT waits for fragment completion through LoadProcessor. The backend health check only considered the alive flag, so a quickly restarted backend could look healthy even though the old process could no longer report fragment completion. Capture the backend process epoch when fragment tasks are created, compare it while joining, and propagate a precise restart error through the existing coordinator cancellation path.
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 14, 2026
…65525) ### What problem does this PR solve? Introduced by #41730 Nereids INSERT waits for fragment completion through LoadProcessor. The backend health check only considered the alive flag, so a quickly restarted backend could look healthy even though the old process could no longer report fragment completion. Capture the backend process epoch when fragment tasks are created, compare it while joining, and propagate a precise restart error through the existing coordinator cancellation path.
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?
Introduced by #41730
Nereids INSERT waits for fragment completion through LoadProcessor. The backend health check only considered the alive flag, so a quickly restarted backend could look healthy even though the old process could no longer report fragment completion. Capture the backend process epoch when fragment tasks are created, compare it while joining, and propagate a precise restart error through the existing coordinator cancellation path.