-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix](fe) move variables in Replica and Tablet to reduce memory #59648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the Replica and Tablet class hierarchies by moving local-mode-specific variables and logic from the base classes to dedicated subclasses (LocalReplica and LocalTablet), making the codebase more maintainable and better separating concerns between local and cloud modes.
Key Changes:
- Created LocalReplica class to hold local-mode-specific fields (version, backendId, pathHash, etc.) previously in the base Replica class
- Created LocalTablet class methods to override base class methods with local-mode-specific implementations
- Updated GsonUtils serialization to properly handle LocalReplica with backward compatibility for existing "Replica" serialized data
- Converted all direct instantiations of
new Replica()tonew LocalReplica()across test and production code
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| fe/fe-core/src/main/java/org/apache/doris/catalog/Replica.java | Base class refactored to remove local-specific fields, converted methods to throw UnsupportedOperationException or return default values |
| fe/fe-core/src/main/java/org/apache/doris/catalog/LocalReplica.java | New subclass containing all local-mode-specific fields and implementations moved from base Replica class |
| fe/fe-core/src/main/java/org/apache/doris/catalog/Tablet.java | Base class refactored to remove local-specific fields, methods now throw UnsupportedOperationException or return defaults |
| fe/fe-core/src/main/java/org/apache/doris/catalog/LocalTablet.java | Extended with local-specific fields and method overrides previously in base Tablet class |
| fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java | Updated to instantiate LocalReplica instead of Replica |
| fe/fe-core/src/main/java/org/apache/doris/catalog/EnvFactory.java | Updated factory methods to create LocalReplica instances |
| fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudReplica.java | Added segmentCount and rowsetCount fields with getter/setter overrides for cloud-mode specifics |
| fe/fe-core/src/main/java/org/apache/doris/persist/gson/GsonUtils.java | Updated serialization adapters to register LocalReplica with backward compatibility mappings |
| fe/fe-core/src/main/java/org/apache/doris/master/ReportHandler.java | Changed to instantiate LocalReplica for tablet reporting |
| fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java | Updated replica creation calls to use LocalReplica |
| fe/fe-core/src/main/java/org/apache/doris/clone/*.java | Added documentation comments indicating "Only used in local mode" for various checker/scheduler classes |
| fe/fe-core/src/main/java/org/apache/doris/clone/TabletSchedCtx.java | Updated to create LocalReplica instances |
| fe/fe-core/src/main/java/org/apache/doris/consistency/ConsistencyChecker.java | Added "Only used in local mode" documentation comment |
| fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java | Updated to instantiate LocalReplica for restore operations |
| fe/fe-core/src/test/java/**/*.java | Multiple test files updated to use LocalReplica instead of Replica |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TPC-H: Total hot run time: 31203 ms |
TPC-DS: Total hot run time: 173213 ms |
ClickBench: Total hot run time: 26.62 s |
1d0a920 to
6774340
Compare
|
run buildall |
TPC-H: Total hot run time: 31416 ms |
TPC-DS: Total hot run time: 173763 ms |
ClickBench: Total hot run time: 26.78 s |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
TPC-H: Total hot run time: 31840 ms |
TPC-DS: Total hot run time: 172482 ms |
FE UT Coverage ReportIncrement line coverage |
dataroaring
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
FE Regression Coverage ReportIncrement line coverage |
deardeng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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)