From e8de07a6a5df6c1c0187fa346d1e9cf6a6b77752 Mon Sep 17 00:00:00 2001 From: Yusheng Xu <30896780+1330571@users.noreply.github.com> Date: Mon, 27 Feb 2023 18:42:31 +0800 Subject: [PATCH] [feature](cooldown) Forbid storage policy for MoW tables (#17148) * disable setting storage policy on MoW table * fix error in regression test * make the name of test table unique * use Strings.isNullOrEmpty to replace equals * fix error in if statement --- .../doris/alter/SchemaChangeHandler.java | 13 ++ .../analysis/ModifyTablePropertiesClause.java | 11 +- .../doris/datasource/InternalCatalog.java | 12 ++ .../disable_storage_policy_MoW.groovy | 134 ++++++++++++++++++ 4 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 regression-test/suites/cold_heat_separation/use_policy/disable_storage_policy_MoW.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java index 58dfddf1c6..3bfee997fd 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java +++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java @@ -1917,8 +1917,10 @@ public class SchemaChangeHandler extends AlterHandler { throws UserException { List partitions = Lists.newArrayList(); OlapTable olapTable = (OlapTable) db.getTableOrMetaException(tableName, Table.TableType.OLAP); + boolean enableUniqueKeyMergeOnWrite = false; olapTable.readLock(); try { + enableUniqueKeyMergeOnWrite = olapTable.getEnableUniqueKeyMergeOnWrite(); partitions.addAll(olapTable.getPartitions()); } finally { olapTable.readUnlock(); @@ -1934,6 +1936,11 @@ public class SchemaChangeHandler extends AlterHandler { } } String storagePolicy = properties.get(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY); + if (enableUniqueKeyMergeOnWrite && !Strings.isNullOrEmpty(storagePolicy)) { + throw new UserException( + "Can not set UNIQUE KEY table that enables Merge-On-write" + + " with storage policy(" + storagePolicy + ")"); + } long storagePolicyId = storagePolicyNameToId(storagePolicy); if (isInMemory < 0 && storagePolicyId < 0) { @@ -1969,6 +1976,12 @@ public class SchemaChangeHandler extends AlterHandler { } } String storagePolicy = properties.get(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY); + boolean enableUniqueKeyMergeOnWrite = olapTable.getEnableUniqueKeyMergeOnWrite(); + if (enableUniqueKeyMergeOnWrite && !Strings.isNullOrEmpty(storagePolicy)) { + throw new DdlException( + "Can not set UNIQUE KEY table that enables Merge-On-write" + + " with storage policy(" + storagePolicy + ")"); + } long storagePolicyId = storagePolicyNameToId(storagePolicy); if (isInMemory < 0 && storagePolicyId < 0) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java index 5b0951d37b..34aa641104 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java @@ -25,6 +25,8 @@ import org.apache.doris.common.util.DynamicPartitionUtil; import org.apache.doris.common.util.PrintableMap; import org.apache.doris.common.util.PropertyAnalyzer; +import com.google.common.base.Strings; + import java.util.Map; // clause which is used to modify table properties @@ -101,7 +103,14 @@ public class ModifyTablePropertiesClause extends AlterTableClause { throw new AnalysisException("Alter tablet type not supported"); } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY)) { this.needTableStable = false; - setStoragePolicy(properties.getOrDefault(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY, "")); + String storagePolicy = properties.getOrDefault(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY, ""); + if (!Strings.isNullOrEmpty(storagePolicy) + && properties.containsKey(PropertyAnalyzer.ENABLE_UNIQUE_KEY_MERGE_ON_WRITE)) { + throw new AnalysisException( + "Can not set UNIQUE KEY table that enables Merge-On-write" + + " with storage policy(" + storagePolicy + ")"); + } + setStoragePolicy(storagePolicy); } else if (properties.containsKey(PropertyAnalyzer.ENABLE_UNIQUE_KEY_MERGE_ON_WRITE)) { throw new AnalysisException("Can not change UNIQUE KEY to Merge-On-Write mode"); } else { diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index 0a5311a2b1..132cebbbdf 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -2013,6 +2013,12 @@ public class InternalCatalog implements CatalogIf { // set storage policy String storagePolicy = PropertyAnalyzer.analyzeStoragePolicy(properties); Env.getCurrentEnv().getPolicyMgr().checkStoragePolicyExist(storagePolicy); + if (olapTable.getEnableUniqueKeyMergeOnWrite() + && !Strings.isNullOrEmpty(storagePolicy)) { + throw new AnalysisException( + "Can not create UNIQUE KEY table that enables Merge-On-write" + + " with storage policy(" + storagePolicy + ")"); + } olapTable.setStoragePolicy(storagePolicy); TTabletType tabletType; @@ -2231,6 +2237,12 @@ public class InternalCatalog implements CatalogIf { DistributionInfo partitionDistributionInfo = distributionDesc.toDistributionInfo(baseSchema); // use partition storage policy if it exist. String partionStoragePolicy = partitionInfo.getStoragePolicy(entry.getValue()); + if (olapTable.getEnableUniqueKeyMergeOnWrite() + && !Strings.isNullOrEmpty(partionStoragePolicy)) { + throw new AnalysisException( + "Can not create UNIQUE KEY table that enables Merge-On-write" + + " with storage policy(" + partionStoragePolicy + ")"); + } if (!partionStoragePolicy.equals("")) { storagePolicy = partionStoragePolicy; } diff --git a/regression-test/suites/cold_heat_separation/use_policy/disable_storage_policy_MoW.groovy b/regression-test/suites/cold_heat_separation/use_policy/disable_storage_policy_MoW.groovy new file mode 100644 index 0000000000..4d18163f9b --- /dev/null +++ b/regression-test/suites/cold_heat_separation/use_policy/disable_storage_policy_MoW.groovy @@ -0,0 +1,134 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("disable_storage_policy_MoW"){ + def s3_source_name = "default_s3_source_test" + def storage_policy_name = "default_storage_policy_test" + + def storage_exist = { name -> + def show_storage_policy = sql """ + SHOW STORAGE POLICY; + """ + for(iter in show_storage_policy){ + if(name == iter[0]){ + return true; + } + } + return false; + } + + if(!storage_exist.call("${storage_policy_name}")){ + def create_s3_resource = sql """ + CREATE RESOURCE "${s3_source_name}" + PROPERTIES( + "type"="s3", + "AWS_REGION" = "bj", + "AWS_ENDPOINT" = "bj.s3.comaaaa", + "AWS_ROOT_PATH" = "path/to/rootaaaa", + "AWS_SECRET_KEY" = "aaaa", + "AWS_ACCESS_KEY" = "bbba", + "AWS_BUCKET" = "test-bucket", + "s3_validity_check" = "false" + ); + """ + def create_storage_policy = sql """ + CREATE STORAGE POLICY ${storage_policy_name} PROPERTIES( + "storage_resource" = "${s3_source_name}", + "cooldown_ttl" = "1008611" + ); + """ + + create_s3_resource + create_storage_policy + } + + def table_name_test_1 = "disable_storage_policy_on_mow1" + def table_name_test_2 = "disable_storage_policy_on_mow2" + def table_name_test_3 = "disable_storage_policy_on_mow3" + //Test case I. Should panic when creates MoW table with storage policy + test{ + sql """ + CREATE TABLE IF NOT EXISTS ${table_name_test_1} ( + k1 DATE, + k2 INT, + V1 VARCHAR(2048) + ) ENGINE = OLAP + UNIQUE KEY(k1,k2) + PARTITION BY RANGE (k1) + ( + PARTITION p1 VALUES LESS THAN ("2022-01-01") ("storage_policy" = "${storage_policy_name}", "replication_num"="1"), + PARTITION p2 VALUES LESS THAN ("2022-02-01") ("storage_policy" = "${storage_policy_name}", "replication_num"="1") + ) DISTRIBUTED BY HASH(k2) BUCKETS 1 + PROPERTIES( + "replication_num" = "1", + "enable_unique_key_merge_on_write" = "true" + ) + """ + exception "with storage policy(${storage_policy_name})" + } + + //Test case II. Should panic when sets storage policy to MoW table or its partitions + sql """ + CREATE TABLE IF NOT EXISTS ${table_name_test_2} ( + id INT, + v1 INT + ) ENGINE = OLAP + UNIQUE KEY(id) + DISTRIBUTED BY HASH(id) BUCKETS 1 + PROPERTIES( + "replication_num" = "1", + "enable_unique_key_merge_on_write" = "true" + ); + """ + + test { + sql """ + ALTER TABLE ${table_name_test_2} MODIFY PARTITION(*) SET("storage_policy" = "${storage_policy_name}"); + """ + exception "with storage policy(${storage_policy_name})" + } + + test { + sql """ + ALTER TABLE ${table_name_test_2} SET ("storage_policy" = "${storage_policy_name}"); + """ + exception "with storage policy(${storage_policy_name})" + } + + //Test case III. Should panic when creates MoW table(without partitions) with storage policy + test { + sql """ + CREATE TABLE IF NOT EXISTS ${table_name_test_3} ( + k1 INT, + V1 INT + ) ENGINE = OLAP + UNIQUE KEY(k1) + DISTRIBUTED BY HASH (k1) BUCKETS 1 + PROPERTIES( + "replication_num" = "1", + "enable_unique_key_merge_on_write" = "true", + "storage_policy" = "${storage_policy_name}" + ); + """ + exception "with storage policy(${storage_policy_name})" + } + + //clean resource + sql""" DROP TABLE IF EXISTS ${table_name_test_1} """ + sql""" DROP TABLE IF EXISTS ${table_name_test_2} """ + sql""" DROP TABLE IF EXISTS ${table_name_test_3} """ +} \ No newline at end of file