Repository navigation
[fix](be) Replay schema-change WAL with duplicate column IDs - #70160
Merged
liaoxin01 merged 2 commits intoNov 5, 2026
Merged
Conversation
### What problem does this PR solve? Issue Number: N/A Related PR: apache#66008 Problem Summary: Heavy schema changes retain the original column ID on a shadow column. Group Commit can therefore write both columns with the same ID into its WAL. The V2 reader rejects this valid header as corruption, leaving asynchronous writes unreplayed. Preserve all physical header entries and reuse the existing first-ID mapping and type conversion, matching the legacy reader's column selection. ### Release note Fix Group Commit WAL replay failures caused by duplicate column IDs during heavy schema changes when FileScannerV2 is enabled. ### Check List (For Author) - Test: ASAN BE build; 5 WalReaderV2Test unit tests; regression for replay during RUNNING, FINISHED and CANCELLED ALTER states; recovery of two original failing WAL files. clang-format v16, build hygiene and clang-tidy checks passed. Unit tests disabled the redundant GLIBC compatibility layer for the local toolchain. - Behavior changed: Yes, accept valid WAL headers containing base and shadow columns with the same ID. - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No actionable defects found in the four changed files at head 7b8ce08. Accepting duplicate WAL column IDs preserves physical positions and the existing first/base-column mapping, restoring replay of valid schema-change WALs. No existing inline findings require retention, and no additional user review focus was supplied.
Critical checkpoint conclusions:
- Goal and evidence: the writer-backed unit test distinguishes base/shadow values, preserves the trailing physical ordinal, and checks reordered INT-to-BIGINT mapping. The regression covers RUNNING, FINISHED and CANCELLED ALTER states, including NULL values.
- Scope and reuse: the production change removes only duplicate-ID rejection and its set; existing mapping and conversion machinery is reused.
- Concurrency and locks: parser state remains reader-local, with no new production synchronization. Regression blocking points distinguish FE completion from BE conversion so cancellation can acquire the FE job monitor.
- Lifecycle and memory safety: physical IDs remain distinct local ordinals; existing first-payload transfer, ownership moves, EOF and close behavior are unchanged. No new allocation ownership or static initialization dependency is introduced.
- Configuration: no production setting is added. The regression restores the global scanner flag in finally and disables its named debug points.
- Compatibility: WAL header/PBlock formats and execution-version checks are unchanged. Legacy emplace and v2 first-match field-ID lookup both retain the first occurrence; valid older duplicate-ID WALs become readable.
- Thrift/protobuf: no definitions, wire IDs, enums or function signatures change.
- Parallel paths: legacy replay already accepts duplicates. Shared WAL production/replay serves cloud and non-cloud modes; the regression selects the matching BE conversion hook.
- Conditions and errors: malformed-token, physical column-count, version and invalid-position failures remain. Duplicate IDs are valid schema-change metadata. Replay SQL deduplicates IDs before projection; TableReader performs file-to-table casts rather than reinterpreting payload bytes.
- Test coverage: focused unit and ordered regression coverage address this fix. New targeted fixtures do not cover duplicate-empty/multiple-block WALs, nested/other-type evolution or crash/failover transitions; these unchanged shared paths were inspected.
- Expected results: every output section correctly includes (1,10,100), (2,20,200), and (3,NULL,300), with order_qt ensuring deterministic comparison. Output-generation provenance was not independently verified.
- Observability: existing parser/schema/read errors and replay/commit logs remain sufficient for the changed admission behavior; no additional asynchronous stage is introduced.
- Persistence and transactions: writer bytes, labels, commit/rollback and FE logging are unchanged. Replay projects each live unique ID once and commits through the existing ordinary stream-load path.
- Writes, crashes and core invariants: this reader change adds no transaction or visibility boundary. Partition-version and MoW delete-bitmap logic is untouched; no new crash-cleanup or atomicity defect was found in the affected path.
- FE/BE variables: none are added, so no transmission paths require changes.
- Performance and further checks: parsing remains linear while removing hash-set work; no new per-row work, copies, cache/pruning changes, header-hygiene changes or ABI dependencies are introduced. Replay bypasses the held group-load error injection, and the bounded FE WAL-drain timeout allows the FINISHED scenario to proceed.
Static review only: no builds, tests, benchmarks or runtime validation were executed. Author-reported executions were not treated as independently verified results. All changed paths, candidate dispositions and both initial risk focuses were checked; no unresolved code question remains. Review converged after two rounds, including final confirmation of the frozen payload.
Review converged after 2 round(s).
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 28764 ms |
Contributor
TPC-DS: Total hot run time: 150792 ms |
Contributor
ClickBench: Total hot run time: 24.34 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Related PR: apache#70160, apache#69723 Problem Summary: Master always uses FileScannerV2 and exposes the deprecated rollout option under a prefixed SHOW name. The WAL schema-change case queries the old name, gets no rows and fails before testing replay. Remove the obsolete global-option setup and restoration. ### Release note None ### Check List (For Author) - Test: Regression test test_group_commit_schema_change_wal on the local integrated ASAN cluster (RUNNING, FINISHED and CANCELLED); existing expected output unchanged. - Behavior changed: No - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28277 ms |
Gabriel39
approved these changes
Nov 5, 2026
Contributor
TPC-DS: Total hot run time: 151429 ms |
Contributor
ClickBench: Total hot run time: 24.42 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
1 similar comment
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
gavinchou
approved these changes
Nov 5, 2026
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#70160
commit: cdd0b0bd02d42fb0b21b5ad28400d99e8e4531a3
base: 736649fcb41544ef977bb7677a1e6a5df5b7996b
reviewed_at: 2026-11-05T17:07+08:00
reviewer: liaoxin01
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
github-actions Bot
pushed a commit
that referenced
this pull request
Nov 5, 2026
### What problem does this PR solve? Heavy ALTER operations can write base and shadow columns with the same ID into Group Commit WALs. V2 rejects these valid headers as corruption. Accept duplicate IDs while preserving physical column order and the existing first-column mapping. Related PR: #66008 ### Release note Fix Group Commit WAL replay failures during heavy schema changes with FileScannerV2.
github-actions Bot
pushed a commit
that referenced
this pull request
Nov 5, 2026
### What problem does this PR solve? Heavy ALTER operations can write base and shadow columns with the same ID into Group Commit WALs. V2 rejects these valid headers as corruption. Accept duplicate IDs while preserving physical column order and the existing first-column mapping. Related PR: #66008 ### Release note Fix Group Commit WAL replay failures during heavy schema changes with FileScannerV2.
liaoxin01
added a commit
to liaoxin01/doris
that referenced
this pull request
Nov 6, 2026
### Background Group commit producers and consumers can be planned on different FEs. Their block layouts follow the table's full schema. ### What problem does this PR solve? Issue Number: None Related PR: apache#69660 (same layout issue), apache#70160 (upstream WAL replay compatibility) PR Type: Bugfix Problem Summary: Different index-map iteration orders can produce different MV column layouts across FEs, causing group commit cast errors or swapped values. ### Details - **Frequency:** Multiple synchronous MVs with producer and consumer plans generated on different FEs. - **Root cause:** Full-schema construction follows map iteration order, which can differ after metadata recovery. - **Why existing tests missed it:** Coverage missed recovery followed by adding another MV across FEs. - **Fix:** Keep base columns first, then visit indexes in ascending ID order and deduplicate columns by definition name. - **Other paths and branches:** Rollup, Schema Change and Gson recovery share this method. Mixed-version FEs can still generate different layouts during rollout. - **Behavior change:** Identical index metadata produces identical full-schema layouts across FEs. ### Testing - 44 FE unit tests passed. Both new regression tests fail before the fix and pass afterward. - FE server build with Checkstyle passed (UI disabled); baseline ASAN BE rebuilt. - Six-MV `multiple_ssb` passed through both FEs. All 54 output columns matched, including after follower restart. - Base-only, multi-MV and Schema Change WAL replay regressions passed through the follower. The initial ASAN run hit the local 16 GB memory cap; rerunning with 32 GB passed. ### Release note Fix group commit failures and incorrect MV values caused by inconsistent column ordering across FEs. ### Check List (For Author) - Behavior changed: Yes (see Details) - Does this need documentation: No - Pre-submission status: Ready; local checks passed, CI pending. ### AI Disclosure - Tool: Codex - Generated: Code, tests, and description - Human review: Pending author review
yiguolei
pushed a commit
that referenced
this pull request
Nov 10, 2026
…IDs #70160 (#70299) Cherry-picked from #70160 Co-authored-by: Xin Liao <[email protected]>
yiguolei
pushed a commit
that referenced
this pull request
Nov 10, 2026
…IDs #70160 (#70300) Cherry-picked from #70160 Co-authored-by: Xin Liao <[email protected]>
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?
Heavy ALTER operations can write base and shadow columns with the same ID into Group Commit WALs. V2 rejects these valid headers as corruption. Accept duplicate IDs while preserving physical column order and the existing first-column mapping.
Related PR: #66008
Release note
Fix Group Commit WAL replay failures during heavy schema changes with FileScannerV2.
Check List (For Author)