[BugFix] LakeRollup should use physical partition ID instead of partition ID. #60073
Conversation
Signed-off-by: sevev <qiangzh95@gmail.com>
I will add some test case. |
|
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 5 / 5 (100.00%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
| @@ -98,8 +98,8 @@ public AlterJobV2 build() throws StarRocksException { | |||
| .collect(Collectors.toList()); | |||
| List<Long> shadowTabletIds = GlobalStateMgr.getCurrentState().getStarOSAgent().createShards( | |||
| originTablets.size(), | |||
| olapTable.getPartitionFilePathInfo(p 8000 artitionId), | |||
| olapTable.getPartitionFileCacheInfo(partitionId), | |||
| olapTable.getPartitionFilePathInfo(physicalPartitionId), | |||
There was a problem hiding this comment.
this may cause the new shadowTablets storage path differs from the original ones, consider before/after upgrading, will there be compatible issues?
There was a problem hiding this comment.
shadowTablets storage path differs from the original
Upgrading may indeed cause path switching, but the logic will not be problematic. This is because during actual operation, it relies on the shard information returned by starmgr to locate the tablet and the change will only affect the save path when creating the tablet.
| @@ -74,7 +74,7 @@ public AlterJobV2 build() throws StarRocksException { | |||
| long physicalPartitionId = physicalPartition.getId(); | |||
| // create shard group | |||
| long shardGroupId = GlobalStateMgr.getCurrentState().getStarOSAgent(). | |||
| createShardGroup(dbId, olapTable.getId(), partitionId, rollupIndexId); | |||
| createShardGroup(dbId, olapTable.getId(), physicalPartitionId, rollupIndexId); | |||
There was a problem hiding this comment.
also with this change, we need to keep in mind that in future if anyone want to use the "partitionId" attached to the shardGroup property, it can be partitionId or physicalPartitionId based on when the shardGroup is created. May need to record this info somewhere.
Why I'm doing:
In shared-data mode, the allocation of
shardGroupandshardshould correspond to the physical partition ID, rather than the partition ID.What I'm doing:
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: