You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2022/06/22 07:28:10 UTC
[doris] branch master updated: [fix] Fix duplicate code for PropertyAnalyzer.analyzeDataProperty() (#10190)
This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 2a0ac05ce7 [fix] Fix duplicate code for PropertyAnalyzer.analyzeDataProperty() (#10190)
2a0ac05ce7 is described below
commit 2a0ac05ce7e23bc0788ee5fcbf98b7f11ed62de4
Author: pengxiangyu <di...@163.com>
AuthorDate: Wed Jun 22 15:28:04 2022 +0800
[fix] Fix duplicate code for PropertyAnalyzer.analyzeDataProperty() (#10190)
PropertyAnalyzer.analyzeDataProperty(Map<String, String> properties, final DataProperty oldDataProperty) has something not suitable.
Parameter oldDataProperty is the old DataProperty, properties should be used to replace some of its members.
If properties has no some members, old ones need to be left, but not be set to default value.
Function modifyPartitionsProperty() uses analyzeDataProperty(), but create a new DataProperty again, it is duplicate.
---
.../main/java/org/apache/doris/alter/Alter.java | 19 ++------
.../apache/doris/common/util/PropertyAnalyzer.java | 55 +++++++++-------------
2 files changed, 27 insertions(+), 47 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
index e4da3cb4e3..bc4159cd8a 100644
--- 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
@@ -24,7 +24,6 @@ import org.apache.doris.analysis.AlterTableStmt;
import org.apache.doris.analysis.AlterViewStmt;
import org.apache.doris.analysis.ColumnRenameClause;
import org.apache.doris.analysis.CreateMaterializedViewStmt;
-import org.apache.doris.analysis.DateLiteral;
import org.apache.doris.analysis.DropMaterializedViewStmt;
import org.apache.doris.analysis.DropPartitionClause;
import org.apache.doris.analysis.ModifyColumnCommentClause;
@@ -52,7 +51,6 @@ import org.apache.doris.catalog.PartitionInfo;
import org.apache.doris.catalog.ReplicaAllocation;
import org.apache.doris.catalog.Table;
import org.apache.doris.catalog.TableIf.TableType;
-import org.apache.doris.catalog.Type;
import org.apache.doris.catalog.View;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.DdlException;
@@ -61,7 +59,6 @@ import org.apache.doris.common.UserException;
import org.apache.doris.common.util.DynamicPartitionUtil;
import org.apache.doris.common.util.MetaLockUtils;
import org.apache.doris.common.util.PropertyAnalyzer;
-import org.apache.doris.common.util.TimeUtils;
import org.apache.doris.persist.AlterViewInfo;
import org.apache.doris.persist.BatchModifyPartitionsInfo;
import org.apache.doris.persist.ModifyCommentOperationLog;
@@ -81,7 +78,6 @@ import org.apache.logging.log4j.Logger;
import java.util.Arrays;
import java.util.Comparator;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -684,17 +680,10 @@ public class Alter {
// 4. data property
// 4.1 get old data property from partition
DataProperty dataProperty = partitionInfo.getDataProperty(partition.getId());
- // 4.2 combine the old properties with new ones
- Map<String, String> newProperties = new HashMap<>();
- newProperties.put(PropertyAnalyzer.PROPERTIES_STORAGE_MEDIUM, dataProperty.getStorageMedium().name());
- DateLiteral dateLiteral = new DateLiteral(dataProperty.getCooldownTimeMs(),
- TimeUtils.getTimeZone(), Type.DATETIME);
- newProperties.put(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TIME, dateLiteral.getStringValue());
- newProperties.put(PropertyAnalyzer.PROPERTIES_REMOTE_STORAGE_POLICY, dataProperty.getRemoteStoragePolicy());
- newProperties.putAll(properties);
- // 4.3 analyze new properties
- DataProperty newDataProperty =
- PropertyAnalyzer.analyzeDataProperty(newProperties, DataProperty.DEFAULT_DATA_PROPERTY);
+ Map<String, String> modifiedProperties = Maps.newHashMap();
+ modifiedProperties.putAll(properties);
+ // 4.2 analyze new properties
+ DataProperty newDataProperty = PropertyAnalyzer.analyzeDataProperty(modifiedProperties, dataProperty);
// 1. date property
if (newDataProperty != null) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
index 84b29b9803..144576af6d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
@@ -44,6 +44,7 @@ import org.apache.doris.thrift.TTabletType;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Sets;
+import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -111,24 +112,28 @@ public class PropertyAnalyzer {
private static final double MAX_FPP = 0.05;
private static final double MIN_FPP = 0.0001;
- public static DataProperty analyzeDataProperty(Map<String, String> properties, DataProperty oldDataProperty)
+ /**
+ * check and replace members of DataProperty by properties.
+ *
+ * @param properties key->value for members to change.
+ * @param oldDataProperty old DataProperty
+ * @return new DataProperty
+ * @throws AnalysisException property has invalid key->value
+ */
+ public static DataProperty analyzeDataProperty(Map<String, String> properties, final DataProperty oldDataProperty)
throws AnalysisException {
if (properties == null || properties.isEmpty()) {
return oldDataProperty;
}
- TStorageMedium storageMedium = null;
- long cooldownTimeStamp = DataProperty.MAX_COOLDOWN_TIME_MS;
- String remoteStoragePolicy = "";
+ TStorageMedium storageMedium = oldDataProperty.getStorageMedium();
+ long cooldownTimeStamp = oldDataProperty.getCooldownTimeMs();
+ String remoteStoragePolicy = oldDataProperty.getRemoteStoragePolicy();
- boolean hasMedium = false;
- boolean hasCooldown = false;
- boolean hasRemoteStoragePolicy = false;
for (Map.Entry<String, String> entry : properties.entrySet()) {
String key = entry.getKey();
String value = entry.getValue();
- if (!hasMedium && key.equalsIgnoreCase(PROPERTIES_STORAGE_MEDIUM)) {
- hasMedium = true;
+ if (key.equalsIgnoreCase(PROPERTIES_STORAGE_MEDIUM)) {
if (value.equalsIgnoreCase(TStorageMedium.SSD.name())) {
storageMedium = TStorageMedium.SSD;
} else if (value.equalsIgnoreCase(TStorageMedium.HDD.name())) {
@@ -136,40 +141,26 @@ public class PropertyAnalyzer {
} else {
throw new AnalysisException("Invalid storage medium: " + value);
}
- } else if (!hasCooldown && key.equalsIgnoreCase(PROPERTIES_STORAGE_COOLDOWN_TIME)) {
+ } else if (key.equalsIgnoreCase(PROPERTIES_STORAGE_COOLDOWN_TIME)) {
DateLiteral dateLiteral = new DateLiteral(value, Type.DATETIME);
cooldownTimeStamp = dateLiteral.unixTimestamp(TimeUtils.getTimeZone());
- if (cooldownTimeStamp != DataProperty.MAX_COOLDOWN_TIME_MS) {
- hasCooldown = true;
- }
- } else if (!hasRemoteStoragePolicy && key.equalsIgnoreCase(PROPERTIES_REMOTE_STORAGE_POLICY)) {
- if (!Strings.isNullOrEmpty(value)) {
- hasRemoteStoragePolicy = true;
- remoteStoragePolicy = value;
- }
+ } else if (key.equalsIgnoreCase(PROPERTIES_REMOTE_STORAGE_POLICY)) {
+ remoteStoragePolicy = value;
}
} // end for properties
- // Check properties
-
- if (!hasCooldown && !hasMedium && !hasRemoteStoragePolicy) {
- return oldDataProperty;
- }
-
properties.remove(PROPERTIES_STORAGE_MEDIUM);
properties.remove(PROPERTIES_STORAGE_COOLDOWN_TIME);
properties.remove(PROPERTIES_REMOTE_STORAGE_POLICY);
- if (hasCooldown && !hasMedium) {
- throw new AnalysisException("Invalid data property. storage medium property is not found");
- }
-
- if (storageMedium == TStorageMedium.HDD && hasCooldown) {
+ if (storageMedium == TStorageMedium.HDD) {
cooldownTimeStamp = DataProperty.MAX_COOLDOWN_TIME_MS;
LOG.info("Can not assign cool down timestamp to HDD storage medium, ignore user setting.");
- hasCooldown = false;
}
+ boolean hasCooldown = cooldownTimeStamp != DataProperty.MAX_COOLDOWN_TIME_MS;
+ boolean hasRemoteStoragePolicy = StringUtils.isNotEmpty(remoteStoragePolicy);
+
long currentTimeMs = System.currentTimeMillis();
if (storageMedium == TStorageMedium.SSD && hasCooldown) {
if (cooldownTimeStamp <= currentTimeMs) {
@@ -183,7 +174,7 @@ public class PropertyAnalyzer {
}
if (hasRemoteStoragePolicy) {
- // check remote resource
+ // check remote storage policy
StoragePolicy checkedPolicy = new StoragePolicy(PolicyTypeEnum.STORAGE, remoteStoragePolicy);
Policy policy = Catalog.getCurrentCatalog().getPolicyMgr().getPolicy(checkedPolicy);
if (!(policy instanceof StoragePolicy)) {
@@ -195,7 +186,7 @@ public class PropertyAnalyzer {
if (storagePolicy.getCooldownDatetime().getTime() <= currentTimeMs) {
throw new AnalysisException("Remote storage cool down time should later than now");
}
- if (hasCooldown && (storagePolicy.getCooldownDatetime().getTime() <= cooldownTimeStamp)) {
+ if (hasCooldown && storagePolicy.getCooldownDatetime().getTime() <= cooldownTimeStamp) {
throw new AnalysisException("`remote_storage_cooldown_time`"
+ " should later than `storage_cooldown_time`.");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org