github-actions[bot] commented on code in PR #63696:
URL: https://github.com/apache/doris/pull/63696#discussion_r3308392391
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/PartitionInfo.java:
##########
@@ -427,15 +427,33 @@ public void resetPartitionIdForRestore(
idToStoragePolicy = Maps.newHashMap();
for (Map.Entry<Long, Long> entry : partitionIdMap.entrySet()) {
- idToDataProperty.put(entry.getKey(),
origIdToDataProperty.get(entry.getValue()));
- idToReplicaAllocation.put(entry.getKey(),
- restoreReplicaAlloc == null ?
origIdToReplicaAllocation.get(entry.getValue())
+ long newPartId = entry.getKey();
+ long origPartId = entry.getValue();
+
+ if (Config.isCloudMode()) {
+ // In cloud mode, storage_medium, cooldown_time, and
storage_policy are not applicable.
Review Comment:
This file now references `Config.isCloudMode()` but does not import
`org.apache.doris.common.Config`, so the FE build will fail with `cannot find
symbol: variable Config` in `PartitionInfo`. Please add the import or avoid the
direct dependency.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/TableProperty.java:
##########
@@ -194,6 +195,12 @@ public TableProperty buildProperty(short opCode) {
*/
public TableProperty resetPropertiesForRestore(boolean
reserveDynamicPartitionEnable, boolean reserveReplica,
ReplicaAllocation
replicaAlloc) {
+ if (Config.isCloudMode()) {
+ // In cloud mode, rewrite all properties that are not supported or
need to be forced.
+ // This handles: replication_num, replication_allocation,
dynamic_partition.replication_num,
+ // dynamic_partition.replication_allocation, storage_policy,
storage_medium, in_memory, etc.
+ PropertyAnalyzer.getInstance().rewriteForceProperties(properties);
+ }
Review Comment:
The cloud rewrite runs before this replica reset, so
`setReplicaAlloc(replicaAlloc)` can reintroduce an unsupported restored
`default.replication_allocation` immediately after
`CloudPropertyAnalyzer.rewriteForceProperties()` removed it. In a cloud restore
with `reserveReplica=false`, `OlapTable.resetPropertiesForRestore()` passes the
cloud/default restore allocation here, this method writes it back into
`properties`, and no second cloud rewrite runs before the table property is
persisted. That leaves the same kind of source-cluster replica setting this PR
is trying to strip. Please perform the cloud rewrite after all restore-specific
mutations, or make `setReplicaAlloc` cloud-aware for restore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
