[fix](fe) Avoid eager allocations in cloud tablet indexes - #66378
Merged
Conversation
### What problem does this PR solve? Issue Number: None Related PR: apache#61318 Problem Summary: Cloud tablet route-cache rebuilding calls fillBeToTablets for current and future placements. The previous putIfAbsent calls eagerly constructed up to six candidate maps and sets for every placement even when the keys already existed, and then performed separate lookups. Use the containers returned by computeIfAbsent at every level and add a focused unit test that verifies all six container paths while preserving the global, table, and partition index contents. An isomorphic multi-scale JVM allocation model with 2 million entities, four-route fan-out, current/future passes, and three index scopes estimated cumulative allocation per modeled rebuild at 12.52 GiB before and 8.59 GiB after the change, saving 3.93 GiB (31.4%). For 4 million tablets, the fill-path cumulative-allocation reduction is expected to remain about 31% under comparable topology, while the absolute GiB saving depends on cluster and replica fan-out. The retained graph remained about 3.76 GiB and the peak proxy about 966 MiB, so this change does not claim a comparable steady-state heap or process-wide reduction. Timing samples were noisy, so no throughput improvement is claimed. ### Release note None ### Check List (For Author) - Test: Unit Test / Manual benchmark - ./run-fe-ut.sh --run org.apache.doris.cloud.catalog.CloudTabletRebalancerTest#testFillBeToTabletsUsesComputedContainers - cd fe && mvn checkstyle:check -pl fe-core - Multi-scale isomorphic JVM allocation model: 12.52 GiB to 8.59 GiB cumulative allocation per modeled rebuild - Behavior changed: No - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
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.
Review opinion: looks good. I found no blocking issue or substantiated inline finding.
Critical checkpoint conclusions:
- Goal and proof: the change replaces all six eager candidate-container allocations and follow-up lookups in
fillBeToTabletswith the installed values returned bycomputeIfAbsent. The focused unit test distinguishes the old implementation at all six levels and verifies the global, table, and partition/index tablet contents. - Scope and clarity: the production change is small, focused, and reuses one existing helper for current and future route-cache rebuilds, direct balance moves, and warmup rollback. No parallel fill path was missed.
- Concurrency: route-map writes occur in the single
CloudTabletRebalancermaster-daemon flow; warmup workers enqueue failures for that daemon rather than mutating these maps directly. The mapping functions are short, non-null, and non-reentrant, andcomputeIfAbsentreturns the installed container identity before the nested mutation. No outer-entry removal or replacement can detach that identity. - Lifecycle: per-cycle maps are built and then published as before; later move/rollback operations mutate the same installed concurrent sets. No initialization, ownership, cleanup, or release behavior changes.
- Conditions, configuration, compatibility, and persistence: no new condition, configuration, protocol, symbol, storage format, FE/BE variable, EditLog, transaction, or data-write behavior is introduced. Error handling and rolling-upgrade behavior are unchanged.
- Performance: present keys no longer allocate discarded maps/sets or require a separate explicit lookup. The change adds no scan, retained state, or heavy work under a lock, and it appropriately makes no throughput claim.
- Tests and result review: the new single-threaded counter test is deterministic, fails the prior
putIfAbsent/getimplementation, exercises absent and present backend-set keys plus present nested containers, and checks all three result scopes. The live CheckStyle check is green. The FE unit-test job is still pending; no local build or test was run because this review runner explicitly prohibits builds. - Observability: this is a behavior-preserving local allocation refactor with existing operational paths unchanged, so no new log or metric is warranted.
- User focus: no additional focus was provided; the complete PR was reviewed.
Review completion: Round 1 included a complete-review subagent and a separate risk-focused concurrency/test-oracle subagent. Both returned NO_NEW_VALUABLE_FINDINGS; the final changed-file, candidate, live-thread, and head/base sweeps found no unresolved point. Inline comments: zero.
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 28710 ms |
Contributor
TPC-DS: Total hot run time: 170674 ms |
Contributor
ClickBench: Total hot run time: 24 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
gavinchou
approved these changes
Aug 3, 2026
16 tasks
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 4, 2026
Problem Summary: Cloud tablet route-cache rebuilding calls fillBeToTablets for current and future placements. The previous putIfAbsent calls eagerly constructed up to six candidate maps and sets for every placement even when the keys already existed, and then performed separate lookups. Use the containers returned by computeIfAbsent at every level and add a focused unit test that verifies all six container paths while preserving the global, table, and partition index contents. An isomorphic multi-scale JVM allocation model with 2 million entities, four-route fan-out, current/future passes, and three index scopes estimated cumulative allocation per modeled rebuild at 12.52 GiB before and 8.59 GiB after the change, saving 3.93 GiB (31.4%). For 4 million tablets, the fill-path cumulative-allocation reduction is expected to remain about 31% under comparable topology, while the absolute GiB saving depends on cluster and replica fan-out. The retained graph remained about 3.76 GiB and the peak proxy about 966 MiB, so this change does not claim a comparable steady-state heap or process-wide reduction. Timing samples were noisy, so no throughput improvement is claimed.
deardeng
added a commit
that referenced
this pull request
Aug 4, 2026
Problem Summary: Cloud tablet route rebuilding repeatedly boxes primitive backend, table, partition, index, and tablet IDs while inserting the same logical IDs into current and future global, table, and partition indexes. This PR contains two related improvements: 1. Reuse boxed IDs across route indexes Hoist boxing to the traversal and incremental update callers, then pass the same immutable `Long` references through a boxed helper overload. Preserve the public primitive method descriptor and the existing `putIfAbsent` container implementation so the optimization applies directly to master without depending on lazy container creation. 2. Preserve in-flight tablet IDs across route rebuilds An asynchronous warmup task may remain in flight when the next rebalance round rebuilds route information. Previously, the rebuild stored a newly boxed tablet ID in the current and future indexes. If the warmup subsequently failed, rollback restored the original boxed ID held by the task only to the future indexes, while the current indexes retained the newly boxed instance. The fix reuses `InfightTask.pickedTabletId` when rebuilding both current and future route indexes. This preserves boxed-ID identity across the global, table-level, and partition-level indexes without changing routing decisions, destination backends, or scheduling semantics. It also prevents one additional `Long` wrapper from being retained for each affected failed in-flight tablet until the next route rebuild. A single-threaded JDK 17 allocation model that keeps eager container candidates in both variants estimates that 4 million tablets across 4 clusters reduce cumulative allocation from 25.47 GiB to 19.76 GiB (22.42%) and approximate post-full-GC retained heap from 6.80 GiB to 4.94 GiB (27.37%). These are path-level model estimates rather than production RSS measurements. The allocation estimates cover the boxed-ID reuse optimization. The cross-generation warmup rollback fix provides an additional lifecycle correctness guarantee and is not included in those estimates. related PR #66378
This was referenced Aug 4, 2026
deardeng
added a commit
to deardeng/incubator-doris
that referenced
this pull request
Aug 5, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#66451, apache#66378, apache#66389 Problem Summary: Cloud tablet route indexes are rebuilt before smooth-upgrade migration, but the scheduling-index cleanup fence previously covered only performBalancing(). If migration or the optional second route rebuild threw after the first rebuild, the daemon propagated the failure without replacing the four current/future table-level and partition-level indexes, retaining their tablet membership graphs until a later cycle. Move the existing try/finally boundary before the first statRouteInfo() call so route rebuild, smooth-upgrade migration, the optional second rebuild, and balancing all release the scheduling-only indexes on every exit path. Global route indexes and exception propagation remain unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.cloud.catalog.CloudTabletRebalancerTest#testRunAfterCatalogReadyReleasesSchedulingIndexesWhenMigrationFails - ./run-fe-ut.sh --run org.apache.doris.cloud.catalog.CloudTabletRebalancerTest (17 tests passed) - mvn checkstyle:check -pl fe-core (0 violations) - Behavior changed: No - Does this need documentation: No
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
3 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.
Problem Summary: Cloud tablet route-cache rebuilding calls fillBeToTablets for current and future placements. The previous putIfAbsent calls eagerly constructed up to six candidate maps and sets for every placement even when the keys already existed, and then performed separate lookups. Use the containers returned by computeIfAbsent at every level and add a focused unit test that verifies all six container paths while preserving the global, table, and partition index contents.
An isomorphic multi-scale JVM allocation model with 2 million entities, four-route fan-out, current/future passes, and three index scopes estimated cumulative allocation per modeled rebuild at 12.52 GiB before and 8.59 GiB after the change, saving 3.93 GiB (31.4%). For 4 million tablets, the fill-path cumulative-allocation reduction is expected to remain about 31% under comparable topology, while the absolute GiB saving depends on cluster and replica fan-out. The retained graph remained about 3.76 GiB and the peak proxy about 966 MiB, so this change does not claim a comparable steady-state heap or process-wide reduction. Timing samples were noisy, so no throughput improvement is claimed.
Release note
None
Check List (For Author)
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)