database/doris
2
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

[fix] Fix duplicate code for PropertyAnalyzer.analyzeDataProperty() (#10190)

Browse Source 
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.
This commit is contained in:
pengxiangyu
2022-06-22 15:28:04 +08:00
committed by GitHub
parent 98b3306e05
commit 2a0ac05ce7
2 changed files with 27 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
View File

@ -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) {

55
fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java
View File

@ -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`.");
}