Repository navigation
[fix](build) Recover interrupted Paimon header and archive installation - #68633
Merged
yiguolei merged 2 commits intoSep 30, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
5 of 11 tasks
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed PR #68633 at f534dcf. Complete static review after two rounds; no new valuable inline findings and no existing blocking inline comments.
- Goal and tests: The new publisher stages and validates the Paimon header, marks the two-file publication incomplete before replacing the archive, and clears the marker after publishing the header. The build gate rebuilds on a missing, empty, or marked installation. The added Python cases cover first installs, replacements, injected copy/rename failures, retry, and gate repair. I did not run builds or tests under the review restrictions; the PR's claimed test results are not independent validation.
- Scope and behavior: The change is confined to the build gate, Paimon installer, and its test script. No new configuration, transmitted FE/BE variable, function symbol, storage format, or rolling-upgrade protocol is involved. The new gate conditions each correspond to an incomplete-install state.
- Concurrency and lifecycle: The in-repository build path runs third-party packages serially. The header staging, archive staging, EXIT cleanup, marker creation, two renames, and marker removal were traced through process-failure windows. No new lock, thread, static-initialization, or ownership lifecycle was added. This review does not establish host power-loss durability.
- Parallel paths and correctness: Direct Paimon package retries run the same publisher. The separate BE unit-test build bypasses the build.sh gate, as it did before this PR; no in-repository code currently includes the generated Paimon header. A nonempty header already truncated by the old installer can pass this gate, but that is an unmarked pre-existing artifact with no demonstrated current consumer or newly introduced failure. These points were independently rechecked and dismissed as new inline findings.
- Error handling, performance, and observability: Copy, strip, test, and rename failures propagate under shell errexit, while the marker preserves the recovery signal after publication starts. Failure remains visible through command status and the existing build log. A full third-party rebuild is triggered only when the new incomplete-state checks match; this is build-time work, not a runtime hot path.
- Other checkpoints: No Doris transaction, persistent metadata, data write, memory allocation, or distributed operation is changed. There are no generated regression result files to validate. No additional user-provided focus was supplied.
All three changed files and all candidate points received a final sweep. Review status: complete, with no accepted new findings.
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Completed static review of PR #68633 at head 1d749953c1e37b5ce5257ec4ac6960c52cf42abc after two convergence rounds. I found no new substantiated inline defect and no existing P0/P1 inline blocker.
Goal and tests: The Paimon publisher stages and validates the header before changing installed artifacts, marks the pair incomplete before publishing the archive, and clears the marker after publishing the header. The build gate rebuilds on missing or empty artifacts or a leftover marker. Added tests cover first and replacement installs, injected copy/rename failures, retry, empty files, and installed-only images. This was a static review: I did not run builds or tests, and the author's test claim is not independent verification. The Paimon Python suite is not invoked by the current third-party CI script job, which limits automatic coverage.
Scope, paths, and compatibility: The three-file change is focused. I traced build.sh through the full third-party rebuild, build_paimon_rust, the archive publisher, and the package loop, including Linux/macOS gates and source versus installed-only compilation images. The full rebuild preserves the existing Lance/Paimon toolchain alignment. There is no new FE/BE protocol, persisted format, configuration item, or runtime function symbol. A nonempty mixed pair left by the old installer before this patch has no marker and still cannot be recognized; this is a pre-existing recovery limit.
Concurrency and lifecycle: No runtime threads or locks are added. Shell command failures propagate through set -e; temporary-file traps do not erase the recovery marker. The marker covers interruption after the first installed-file change. Concurrent direct third-party builds are not coordinated, as in the base revision. There is no cross-translation-unit static initialization or runtime resource lifecycle.
Conditions, errors, and observability: The new checks target missing/empty files, the incomplete marker, and the absence of a rebuild script. Installed-only images fail with a clear error before installed is removed. No transaction, EditLog, database data-write, or FE/BE variable path is changed. File checks are cheap; the full rebuild on detected damage follows the existing dependency strategy. No new runtime metrics are warranted. No test-result file was changed, and no end-to-end third-party rebuild was verified in this review.
There were no additional user focus points. Review complete: all changed files and candidate points were swept, and all second-round reviewers returned NO_NEW_VALUABLE_FINDINGS.
Contributor
TPC-H: Total hot run time: 28145 ms |
Contributor
TPC-DS: Total hot run time: 152537 ms |
Contributor
ClickBench: Total hot run time: 24.07 s |
Contributor
TPC-H: Total hot run time: 27781 ms |
Contributor
TPC-DS: Total hot run time: 152401 ms |
Contributor
ClickBench: Total hot run time: 23.99 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
yiguolei
approved these changes
Sep 30, 2026
Contributor
|
PR approved by anyone and no changes requested. |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
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
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?
Related PR: #66227, #68562
An interrupted Paimon dependency installation can leave a truncated header or a header/archive pair from different builds that the next build incorrectly reuses.
Stage and validate the header before publishing either file, keep an incomplete-install marker until both publications succeed, and rebuild when that marker or an empty installed file is found. Tests inject header-copy and both rename failures for first installs and replacements, and verify successful repair on retry.
Release note
Fix reuse of incomplete Paimon dependency installations.
Check List (For Author)
python3 thirdparty/test/paimon-build-test.py(16 tests passed)bash -n build.sh thirdparty/build-thirdparty.shCheck List (For Reviewer who merge this PR)