[opt](memory) Remove unused fields from CloudReplica#62125
Merged
gavinchou merged 3 commits intoapache:masterfrom Apr 7, 2026
Merged
[opt](memory) Remove unused fields from CloudReplica#62125gavinchou merged 3 commits intoapache:masterfrom
gavinchou merged 3 commits intoapache:masterfrom
Conversation
Remove segmentCount, rowsetCount, and memClusterToBackends from CloudReplica. Move segmentCount and rowsetCount storage to the base Replica class so all replica types can use them directly. Remove the memClusterToBackends in-memory cache and its associated initMemClusterToBackends method, simplifying the multi-replica backend selection path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Keep the base Replica class lean by using no-op setters instead of storing the values. These counts are not needed for non-cloud replicas. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…llers Remove getSegmentCount/setSegmentCount/getRowsetCount/setRowsetCount from Replica base class and clean up all call sites in CloudTabletStatMgr and Checkpoint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
run buildall |
Contributor
There was a problem hiding this comment.
Pull request overview
Reduces memory footprint in cloud FE metadata by removing unused per-CloudReplica fields/caches and consolidating replica stat storage.
Changes:
- Removed
segmentCount/rowsetCountfields and overrides fromCloudReplica. - Removed
memClusterToBackendscache and related init/update logic from cloud multi-replica backend selection. - Added
segmentCount/rowsetCountstorage + serialization to baseReplica.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudReplica.java | Drops unused fields/cache and simplifies multi-replica BE selection path. |
| fe/fe-core/src/main/java/org/apache/doris/catalog/Replica.java | Adds stat fields + (de)serialization and enables base getters/setters to store values. |
Comments suppressed due to low confidence (2)
fe/fe-core/src/main/java/org/apache/doris/catalog/Replica.java:110
- Moving
segmentCount/rowsetCountstorage into the baseReplicaclass adds two longs to all replica types (e.g.LocalReplica), even though these stats appear to be cloud-only (Prometheus export is gated byConfig.isNotCloudMode()). This is likely a net memory increase for non-cloud deployments and seems at odds with the PR’s memory-optimization goal. Consider keeping these fields inCloudReplica(with overrides) or introducing a cloud-specific replica base/type so local replicas don’t pay the per-object cost.
public Replica() {
}
fe/fe-core/src/main/java/org/apache/doris/catalog/Replica.java:110
CloudReplicapreviously defaultedrowsetCountto 1 (// [0-1] rowset), but the new base field initializes it to 0. If persisted images (or newly constructed replicas) rely on the prior default, this changes reported stats and could affect the tablet-stats polling interval logic that compares previous vs new rowset counts. Consider preserving the prior default for cloud replicas (e.g., initialize to 1 inCloudReplicaconstructors or document/justify the new default).
public Replica() {
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
313
to
317
| if (Config.enable_cloud_multi_replica) { | ||
| int indexRand = rand.nextInt(Config.cloud_replica_num); | ||
| int coldReadRand = rand.nextInt(100); | ||
| boolean allowColdRead = coldReadRand < Config.cloud_cold_read_percent; | ||
| initMemClusterToBackends(); | ||
| boolean replicaEnough = memClusterToBackends.get(clusterId) != null | ||
| && memClusterToBackends.get(clusterId).size() > indexRand; | ||
|
|
||
| long backendId = -1; | ||
| if (replicaEnough) { | ||
| backendId = memClusterToBackends.get(clusterId).get(indexRand); | ||
| } | ||
|
|
||
| if (!replicaEnough && !allowColdRead && primaryClusterToBackend.containsKey(clusterId)) { | ||
| backendId = primaryClusterToBackend.get(clusterId); | ||
| } | ||
|
|
||
| if (backendId > 0) { | ||
| Backend be = Env.getCurrentSystemInfo().getBackend(backendId); | ||
| if (be != null && be.isQueryAvailable()) { | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("backendId={} ", backendId); | ||
| } | ||
| return backendId; | ||
| } | ||
| } | ||
|
|
||
| List<Long> res = hashReplicaToBes(clusterId, false, Config.cloud_replica_num); | ||
| if (res.size() < indexRand + 1) { |
There was a problem hiding this comment.
enable_cloud_multi_replica path now returns a backend ID from hashReplicaToBes(...) without validating that the selected backend is query-available. hashReplicaToBes filters by isAlive() / heartbeat time, not isQueryAvailable(), so this can route queries to an unavailable backend. Consider filtering by be.isQueryAvailable() (like hashReplicaToBe) or checking availability for the chosen ID and falling back to another candidate (or primary/secondary mapping) before returning.
TPC-H: Total hot run time: 29262 ms |
TPC-DS: Total hot run time: 178368 ms |
Yukang-Lian
approved these changes
Apr 6, 2026
Contributor
|
PR approved by anyone and no changes requested. |
gavinchou
approved these changes
Apr 7, 2026
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions Bot
pushed a commit
that referenced
this pull request
Apr 7, 2026
- Remove segmentCount, rowsetCount fields from CloudReplica, moving storage to base Replica class - Remove memClusterToBackends in-memory cache and initMemClusterToBackends() method - Simplifies multi-replica backend selection path in getBackendIdImpl()
iaorekhov-1980
pushed a commit
to iaorekhov-1980/doris
that referenced
this pull request
Apr 7, 2026
- Remove segmentCount, rowsetCount fields from CloudReplica, moving storage to base Replica class - Remove memClusterToBackends in-memory cache and initMemClusterToBackends() method - Simplifies multi-replica backend selection path in getBackendIdImpl()
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.
Summary
segmentCount,rowsetCountfields and their getter/setter overrides fromCloudReplica, moving the storage to the baseReplicaclassmemClusterToBackendsin-memory cache andinitMemClusterToBackends()method, simplifying the multi-replica backend selection path ingetBackendIdImpl()dbIdfieldTest plan
run buildall🤖 Generated with Claude Code