[fix](cloud) modify primary cluster bes to be in CloudReplica to reduce memory - #59932
Conversation
|
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 optimizes memory usage in CloudReplica by changing the storage of primary cluster backend IDs from a List to a single Long value. The refactoring reflects the reality that each replica is mapped to only one backend per cluster, despite the previous data structure suggesting multiple backends.
Changes:
- Refactored CloudReplica to use a single Long instead of List for primary cluster backend mapping
- Made Replica class abstract with abstract methods for getBackendId() and checkVersionCatchUp()
- Moved method implementations from Replica to LocalReplica to support the abstract class pattern
- Added migration logic via GsonPostProcessable interface to ensure backward compatibility during deserialization
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| CloudReplica.java | Replaced primaryClusterToBackends (Map<String, List>) with primaryClusterToBackend (Map<String, Long>) to reduce memory; added gsonPostProcess migration logic; updated all usages |
| Replica.java | Converted to abstract class; made getBackendId() and checkVersionCatchUp() abstract methods |
| LocalReplica.java | Implemented checkVersionCatchUp() abstract method with the original logic from Replica class |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
run buildall |
TPC-H: Total hot run time: 31453 ms |
TPC-DS: Total hot run time: 173602 ms |
ClickBench: Total hot run time: 26.74 s |
FE UT Coverage ReportIncrement line coverage |
| = new ConcurrentHashMap<String, List<Long>>(); | ||
| private ConcurrentHashMap<String, List<Long>> primaryClusterToBackends = null; | ||
| @SerializedName(value = "be") | ||
| private ConcurrentHashMap<String, Long> primaryClusterToBackend = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
The type of value has to be Long or long works too.
|
run cloud_p0 |
FE Regression Coverage ReportIncrement line coverage |
|
PR approved by anyone and no changes requested. |
|
PR approved by at least one committer and no changes requested. |
### What problem does this PR solve? #59932 modify bes to be in CloudReplica, and will merge into 4.1. This pr add compatible code for CloudReplica to make 4.1 can downgrade to 4.0. Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
### What problem does this PR solve? apache#59932 modify bes to be in CloudReplica, and will merge into 4.1. This pr add compatible code for CloudReplica to make 4.1 can downgrade to 4.0. Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
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)