From 51490ac8575b7a8eb407b72eb9672da4cfb0ce61 Mon Sep 17 00:00:00 2001 From: Yulei-Yang Date: Tue, 4 Jun 2024 19:35:14 +0800 Subject: [PATCH 1/3] [fix](coldhot) fix cannot cancel storage policy of partition --- .../java/org/apache/doris/alter/Alter.java | 6 +++++ .../doris/common/util/PropertyAnalyzer.java | 10 +++---- .../test_show_storage_policy_using.groovy | 26 +++++++++++++++++++ 3 files changed, 36 insertions(+), 6 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 11fbd3a0ecf277..71e98bbe17b829 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 @@ -764,6 +764,12 @@ public void modifyPartitionsProperty(Database db, // check currentStoragePolicy resource exist. Env.getCurrentEnv().getPolicyMgr().checkStoragePolicyExist(currentStoragePolicy); partitionInfo.setStoragePolicy(partition.getId(), currentStoragePolicy); + } else { + if (partition.getRemoteDataSize() > 0) { + throw new AnalysisException( + "Cannot cancel storage policy for partition which is already on code storage."); + } + partitionInfo.setStoragePolicy(partition.getId(), ""); } // 4.4 analyze new properties 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 0972018f5721e9..2e78e159d3c08d 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 @@ -319,11 +319,9 @@ public static DataProperty analyzeDataProperty(Map properties, f } else if (key.equalsIgnoreCase(PROPERTIES_STORAGE_COOLDOWN_TIME)) { DateLiteral dateLiteral = new DateLiteral(value, ScalarType.getDefaultDateType(Type.DATETIME)); cooldownTimestamp = dateLiteral.unixTimestamp(TimeUtils.getTimeZone()); - } else if (!hasStoragePolicy && key.equalsIgnoreCase(PROPERTIES_STORAGE_POLICY)) { - if (!Strings.isNullOrEmpty(value)) { - hasStoragePolicy = true; - newStoragePolicy = value; - } + } else if (key.equalsIgnoreCase(PROPERTIES_STORAGE_POLICY)) { + hasStoragePolicy = true; + newStoragePolicy = value; } } // end for properties @@ -353,7 +351,7 @@ public static DataProperty analyzeDataProperty(Map properties, f cooldownTimestamp = DataProperty.MAX_COOLDOWN_TIME_MS; } - if (hasStoragePolicy) { + if (hasStoragePolicy && !"".equals(newStoragePolicy)) { // check remote storage policy StoragePolicy checkedPolicy = StoragePolicy.ofCheck(newStoragePolicy); Policy policy = Env.getCurrentEnv().getPolicyMgr().getPolicy(checkedPolicy); diff --git a/regression-test/suites/cold_heat_separation_p2/test_show_storage_policy_using.groovy b/regression-test/suites/cold_heat_separation_p2/test_show_storage_policy_using.groovy index fb56422ad6cc5f..73835a0c2ccd0f 100644 --- a/regression-test/suites/cold_heat_separation_p2/test_show_storage_policy_using.groovy +++ b/regression-test/suites/cold_heat_separation_p2/test_show_storage_policy_using.groovy @@ -156,6 +156,32 @@ suite("test_show_storage_policy_using") { """ assertTrue(show_result.size() >= 4) + // test cancel a partition's storage policy + sql """ + ALTER TABLE partition_with_multiple_storage_policy MODIFY PARTITION (`p201701`) SET ("storage_policy"="") + """ + show_result = sql """ + show storage policy using for ${policy_name} + """ + assertEquals(show_result.size(), 1) + assertTrue(show_result[0][2].equals("table_with_storage_policy_1")) + + sql """ + ALTER TABLE partition_with_multiple_storage_policy MODIFY PARTITION (`p201701`) SET ("storage_policy"="${policy_name}") + """ + show_result = sql """ + show storage policy using for ${policy_name} + """ + assertEquals(show_result.size(), 2) + + sql """ + ALTER TABLE partition_with_multiple_storage_policy MODIFY PARTITION (`p201701`) SET ("replication_num"="1") + """ + show_result = sql """ + show storage policy using for ${policy_name} + """ + assertEquals(show_result.size(), 2) + // cleanup sql """ DROP TABLE IF EXISTS table_with_storage_policy_1 """ sql """ DROP TABLE IF EXISTS table_no_storage_policy_1 """ From d13b6ce8bca07c588bed0362cc7297bda4c698a0 Mon Sep 17 00:00:00 2001 From: Yulei-Yang Date: Wed, 5 Jun 2024 19:00:25 +0800 Subject: [PATCH 2/3] fix typo --- fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 71e98bbe17b829..ad483f26f0f121 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 @@ -767,7 +767,7 @@ public void modifyPartitionsProperty(Database db, } else { if (partition.getRemoteDataSize() > 0) { throw new AnalysisException( - "Cannot cancel storage policy for partition which is already on code storage."); + "Cannot cancel storage policy for partition which is already on cold storage."); } partitionInfo.setStoragePolicy(partition.getId(), ""); } From 69419d864a2665bac948b804820b86558ecfa41c Mon Sep 17 00:00:00 2001 From: Yulei-Yang Date: Fri, 14 Jun 2024 09:19:34 +0800 Subject: [PATCH 3/3] update restriction of operation --- .../java/org/apache/doris/alter/Alter.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) 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 ad483f26f0f121..0513aafa3c22e9 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 @@ -72,6 +72,7 @@ import org.apache.doris.persist.ModifyPartitionInfo; import org.apache.doris.persist.ModifyTableEngineOperationLog; import org.apache.doris.persist.ReplaceTableOperationLog; +import org.apache.doris.policy.StoragePolicy; import org.apache.doris.qe.ConnectContext; import org.apache.doris.thrift.TOdbcTableType; import org.apache.doris.thrift.TSortType; @@ -765,10 +766,30 @@ public void modifyPartitionsProperty(Database db, 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) { + throw new AnalysisException( + "Cannot cancel storage policy for partition which already be cooldown" + + " or will be cooldown soon later"); + } + } + partitionInfo.setStoragePolicy(partition.getId(), ""); }