Uh oh!
There was an error while loading. Please reload this page.
branch-4.0: [opt](cloud) cache cluster id per query and drop redundant locks on getBackendId hot path #63636 - #64275
Conversation
hello-stephen
commented
Jun 9, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
There was a problem hiding this comment.
Pull request overview
This PR backports #63636 to optimize cloud-mode hot paths by resolving the current compute-group/cluster id once per request and reusing it across many tablet/replica lookups, reducing repeated ConnectContext/priv/status/auto-start checks and lock contention.
Changes:
- Added
CloudSystemInfoService.getCurrentClusterId()/resolveClusterIdByName()and a fastcontainsCloudCluster()helper. - Updated
CloudReplica/CloudTabletand multiple call sites (planner + FE service) to reuse a cached cluster id when mapping replicas/tablets to backend ids. - Added a small unit test for
containsCloudCluster.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| fe/fe-core/src/test/java/org/apache/doris/cloud/system/CloudSystemInfoServiceTest.java | Adds a unit test for the new containsCloudCluster() helper. |
| fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java | Caches cluster id per request when building tablet backend mappings for partition operations. |
| fe/fe-core/src/main/java/org/apache/doris/planner/OlapTableSink.java | Caches cluster id during sink location generation and improves exception chaining. |
| fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java | Caches cluster id while selecting backend ids for cloud replicas during scan-range generation. |
| fe/fe-core/src/main/java/org/apache/doris/cloud/system/CloudSystemInfoService.java | Introduces centralized “current cluster id” resolution pipeline and containsCloudCluster(). |
| fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudTablet.java | Adds getNormalReplicaBackendPathMapByClusterId() and makes default path resolve cluster id first. |
| fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudReplica.java | Adds getBackendIdWithClusterId() and refactors cluster-id resolution into CloudSystemInfoService. |
| fe/fe-core/src/main/java/org/apache/doris/alter/AlterJobV2.java | Updates a comment to reflect the new cluster-id resolution entry point. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…etBackendId hot path (apache#63636) `CloudReplica.getBackendId` was the dominant FE hotspot in cloud-mode query/load planning. On a profiled query it accounted for **65.6% of total FE CPU samples** (async-profiler). Every replica in the plan re-ran the full cluster-id resolution pipeline even though the resolved cluster id is identical for every tablet in the same request. Call breakdown from the flame graph: ``` Tablet.getNormalReplicaBackendPathMap 68.6% └─ CloudReplica.getBackendId 65.6% └─ getCurrentClusterId 61.5% ├─ getCloudClusterIdByName 35.6% │ ├─ getCloudClusterNames 24.7% (rw-lock + ArrayList + stream.sorted) │ └─ waitForAutoStart 9.7% (StopWatch.start/stop even on NORMAL) ├─ getPhysicalCluster 14.5% │ └─ getComputeGroupByName 14.5% (rw-lock around ConcurrentHashMap) └─ getCloudStatusByName 8.3% └─ getCloudStatusByIdNoLock 14.0% (two stream passes, String.valueOf per BE) ``` `ReadLock.unlock` consistently outweighed `ReadLock.lock` in the profile -- a classic cache-line bouncing signature, meaning the read-lock CAS was already operating in the non-linear regime where adding concurrent threads disproportionately hurts throughput. With high tablet counts the call rate (`tablets × replicas × concurrent_queries`) easily reached 100k+ lock/unlock per second per FE, putting the FE one nudge away from a metastable collapse. - **`CloudReplica.getCurrentClusterId`** becomes `public static` so callers can resolve once per request. Adds `getBackendIdWithClusterId(String)` that bypasses the per-replica pipeline. - **`OlapTableSink.createLocation`** and **`FrontendServiceImpl.{createPartition, replacePartition}`** resolve the cluster id once before iterating tablets and pass it down via the new `CloudTablet.getNormalReplicaBackendPathMap(String)` overload. For a 10k-tablet query this collapses 10k full pipelines into 1. - **`CloudSystemInfoService.getComputeGroupByName`**: drop the rw-lock around `ConcurrentHashMap` reads, merge `containsKey+get` into a single `get`, guard the debug log behind `isDebugEnabled` (the map `toString` is expensive). - **`CloudSystemInfoService.containsCloudCluster`** (new): cheap existence check that replaces `getCloudClusterNames().contains(name)`. Avoids an ArrayList copy + stream filter + natural sort + collect under a read lock for a single existence query. - **`CloudSystemInfoService.getCloudStatusByIdNoLock`**: single-pass loop with a precomputed `NORMAL` constant in place of two stream pipelines that re-evaluated `String.valueOf(NORMAL)` per backend. - **`CloudSystemInfoService.waitForAutoStart`**: fast-path return when the cluster is already `NORMAL`, skipping the `withTemporaryNereidsTimeout` wrap and the `StopWatch.start/stop` inside `waitForClusterToResume` (the while loop never executed in that state but the wrapping still ran per call -- ~3% of total FE CPU in the profile). Semantics preserved on all paths: - `waitForAutoStart` NORMAL fast-path: the original code already had `existAliveBe = true` as initializer, so `waitForClusterToResume` was a no-op for NORMAL clusters. - `getComputeGroupByName` without rw-lock: the brief window between a rename's two map updates can now return `null`; same as the existing read-only paths everywhere else in this class that already access these maps without locks. - `containsCloudCluster` matches `getCloudClusterNames().contains(name)` for non-empty `name` (empty-name filtering in `getCloudClusterNames` was for the returned list, not for `.contains` semantics). The cluster id resolution is hoisted only on the no-BE-endpoint paths (the hot ones in the profile). The endpoint-resolved path in `FrontendServiceImpl` already has its own resolution logic and is untouched.
5ef44ec to
d8a5a81Compareliaoxin01
commented
Jun 9, 2026
run buildall |
hello-stephen
commented
Jun 9, 2026
FE UT Coverage ReportIncrement line coverage |
liaoxin01
commented
Jun 23, 2026
@copilot resolve the merge conflicts in this pull request |
### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#64275 Problem Summary: Resolve the branch-4.0 merge conflict in FrontendServiceImpl by preserving both the upstream loadToSingleTablet handling and the PR's cached cloud cluster id lookup for CloudTablet backend path resolution. ### Release note None ### Check List (For Author) - Test: Manual test - Ran git diff --check successfully for the resolved working tree. - Attempted ./run-fe-ut.sh --run org.apache.doris.cloud.system.CloudSystemInfoServiceTest, but it failed during fe-common compilation before running the target test. The compile errors were in existing fe-common Lombok-generated methods/log fields under the local Java 26 environment, unrelated to the resolved FrontendServiceImpl conflict. - git diff --cached --check reports whitespace in upstream branch-4.0 files brought in by the merge, not in the conflict resolution file. - Behavior changed: No - Does this need documentation: No
liaoxin01
commented
Jun 24, 2026
run buildall |
Uh oh!
There was an error while loading. Please reload this page.
Pick #63636