[fix](binlog) Fix row binlog routing for auto partitions - #66665
Merged
hello-stephen merged 1 commit intoAug 13, 2026
Merged
Conversation
Issue Number: None
Related PR: None
Problem Summary: Creating an automatic partition returned the row binlog index as a regular partition index even though the sink schema carries row binlog separately. BE therefore rejected the partition because its index count did not match the schema. The response also omitted the base tablet mapping required to route row binlog writes. Exclude row binlog indexes from partition metadata, return their tablet locations with base tablet IDs, and reuse colocated replica placement in local mode.
Fix imports into auto-partitioned tables with row binlog enabled.
- Test: Regression test / Unit Test
- Unit Test: FrontendServiceImplTest (20 tests)
- Regression test: row_binlog_p0/test_row_binlog_auto_partition
- Behavior changed: Yes. Auto partition imports now route row binlog tablets correctly.
- Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
morrySnow
changed the title
Contributor
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.
Automated review complete: no blocking issues found and no inline comments are proposed.
The change fixes the FE/BE contract for row-binlog auto partitions: TOlapTablePartition.indexes now contains only the ordinary indexes represented by the sink schema, while companion row-binlog tablets are returned as locations carrying the existing base_tablet_id mapping. The same construction is used by auto-create and insert-overwrite replacement, and local mode reuses the established same-disk intersection logic.
Critical checkpoint conclusions:
- Goal and proof: The implementation addresses both reported failure modes (index-count mismatch and missing companion routing). The FE unit test checks ordinary-index cardinality and every returned row-binlog base-tablet ID; the regression test performs a real two-partition insert and verifies both table rows and row-binlog rows with deterministic ordering.
- Scope and clarity: The production change is focused on one shared snapshot representation plus reuse of the existing colocation helper. No unrelated behavior was changed.
- Concurrency and lifecycle: Partition/index/tablet relationships are captured while the table read lock is held. The existing concurrent auto-partition cache retains the complete
TTabletLocationobjects, includingbase_tablet_id, and create/replace callbacks install locations before incremental channel opening. No new threads, lock ordering, static initialization, or resource-lifetime mechanism is introduced. - Configuration and compatibility: No configuration, storage format, function symbol, or persistence format changes are introduced.
base_tablet_idis an existing optional Thrift field. Ordinary schema indexes and row-binlog schemas remain separately transmitted, matching all BE partition constructors; row-binlog sinks remain explicitly gated away from the V2 writer that does not consume this mapping. - Parallel and conditional paths: Reviewed create versus replace, cache hit versus miss, local versus cloud routing, adaptive-random-bucket handling, rollup/shadow index ordering, and replacement-result conversion. Extra row-binlog locations do not enter ordinary index or adaptive bucket assignment; cloud mode uses the intended base-tablet fallback independent of node placement.
- Transactions and data correctness: On local nodes without a colocated companion, BE omits the row-binlog writer, but the transaction's loaded-index set includes the row-binlog index. Normal commit, 2PC precommit, and publish enforce its configured load-required replica count before visibility, so a partial base-only result cannot become visible. There are no delete-bitmap or visible-version changes.
- Error handling and observability: The new invariant failure for a missing base tablet is explicit. Existing commit/quorum errors and tablet identifiers provide the relevant failure evidence; no additional metric or logging path is required for this focused routing fix.
- Performance and memory: Work remains linear in the number of tablets/replicas. The extra companion snapshot/location objects are required for routing, with no new heavy work under locks or untracked BE allocation.
- Test standards/results: The new regression drops before use, orders both result sets, and includes the generated output. The PR reports the FE test class and regression case as passed; visible CI style/license/title checks are also passing. Per the review-runner instruction, this review did not rerun builds or tests.
- Additional user focus: No extra review focus was provided. The full PR was reviewed.
Review convergence: both complete-review agents and the separate risk-focused agent returned NO_NEW_VALUABLE_FINDINGS; the one degraded-colocation suspicion was dismissed with end-to-end commit/precommit/publish evidence. The final changed-file and unresolved-candidate sweep found no remaining issue.
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 28891 ms |
Contributor
TPC-DS: Total hot run time: 158357 ms |
Contributor
ClickBench: Total hot run time: 23.79 s |
hello-stephen
approved these changes
Aug 13, 2026
wyxxxcat
pushed a commit
to wyxxxcat/doris
that referenced
this pull request
Aug 17, 2026
Problem Summary: Creating an automatic partition returned the row binlog index as a regular partition index even though the sink schema carries row binlog separately. BE therefore rejected the partition because its index count did not match the schema. The response also omitted the base tablet mapping required to route row binlog writes. Exclude row binlog indexes from partition metadata, return their tablet locations with base tablet IDs, and reuse colocated replica placement in local mode. ### Release note Fix imports into auto-partitioned tables with row binlog enabled.
69 tasks
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
2 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: Creating an automatic partition returned the row binlog index as a regular partition index even though the sink schema carries row binlog separately. BE therefore rejected the partition because its index count did not match the schema. The response also omitted the base tablet mapping required to route row binlog writes. Exclude row binlog indexes from partition metadata, return their tablet locations with base tablet IDs, and reuse colocated replica placement in local mode.
Release note
Fix imports into auto-partitioned tables with row binlog enabled.
Check List (For Author)
Test
Unit Test:
FrontendServiceImplTest(20 tests passed).Regression test:
row_binlog_p0/test_row_binlog_auto_partition.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)