Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Refactor] Refactor PartitionImpl to Logical Partition and Physical Partition #50598

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[Refactor] Refactor PartitionImpl to Logical Partition and Physical Partition #50598

wants to merge 6 commits into from
+2,220 −2,411

Conversation

HangyuanLiu
Copy link
Contributor

@HangyuanLiu HangyuanLiu commented Sep 3, 2024 •
edited by nshangyiming
Loading

Why I'm doing:

The original inheritance logic, Partition and PhysicalPartition each implement duplicate logic, which is prone to errors

What I'm doing:

  1. Delete the actual storage-related functions in Partition
  2. Use PhysicalPartition to store related logic
  3. Partition contains a PhysicalPartition by default, named defaultPhysicalPartition
  4. Add compatible logic in Partition deserialization
  5. For some existing logic that does not use PhysicalPartition, keep the status quo and use defaultPhysicalPartition

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 3.3
    • 3.2
    • 3.1
    • 3.0
    • 2.5

@HangyuanLiu HangyuanLiu requested review from a team as code owners September 3, 2024 01:46
@wanpengfei-git wanpengfei-git requested a review from a team September 3, 2024 01:46
@nshangyiming nshangyiming self-assigned this Sep 3, 2024
meegoo
meegoo previously approved these changes Sep 4, 2024
View reviewed changes
@HangyuanLiu HangyuanLiu force-pushed the 0829-logical-partition branch 3 times, most recently from 5a67eb5 to 6947d86 Compare September 6, 2024 01:57
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Sep 6, 2024

Quality Gate Passed Quality Gate passed

Issues
21 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.8% Duplication on New Code

See analysis details on SonarCloud

@gengjun-git gengjun-git self-assigned this Sep 6, 2024
gengjun-git
gengjun-git previously approved these changes Sep 6, 2024
View reviewed changes
gengjun-git
gengjun-git reviewed Sep 6, 2024
View reviewed changes
fe/fe-core/src/main/java/com/starrocks/catalog/Partition.java
}
}

/**************************************PhysicalPartition **********************************************/
Copy link
Contributor

@gengjun-git gengjun-git Sep 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some comments as to why these properties are kept?

@HangyuanLiu HangyuanLiu force-pushed the 0829-logical-partition branch 9 times, most recently from a9e8693 to 5be9714 Compare November 6, 2024 08:43
@HangyuanLiu HangyuanLiu force-pushed the 0829-logical-partition branch 5 times, most recently from dbfc8d9 to 7d549b1 Compare November 8, 2024 06:17
@HangyuanLiu
fix
cd1339b 
Signed-off-by: HangyuanLiu <[email protected]>
@HangyuanLiu
fix
f6955cd 
Signed-off-by: HangyuanLiu <[email protected]>
@HangyuanLiu HangyuanLiu force-pushed the 0829-logical-partition branch 3 times, most recently from 175fe14 to 6d19325 Compare November 10, 2024 06:52
@HangyuanLiu
fix
1a3d7a9 
Signed-off-by: HangyuanLiu <[email protected]>
@HangyuanLiu
fix
7e95356 
Signed-off-by: HangyuanLiu <[email protected]>
@HangyuanLiu
remove set physical id
373b031 
Signed-off-by: HangyuanLiu <[email protected]>
@HangyuanLiu HangyuanLiu force-pushed the 0829-logical-partition branch 2 times, most recently from 7501490 to 124b7fb Compare November 12, 2024 15:17
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Nov 12, 2024

Quality Gate Passed Quality Gate passed

Issues
29 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.3% Duplication on New Code

See analysis details on SonarQube Cloud

@github-actions GitHub Actions
Copy link

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

@github-actions GitHub Actions
Copy link

[FE Incremental Coverage Report]

✅ pass : 581 / 655 (88.70%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/http/rest/MigrationAction.java 0 2 00.00% [122, 148]
🔵 com/starrocks/sql/analyzer/AstToStringBuilder.java 0 1 00.00% [1847]
🔵 com/starrocks/common/proc/PartitionsProcDir.java 0 2 00.00% [442, 444]
🔵 com/starrocks/lake/backup/LakeRestoreJob.java 0 6 00.00% [102, 103, 131, 175, 281, 282]
🔵 com/starrocks/load/InsertOverwriteJobRunner.java 1 3 33.33% [440, 441]
🔵 com/starrocks/lake/LakeTable.java 2 5 40.00% [183, 184, 186]
🔵 com/starrocks/backup/RestoreJob.java 15 34 44.12% [501, 502, 540, 573, 574, 739, 759, 1034, 1041, 1046, 1047, 1146, 1178, 1200, 1201, 1712, 1713, 1725, 1749]
🔵 com/starrocks/alter/OptimizeJobV2.java 2 4 50.00% [578, 579]
🔵 com/starrocks/transaction/PublishVersionDaemon.java 1 2 50.00% [816]
🔵 com/starrocks/transaction/TableCommitInfo.java 1 2 50.00% [84]
🔵 com/starrocks/connector/partitiontraits/OlapPartitionTraits.java 4 7 57.14% [87, 88, 89]
🔵 com/starrocks/alter/OnlineOptimizeJobV2.java 3 5 60.00% [553, 554]
🔵 com/starrocks/load/OlapDeleteJob.java 2 3 66.67% [310]
🔵 com/starrocks/transaction/PartitionCommitInfo.java 3 4 75.00% [194]
🔵 com/starrocks/catalog/ExternalOlapTable.java 9 12 75.00% [507, 509, 514]
🔵 com/starrocks/catalog/Partition.java 44 53 83.02% [185, 250, 298, 299, 300, 302, 303, 304, 305]
🔵 com/starrocks/consistency/ConsistencyChecker.java 5 6 83.33% [594]
🔵 com/starrocks/statistic/sample/TabletSampleManager.java 16 19 84.21% [96, 98, 100]
🔵 com/starrocks/transaction/OlapTableTxnStateListener.java 8 9 88.89% [117]
🔵 com/starrocks/clone/DiskAndTabletLoadReBalancer.java 17 19 89.47% [1840, 1842]
🔵 com/starrocks/leader/LeaderImpl.java 9 10 90.00% [557]
🔵 com/starrocks/server/LocalMetastore.java 52 55 94.55% [1606, 1607, 4473]
🔵 com/starrocks/transaction/DatabaseTransactionMgr.java 22 23 95.65% [1124]
🔵 com/starrocks/planner/OlapScanNode.java 27 28 96.43% [437]
🔵 com/starrocks/catalog/PhysicalPartition.java 197 200 98.50% [142, 143, 444]
🔵 com/starrocks/catalog/MaterializedView.java 1 1 100.00% []
🔵 com/starrocks/scheduler/PartitionBasedMvRefreshProcessor.java 3 3 100.00% []
🔵 com/starrocks/alter/LakeTableAlterMetaJobBase.java 6 6 100.00% []
🔵 com/starrocks/replication/ReplicationJob.java 8 8 100.00% []
🔵 com/starrocks/service/FrontendServiceImpl.java 5 5 100.00% []
🔵 com/starrocks/alter/LakeRollupJob.java 18 18 100.00% []
🔵 com/starrocks/alter/LakeTableAlterJobV2Builder.java 2 2 100.00% []
🔵 com/starrocks/persist/PhysicalPartitionPersistInfoV2.java 1 1 100.00% []
🔵 com/starrocks/load/loadv2/SparkLoadJob.java 3 3 100.00% []
🔵 com/starrocks/sql/plan/PlanFragmentBuilder.java 1 1 100.00% []
🔵 com/starrocks/lake/LakeTableHelper.java 3 3 100.00% []
🔵 com/starrocks/catalog/OlapTable.java 13 13 100.00% []
🔵 com/starrocks/planner/MetaScanNode.java 2 2 100.00% []
🔵 com/starrocks/catalog/MaterializedIndex.java 2 2 100.00% []
🔵 com/starrocks/transaction/TransactionChecker.java 1 1 100.00% []
🔵 com/starrocks/load/TabletDeleteInfo.java 2 2 100.00% []
🔵 com/starrocks/planner/OlapTableSink.java 3 3 100.00% []
🔵 com/starrocks/http/rest/v2/vo/PartitionInfoView.java 5 5 100.00% []
🔵 com/starrocks/load/DeleteMgr.java 1 1 100.00% []
🔵 com/starrocks/catalog/mv/MVTimelinessArbiter.java 1 1 100.00% []
🔵 com/starrocks/mv/MVRepairHandler.java 1 1 100.00% []
🔵 com/starrocks/transaction/OlapTableTxnLogApplier.java 2 2 100.00% []
🔵 com/starrocks/statistic/StatisticExecutor.java 1 1 100.00% []
🔵 com/starrocks/transaction/LakeTableTxnLogApplier.java 2 2 100.00% []
🔵 com/starrocks/alter/LakeTableRollupBuilder.java 1 1 100.00% []
🔵 com/starrocks/transaction/LakeTableTxnStateListener.java 6 6 100.00% []
🔵 com/starrocks/clone/ColocateTableBalancer.java 2 2 100.00% []
🔵 com/starrocks/sql/optimizer/rule/tree/lowcardinality/DecodeCollector.java 2 2 100.00% []
🔵 com/starrocks/catalog/TempPartitions.java 1 1 100.00% []
🔵 com/starrocks/sql/common/MetaUtils.java 1 1 100.00% []
🔵 com/starrocks/sql/optimizer/rule/tree/AddDecodeNodeForDictStringRule.java 2 2 100.00% []
🔵 com/starrocks/statistic/StatisticUtils.java 3 3 100.00% []
🔵 com/starrocks/alter/LakeTableSchemaChangeJob.java 7 7 100.00% []
🔵 com/starrocks/planner/FragmentNormalizer.java 8 8 100.00% []
🔵 com/starrocks/transaction/TransactionState.java 1 1 100.00% []
🔵 com/starrocks/task/TabletTaskExecutor.java 8 8 100.00% []
🔵 com/starrocks/sql/optimizer/rule/mv/MaterializedViewRule.java 1 1 100.00% []
🔵 com/starrocks/consistency/CheckConsistencyJob.java 2 2 100.00% []
🔵 com/starrocks/catalog/CatalogUtils.java 1 1 100.00% []
🔵 com/starrocks/backup/BackupJobInfo.java 3 3 100.00% []
🔵 com/starrocks/lake/StarOSAgent.java 2 2 100.00% []
🔵 com/starrocks/sql/analyzer/ExpressionAnalyzer.java 1 1 100.00% []
🔵 com/starrocks/catalog/mv/MVTimelinessNonPartitionArbiter.java 1 1 100.00% []
🔵 com/starrocks/qe/ShowExecutor.java 1 1 100.00% []

@github-actions GitHub Actions
Copy link

[BE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gengjun-git gengjun-git gengjun-git left review comments

@meegoo meegoo meegoo left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

@HangyuanLiu HangyuanLiu

@gengjun-git gengjun-git

@nshangyiming nshangyiming

Labels
META-REVIEW
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@HangyuanLiu @meegoo @gengjun-git @wanpengfei-git @nshangyiming