fix fe_ut

apache · Sep 11, 2024 · f803759 · f803759
1 parent fa41c5b
commit f803759
Show file tree

Hide file tree

Showing 9 changed files with 46 additions and 62 deletions.
diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
@@ -836,37 +836,39 @@ public void modifyPartitionsProperty(Database db,
 
             // 4.3 modify partition storage policy
             // can set multi times storage policy
-            String currentStoragePolicy = PropertyAnalyzer.analyzeStoragePolicy(properties);
-            if (!currentStoragePolicy.equals("")) {
-                // check currentStoragePolicy resource exist.
-                Env.getCurrentEnv().getPolicyMgr().checkStoragePolicyExist(currentStoragePolicy);
-                partitionInfo.setStoragePolicy(partition.getId(), currentStoragePolicy);
-            } else {
-                // if current partition is already in remote storage
-                if (partition.getRemoteDataSize() > 0) {
-                    throw new AnalysisException(
-                        "Cannot cancel storage policy for partition which is already on cold storage.");
-                }
-
-                // if current partition will be cooldown in 20s later
-                StoragePolicy checkedPolicyCondition = StoragePolicy.ofCheck(dataProperty.getStoragePolicy());
-                StoragePolicy policy = (StoragePolicy) Env.getCurrentEnv().getPolicyMgr()
-                        .getPolicy(checkedPolicyCondition);
-                if (policy != null) {
-                    long latestTime = policy.getCooldownTimestampMs() > 0 ? policy.getCooldownTimestampMs()
-                            : Long.MAX_VALUE;
-                    if (policy.getCooldownTtl() > 0) {
-                        latestTime = Math.min(latestTime,
-                            partition.getVisibleVersionTime() + policy.getCooldownTtl() * 1000);
-                    }
-                    if (latestTime < System.currentTimeMillis() + 20 * 1000) {
+            if (properties.containsKey(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY)) {
+                String currentStoragePolicy = PropertyAnalyzer.analyzeStoragePolicy(properties);
+                if (!currentStoragePolicy.equals("")) {
+                    // check currentStoragePolicy resource exist.
+                    Env.getCurrentEnv().getPolicyMgr().checkStoragePolicyExist(currentStoragePolicy);
+                    dataProperty.setStoragePolicy(currentStoragePolicy);
+                } else {
+                    // if current partition is already in remote storage
+                    if (partition.getRemoteDataSize() > 0) {
                         throw new AnalysisException(
-                            "Cannot cancel storage policy for partition which already be cooldown"
-                                + " or will be cooldown soon later");
+                                "Cannot cancel storage policy for partition which is already on cold storage.");
                     }
-                }
 
-                partitionInfo.setStoragePolicy(partition.getId(), "");
+                    // if current partition will be cooldown in 20s later
+                    StoragePolicy checkedPolicyCondition = StoragePolicy.ofCheck(dataProperty.getStoragePolicy());
+                    StoragePolicy policy = (StoragePolicy) Env.getCurrentEnv().getPolicyMgr()
+                            .getPolicy(checkedPolicyCondition);
+                    if (policy != null) {
+                        long latestTime = policy.getCooldownTimestampMs() > 0 ? policy.getCooldownTimestampMs()
+                                : Long.MAX_VALUE;
+                        if (policy.getCooldownTtl() > 0) {
+                            latestTime = Math.min(latestTime,
+                                    partition.getVisibleVersionTime() + policy.getCooldownTtl() * 1000);
+                        }
+                        if (latestTime < System.currentTimeMillis() + 20 * 1000) {
+                            throw new AnalysisException(
+                                    "Cannot cancel storage policy for partition which already be cooldown"
+                                            + " or will be cooldown soon later");
+                        }
+                    }
+
+                    dataProperty.setStoragePolicy("");
+                }
             }
 
             // 4.4 analyze new properties
@@ -890,8 +892,7 @@ public void modifyPartitionsProperty(Database db,
                 partitionInfo.setTabletType(partition.getId(), tTabletType);
             }
             ModifyPartitionInfo info = new ModifyPartitionInfo(db.getId(), olapTable.getId(), partition.getId(),
-                    newDataProperty, replicaAlloc, hasInMemory ? newInMemory : oldInMemory, currentStoragePolicy,
-                    Maps.newHashMap());
+                    newDataProperty, replicaAlloc, hasInMemory ? newInMemory : oldInMemory, Maps.newHashMap());
             modifyPartitionInfos.add(info);
         }
 

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SinglePartitionDesc.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SinglePartitionDesc.java
@@ -47,7 +47,6 @@ public class SinglePartitionDesc implements AllPartitionDesc {
     private boolean isInMemory = false;
     private TTabletType tabletType = TTabletType.TABLET_TYPE_DISK;
     private Long versionInfo;
-    private String storagePolicy;
     private boolean isMutable;
 
     public SinglePartitionDesc(boolean ifNotExists, String partName, PartitionKeyDesc partitionKeyDesc,
@@ -62,7 +61,6 @@ public SinglePartitionDesc(boolean ifNotExists, String partName, PartitionKeyDes
 
         this.partitionDataProperty = new DataProperty(DataProperty.DEFAULT_STORAGE_MEDIUM);
         this.replicaAlloc = ReplicaAllocation.DEFAULT_ALLOCATION;
-        this.storagePolicy = "";
     }
 
     /**
@@ -71,7 +69,7 @@ public SinglePartitionDesc(boolean ifNotExists, String partName, PartitionKeyDes
     public SinglePartitionDesc(boolean ifNotExists, String partName,
             PartitionKeyDesc partitionKeyDesc, ReplicaAllocation replicaAlloc,
             Map<String, String> properties, DataProperty partitionDataProperty, boolean isInMemory,
-            TTabletType tabletType, Long versionInfo, String storagePolicy, boolean isMutable) {
+            TTabletType tabletType, Long versionInfo, boolean isMutable) {
         this.isAnalyzed = true;
         this.ifNotExists = ifNotExists;
         this.partName = partName;
@@ -82,7 +80,6 @@ public SinglePartitionDesc(boolean ifNotExists, String partName,
         this.isInMemory = isInMemory;
         this.tabletType = tabletType;
         this.versionInfo = versionInfo;
-        this.storagePolicy = storagePolicy;
         this.isMutable = isMutable;
     }
 
@@ -142,7 +139,7 @@ public void analyze(int partColNum, Map<String, String> otherProperties) throws
                         boolean equal = iter.compareToIgnoreCase(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY) == 0;
                         // when find has storage policy properties, here will set it in partition
                         if (equal) {
-                            storagePolicy = properties.get(iter);
+                            partitionDataProperty.setStoragePolicy(properties.get(iter));
                         }
                         return equal;
                     });
@@ -210,7 +207,7 @@ public void setAnalyzed(boolean analyzed) {
     }
 
     public String getStoragePolicy() {
-        return this.storagePolicy;
+        return this.partitionDataProperty.getStoragePolicy();
     }
 
     public String toSql() {

diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java
@@ -4265,6 +4265,7 @@ public HashMap<Long, TStorageMedium> getPartitionIdToStorageMediumMap() {
                                 && dataProperty.getCooldownTimeMs() < currentTimeMs) {
                             // expire. change to HDD.
                             DataProperty hddProperty = new DataProperty(TStorageMedium.HDD);
+                            hddProperty.setStoragePolicy(partitionInfo.getStoragePolicy(partitionId));
                             partitionInfo.setDataProperty(partition.getId(), hddProperty);
                             storageMediumMap.put(partitionId, TStorageMedium.HDD);
                             LOG.info("partition[{}-{}-{}] storage medium changed from SSD to HDD. "
@@ -4275,8 +4276,7 @@ public HashMap<Long, TStorageMedium> getPartitionIdToStorageMediumMap() {
                             // log
                             ModifyPartitionInfo info = new ModifyPartitionInfo(db.getId(), olapTable.getId(),
                                     partition.getId(), hddProperty, ReplicaAllocation.NOT_SET,
-                                    partitionInfo.getIsInMemory(partition.getId()),
-                                    partitionInfo.getStoragePolicy(partitionId), Maps.newHashMap());
+                                    partitionInfo.getIsInMemory(partition.getId()), Maps.newHashMap());
 
                             editLog.logModifyPartition(info);
                         }
@@ -5237,8 +5237,7 @@ public void modifyTableReplicaAllocation(Database db, OlapTable table, Map<Strin
 
         // log
         ModifyPartitionInfo info = new ModifyPartitionInfo(db.getId(), table.getId(), partition.getId(),
-                newDataProperty, replicaAlloc, isInMemory, partitionInfo.getStoragePolicy(partition.getId()),
-                tblProperties);
+                newDataProperty, replicaAlloc, isInMemory, tblProperties);
         editLog.logModifyPartition(info);
         if (LOG.isDebugEnabled()) {
             LOG.debug("modify partition[{}-{}-{}] replica allocation to {}",
@@ -5352,9 +5351,6 @@ public void replayModifyTableProperty(short opCode, ModifyTablePropertyOperation
                         // storage policy re-use modify in memory
                         Optional.ofNullable(tableProperty.getStoragePolicy()).filter(p -> !p.isEmpty())
                                 .ifPresent(p -> olapTable.getPartitionInfo().setStoragePolicy(partition.getId(), p));
-                        Optional.ofNullable(tableProperty.getStoragePolicy()).filter(p -> !p.isEmpty())
-                                .ifPresent(p -> olapTable.getPartitionInfo().getDataProperty(partition.getId())
-                                .setStoragePolicy(p));
                     }
                     break;
                 case OperationType.OP_UPDATE_BINLOG_CONFIG:

diff --git a/...src/main/java/org/apache/doris/nereids/trees/plans/commands/info/FixedRangePartition.java b/...src/main/java/org/apache/doris/nereids/trees/plans/commands/info/FixedRangePartition.java
@@ -79,7 +79,6 @@ public SinglePartitionDesc translateToCatalogStyle() {
                 .collect(Collectors.toList());
         return new SinglePartitionDesc(ifNotExists, partitionName,
                 PartitionKeyDesc.createFixed(lowerValues, upperValues), replicaAllocation,
-                properties, partitionDataProperty, isInMemory, tabletType, versionInfo,
-                storagePolicy, isMutable);
+                properties, partitionDataProperty, isInMemory, tabletType, versionInfo, isMutable);
     }
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/InPartition.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/InPartition.java
@@ -70,7 +70,6 @@ public AllPartitionDesc translateToCatalogStyle() {
                 .collect(Collectors.toList())).collect(Collectors.toList());
         return new SinglePartitionDesc(ifNotExists, partitionName,
                 PartitionKeyDesc.createIn(catalogValues), replicaAllocation, properties,
-                partitionDataProperty, isInMemory, tabletType, versionInfo, storagePolicy,
-                isMutable);
+                partitionDataProperty, isInMemory, tabletType, versionInfo, isMutable);
     }
 }
diff --git a/...e/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/LessThanPartition.java b/...e/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/LessThanPartition.java
@@ -60,14 +60,12 @@ public SinglePartitionDesc translateToCatalogStyle() {
         if (values.get(0) instanceof MaxValue) {
             return new SinglePartitionDesc(ifNotExists, partitionName,
                     PartitionKeyDesc.createMaxKeyDesc(), replicaAllocation, properties,
-                    partitionDataProperty, isInMemory, tabletType, versionInfo, storagePolicy,
-                    isMutable);
+                    partitionDataProperty, isInMemory, tabletType, versionInfo, isMutable);
         }
         List<PartitionValue> partitionValues =
                 values.stream().map(this::toLegacyPartitionValueStmt).collect(Collectors.toList());
         return new SinglePartitionDesc(ifNotExists, partitionName,
                 PartitionKeyDesc.createLessThan(partitionValues), replicaAllocation, properties,
-                partitionDataProperty, isInMemory, tabletType, versionInfo, storagePolicy,
-                isMutable);
+                partitionDataProperty, isInMemory, tabletType, versionInfo, isMutable);
     }
 }
diff --git a/...src/main/java/org/apache/doris/nereids/trees/plans/commands/info/PartitionDefinition.java b/...src/main/java/org/apache/doris/nereids/trees/plans/commands/info/PartitionDefinition.java
@@ -53,7 +53,6 @@ public abstract class PartitionDefinition {
     protected boolean isInMemory = false;
     protected TTabletType tabletType = TTabletType.TABLET_TYPE_DISK;
     protected Long versionInfo;
-    protected String storagePolicy = "";
     protected boolean isMutable;
 
     public PartitionDefinition(boolean ifNotExists, String partName) {
@@ -89,7 +88,7 @@ public void validate(Map<String, String> otherProperties) {
                             .compareToIgnoreCase(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY) == 0;
                     // when find has storage policy properties, here will set it in partition
                     if (equal) {
-                        storagePolicy = properties.get(iter);
+                        partitionDataProperty.setStoragePolicy(properties.get(iter));
                     }
                     return equal;
                 });

diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/ModifyPartitionInfo.java b/fe/fe-core/src/main/java/org/apache/doris/persist/ModifyPartitionInfo.java
@@ -50,30 +50,25 @@ public class ModifyPartitionInfo implements Writable {
 
     @SerializedName(value = "replicaAlloc")
     private ReplicaAllocation replicaAlloc;
-
-    @SerializedName(value = "storagePolicy")
-    private String storagePolicy;
     @SerializedName(value = "tableProperties")
     private Map<String, String> tblProperties;
 
     public String getStoragePolicy() {
-        return storagePolicy;
+        return dataProperty.getStoragePolicy();
     }
 
     public ModifyPartitionInfo() {
         // for persist
     }
 
     public ModifyPartitionInfo(long dbId, long tableId, long partitionId, DataProperty dataProperty,
-            ReplicaAllocation replicaAlloc, boolean isInMemory, String storagePolicy,
-            Map<String, String> tblProperties) {
+            ReplicaAllocation replicaAlloc, boolean isInMemory, Map<String, String> tblProperties) {
         this.dbId = dbId;
         this.tableId = tableId;
         this.partitionId = partitionId;
         this.dataProperty = dataProperty;
         this.replicaAlloc = replicaAlloc;
         this.isInMemory = isInMemory;
-        this.storagePolicy = storagePolicy;
         this.tblProperties = tblProperties;
         if (this.tblProperties == null) {
             this.tblProperties = Maps.newHashMap();
@@ -134,7 +129,7 @@ public boolean equals(Object other) {
         ModifyPartitionInfo otherInfo = (ModifyPartitionInfo) other;
         return dbId == otherInfo.getDbId() && tableId == otherInfo.getTableId()
                 && dataProperty.equals(otherInfo.getDataProperty()) && replicaAlloc.equals(otherInfo.replicaAlloc)
-                && isInMemory == otherInfo.isInMemory() && storagePolicy.equals(otherInfo.storagePolicy);
+                && isInMemory == otherInfo.isInMemory();
     }
 
     @Override

diff --git a/fe/fe-core/src/test/java/org/apache/doris/persist/BatchModifyPartitionsInfoTest.java b/fe/fe-core/src/test/java/org/apache/doris/persist/BatchModifyPartitionsInfoTest.java
@@ -62,7 +62,7 @@ public void testSerializeBatchModifyPartitionsInfo() throws IOException, Analysi
         for (long partitionId : partitionIds) {
             modifyInfos.add(new ModifyPartitionInfo(DB_ID, TB_ID, partitionId,
                     new DataProperty(DataProperty.DEFAULT_STORAGE_MEDIUM), ReplicaAllocation.DEFAULT_ALLOCATION,
-                    true, "", Maps.newHashMap()));
+                    true, Maps.newHashMap()));
         }
 
         BatchModifyPartitionsInfo batchModifyPartitionsInfo = new BatchModifyPartitionsInfo(modifyInfos);