Skip to content

Commit

Permalink
Fix a batch of bugfix for storage_cooldown_ttl (#30040)
Browse files Browse the repository at this point in the history 
Signed-off-by: Astralidea <astralidea@163.com>
  • Loading branch information
@Astralidea
Astralidea authored Aug 28, 2023
1 parent 877d0bd commit 820882e
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 18 deletions.
9 changes: 8 additions & 1 deletion fe/fe-core/src/main/java/com/starrocks/alter/AlterJobMgr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@
import com.starrocks.thrift.TTabletType;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.util.Strings;
import org.threeten.extra.PeriodDuration;

import java.io.IOException;
Expand Down Expand Up @@ -1105,7 +1106,9 @@ public void modifyPartitionsProperty(Database db,
if (!partitionInfo.isRangePartition()) {
throw new DdlException("Only support range partition table to modify storage_cooldown_ttl");
}
periodDuration = TimeUtils.parseHumanReadablePeriodOrDuration(storageCoolDownTTL);
if (Strings.isNotBlank(storageCoolDownTTL)) {
periodDuration = TimeUtils.parseHumanReadablePeriodOrDuration(storageCoolDownTTL);
}
}
DataProperty newDataProperty =
PropertyAnalyzer.analyzeDataProperty(properties, null, false);
Expand All @@ -1124,6 +1127,10 @@ public void modifyPartitionsProperty(Database db,
Partition partition = olapTable.getPartition(partitionName);
// 1. date property

if (partitionName.startsWith(ExpressionRangePartitionInfo.SHADOW_PARTITION_PREFIX)) {
continue;
}

if (newDataProperty != null) {
// for storage_cooldown_ttl
if (periodDuration != null) {
Expand Down
9 changes: 7 additions & 2 deletions fe/fe-core/src/main/java/com/starrocks/catalog/TableProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.gson.annotations.SerializedName;
Expand Down Expand Up @@ -465,8 +466,12 @@ public TableProperty buildConstraint() {

public TableProperty buildStorageCoolDownTTL() {
if (properties.containsKey(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TTL)) {
storageCoolDownTTL = TimeUtils.parseHumanReadablePeriodOrDuration(
properties.get(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TTL));
String storageCoolDownTTL = properties.get(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TTL);
if (Strings.isNullOrEmpty(storageCoolDownTTL)) {
this.storageCoolDownTTL = null;
} else {
this.storageCoolDownTTL = TimeUtils.parseHumanReadablePeriodOrDuration(storageCoolDownTTL);
}
}
return this;
}
Expand Down
7 changes: 5 additions & 2 deletions fe/fe-core/src/main/java/com/starrocks/common/util/PropertyAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -962,12 +962,15 @@ public static List<ForeignKeyConstraint> analyzeForeignKeyConstraint(
return foreignKeyConstraints;
}

public static PeriodDuration analyzeStorageCoolDownTTL(Map<String, String> properties) throws AnalysisException {
public static PeriodDuration analyzeStorageCoolDownTTL(Map<String, String> properties,
boolean removeProperties) throws AnalysisException {
String text = properties.get(PROPERTIES_STORAGE_COOLDOWN_TTL);
if (text == null) {
return null;
}
properties.remove(PROPERTIES_STORAGE_COOLDOWN_TTL);
if (removeProperties) {
properties.remove(PROPERTIES_STORAGE_COOLDOWN_TTL);
}
PeriodDuration periodDuration;
try {
periodDuration = TimeUtils.parseHumanReadablePeriodOrDuration(text);
Expand Down
36 changes: 35 additions & 1 deletion fe/fe-core/src/main/java/com/starrocks/server/LocalMetastore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@
import org.apache.hadoop.util.ThreadUtil;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.threeten.extra.PeriodDuration;

import java.io.DataInputStream;
import java.io.DataOutputStream;
Expand Down Expand Up @@ -2328,6 +2329,39 @@ private void createOlapOrLakeTable(Database db, CreateTableStmt stmt) throws Ddl
throw new DdlException(e.getMessage());
}

if (properties != null) {
if (properties.containsKey(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TTL) ||
properties.containsKey(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TIME)) {
if (table.getKeysType() == KeysType.PRIMARY_KEYS) {
throw new DdlException("Primary key table does not support storage medium cool down currently.");
}
if (partitionInfo instanceof ListPartitionInfo) {
throw new DdlException("List partition table does not support storage medium cool down currently.");
}
if (partitionInfo instanceof RangePartitionInfo) {
RangePartitionInfo rangePartitionInfo = (RangePartitionInfo) partitionInfo;
List<Column> partitionColumns = rangePartitionInfo.getPartitionColumns();
if (partitionColumns.size() > 1) {
throw new DdlException("Multi-column range partition table " +
"does not support storage medium cool down currently.");
}
Column column = partitionColumns.get(0);
if (!column.getType().getPrimitiveType().isDateType()) {
throw new DdlException("Only support partition is date type for" +
" storage medium cool down currently.");
}
}
}
try {
PeriodDuration duration = PropertyAnalyzer.analyzeStorageCoolDownTTL(properties, false);
if (duration != null) {
table.setStorageCoolDownTTL(duration);
}
} catch (AnalysisException e) {
throw new DdlException(e.getMessage());
}
}

if (partitionInfo.getType() == PartitionType.UNPARTITIONED) {
// if this is an unpartitioned table, we should analyze data property and replication num here.
// if this is a partitioned table, there properties are already analyzed in RangePartitionDesc analyze phase.
Expand Down Expand Up @@ -4180,7 +4214,7 @@ public void alterTableProperties(Database db, OlapTable table, Map<String, Strin
}
if (properties.containsKey(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TTL)) {
try {
PropertyAnalyzer.analyzeStorageCoolDownTTL(properties);
PropertyAnalyzer.analyzeStorageCoolDownTTL(properties, true);
} catch (AnalysisException ex) {
throw new RuntimeException(ex.getMessage());
}
Expand Down
2 changes: 1 addition & 1 deletion fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AlterTableStatementAnalyzer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public Void visitModifyTablePropertiesClause(ModifyTablePropertiesClause clause,
} else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TTL)) {
String storageCoolDownTTL = properties.get(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TTL);
try {
PropertyAnalyzer.analyzeStorageCoolDownTTL(properties);
PropertyAnalyzer.analyzeStorageCoolDownTTL(properties, true);
} catch (AnalysisException e) {
ErrorReport.reportSemanticException(ErrorCode.ERR_COMMON_ERROR, e.getMessage());
}
Expand Down
25 changes: 14 additions & 11 deletions fe/fe-core/src/main/java/com/starrocks/sql/ast/SingleRangePartitionDesc.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.starrocks.sql.analyzer.FeNameFormat;
import com.starrocks.thrift.TStorageMedium;
import com.starrocks.thrift.TTabletType;
import org.apache.logging.log4j.util.Strings;
import org.threeten.extra.PeriodDuration;

import java.time.LocalDateTime;
Expand Down Expand Up @@ -138,17 +139,19 @@ public void analyze(int partColNum, Map<String, String> otherProperties) throws
if (partColNum == 1 && properties != null
&& properties.containsKey(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TTL)) {
String storageCoolDownTTL = properties.get(PropertyAnalyzer.PROPERTIES_STORAGE_COOLDOWN_TTL);
PeriodDuration periodDuration = TimeUtils.parseHumanReadablePeriodOrDuration(storageCoolDownTTL);
if (partitionKeyDesc.isMax()) {
partitionDataProperty = new DataProperty(TStorageMedium.SSD, DataProperty.MAX_COOLDOWN_TIME_MS);
} else {
String stringUpperValue = partitionKeyDesc.getUpperValues().get(0).getStringValue();
DateTimeFormatter dateTimeFormatter = DateUtils.probeFormat(stringUpperValue);
LocalDateTime upperTime = DateUtils.parseStringWithDefaultHSM(stringUpperValue, dateTimeFormatter);
LocalDateTime updatedUpperTime = upperTime.plus(periodDuration);
DateLiteral dateLiteral = new DateLiteral(updatedUpperTime, Type.DATETIME);
long coolDownTimeStamp = dateLiteral.unixTimestamp(TimeUtils.getTimeZone());
partitionDataProperty = new DataProperty(TStorageMedium.SSD, coolDownTimeStamp);
if (Strings.isNotBlank(storageCoolDownTTL)) {
PeriodDuration periodDuration = TimeUtils.parseHumanReadablePeriodOrDuration(storageCoolDownTTL);
if (partitionKeyDesc.isMax()) {
partitionDataProperty = new DataProperty(TStorageMedium.SSD, DataProperty.MAX_COOLDOWN_TIME_MS);
} else {
String stringUpperValue = partitionKeyDesc.getUpperValues().get(0).getStringValue();
DateTimeFormatter dateTimeFormatter = DateUtils.probeFormat(stringUpperValue);
LocalDateTime upperTime = DateUtils.parseStringWithDefaultHSM(stringUpperValue, dateTimeFormatter);
LocalDateTime updatedUpperTime = upperTime.plus(periodDuration);
DateLiteral dateLiteral = new DateLiteral(updatedUpperTime, Type.DATETIME);
long coolDownTimeStamp = dateLiteral.unixTimestamp(TimeUtils.getTimeZone());
partitionDataProperty = new DataProperty(TStorageMedium.SSD, coolDownTimeStamp);
}
}
} else {
// analyze data property
Expand Down
69 changes: 69 additions & 0 deletions fe/fe-core/src/test/java/com/starrocks/catalog/CreateTableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1324,4 +1324,73 @@ public void testCreateCrossDatabaseColocateTable() throws Exception {
// colocate groups in different db should have same `GroupId.grpId`
Assert.assertEquals(groupIds.get(0).split("\\.")[1], groupIds.get(1).split("\\.")[1]);
}

@Test
public void testPrimaryKeyNotSupportCoolDown() {
ExceptionChecker.expectThrowsWithMsg(DdlException.class,
"Primary key table does not support storage medium cool down currently.",
() -> createTable(
"CREATE TABLE test.`primary_table_not_support_cool_down`\n" +
" ( `k1` date, `k2` datetime,`k3` string, `k4` varchar(20), " +
"`k5` boolean, `k6` tinyint, `k7` smallint, `k8` int, `k9` bigint, " +
"`k10` largeint, `k11` float, `k12` double, `k13` decimal(27,9))\n" +
" primary KEY(`k1`, `k2`, `k3`, `k4`, `k5`)\n" +
" PARTITION BY range(k1)\n" +
" (\n" +
" PARTITION p1 VALUES LESS THAN (\"2021-01-02\"),\n" +
" PARTITION p2 VALUES LESS THAN (\"2021-08-18\"),\n" +
" PARTITION p3 VALUES LESS THAN (\"2022-08-17\"),\n" +
" PARTITION p4 VALUES LESS THAN (\"2022-08-18\"),\n" +
" PARTITION p5 VALUES LESS THAN (\"2022-08-19\"),\n" +
" PARTITION p6 VALUES LESS THAN (\"2023-08-18\"),\n" +
" PARTITION p7 VALUES LESS THAN (\"2024-08-18\")\n" +
" ) DISTRIBUTED BY HASH(`k1`, `k2`, `k3`)\n" +
" PROPERTIES (\"storage_medium\" = \"SSD\", \"storage_cooldown_ttl\" = \"0 year\");"
));
ExceptionChecker.expectThrowsWithMsg(DdlException.class,
"List partition table does not support storage medium cool down currently.",
() -> createTable(
"CREATE TABLE test.list_partition_table_not_support_cool_down (\n" +
" `k1` date not null, `k2` datetime,`k3` char(20), " +
"`k4` varchar(20), `k5` boolean, `k6` tinyint, `k7` smallint, `k8` int, " +
"`k9` bigint, `k10` largeint, `k11` float, `k12` double, `k13` decimal(27,9)\n" +
" )\n" +
" DUPLICATE KEY(k1)\n" +
" PARTITION BY LIST (k1) (\n" +
" PARTITION p1 VALUES IN (\"2020-01-01\",\"2020-01-02\"),\n" +
" PARTITION p2 VALUES IN (\"2021-01-01\")\n" +
" )\n" +
" DISTRIBUTED BY HASH(k1)\n" +
" PROPERTIES (\"storage_medium\" = \"SSD\", \"storage_cooldown_ttl\" = \"0 day\");"
));

ExceptionChecker.expectThrowsWithMsg(DdlException.class,
"Only support partition is date type for storage medium cool down currently.",
() -> createTable(
"CREATE TABLE test.t_partition_table_only_support_date (\n" +
" `k1` int, v1 int, v2 int\n" +
" )\n" +
" DUPLICATE KEY(k1)\n" +
" PARTITION BY range (k1) (\n" +
" PARTITION p1 VALUES less than (\"20200101\"),\n" +
" PARTITION p2 VALUES less than (\"20210101\")\n" +
" )\n" +
" DISTRIBUTED BY HASH(k1)\n" +
" PROPERTIES (\"storage_medium\" = \"SSD\", \"storage_cooldown_ttl\" = \"0 day\");"
));

ExceptionChecker.expectThrowsWithMsg(DdlException.class,
"Invalid data property. storage medium property is not found",
() -> createTable(
"CREATE TABLE `cooldown_ttl_month1_table_with_null`\n" +
" ( `k1` date, `k2` datetime,`k3` string, `k4` varchar(20), " +
"`k5` boolean, `k6` tinyint, `k7` smallint, `k8` int, `k9` bigint, " +
"`k10` largeint, `k11` float, `k12` double, `k13` decimal(27,9))\n" +
" unique KEY(`k1`, `k2`, `k3`, `k4`, `k5`)\n" +
" PARTITION BY time_slice(k2, interval 1 month)\n" +
" DISTRIBUTED BY HASH(`k1`, `k2`, `k3`)\n" +
" PROPERTIES (\"storage_cooldown_ttl\" = \"1 month\");"
));
}

}

0 comments on commit 820882e

Please sign in to comment.