Skip to content

Commit

Permalink
fix that set table storage policy after set partition policy with dif…
Browse files Browse the repository at this point in the history
…ferent resource still can succeed
  • Loading branch information
shenshoucheng authored and Johnnyssc committed Jun 7, 2024
1 parent e3e3418 commit 07160b7
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 13 deletions.
30 changes: 22 additions & 8 deletions fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand Down Expand Up @@ -188,17 +189,30 @@ private boolean processAlterOlapTable(AlterTableStmt stmt, OlapTable olapTable,
if (currentAlterOps.checkTableStoragePolicy(alterClauses)) {
String tableStoragePolicy = olapTable.getStoragePolicy();
String currentStoragePolicy = currentAlterOps.getTableStoragePolicy(alterClauses);

// If the two policy has one same resource, then it's safe for the table to change policy
// There would only be the cooldown ttl or cooldown time would be affected
boolean enableUniqueKeyMergeOnWrite = olapTable.getEnableUniqueKeyMergeOnWrite();
if (!Env.getCurrentEnv().getPolicyMgr()
.checkStoragePolicyIfSameResource(tableStoragePolicy, currentStoragePolicy)
&& !tableStoragePolicy.isEmpty()) {
for (Partition partition : olapTable.getAllPartitions()) {
if (Partition.PARTITION_INIT_VERSION < partition.getVisibleVersion()) {
throw new DdlException("Do not support alter table's storage policy , this table ["
&& !enableUniqueKeyMergeOnWrite) {
// If the two policy has one same resource, then it's safe for the table to change policy
// There would only be the cooldown ttl or cooldown time would be affected
if (!tableStoragePolicy.isEmpty()) {
for (Partition partition : olapTable.getAllPartitions()) {
if (Partition.PARTITION_INIT_VERSION < partition.getVisibleVersion()) {
throw new DdlException("Do not support alter table's storage policy , this table ["
+ olapTable.getName() + "] has storage policy " + tableStoragePolicy
+ ", the table need to be empty.");
}
}
} else {
// if any partitions' policy has been set up, table set policy will be forbidden
PartitionInfo partitionInfo = olapTable.getPartitionInfo();
for (Partition partition : olapTable.getAllPartitions()) {
String partitionStoragePolicy = partitionInfo.getStoragePolicy(partition.getId());
if (!StringUtils.isEmpty(partitionStoragePolicy)) {
throw new DdlException("Do not support alter table's storage policy , this table ["
+ olapTable.getName() + "] storage policy is empty, but partition ["
+ partition.getName() + "] already has storage policy ");
}
}
}
}
Expand Down Expand Up @@ -476,7 +490,7 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
+ table.getType().toString() + " table[" + tableName + "]");
}

// the following ops should done outside table lock. because it contain synchronized create operation
// the following ops should be done outside table lock. because it contains synchronized create operation
if (needProcessOutsideTableLock) {
Preconditions.checkState(alterClauses.size() == 1);
AlterClause alterClause = alterClauses.get(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ suite("modify_partition_storage_policy") {
CREATE STORAGE POLICY IF NOT EXISTS `${policy_name1}`
PROPERTIES(
"storage_resource" = "${resource1}",
"cooldown_datetime" = "2025-06-18 00:00:00"
"cooldown_datetime" = "2999-06-18 00:00:00"
);
"""

sql """
CREATE STORAGE POLICY IF NOT EXISTS `${policy_name2}`
PROPERTIES(
"storage_resource" = "${resource2}",
"cooldown_datetime" = "2026-06-18 00:00:00"
"cooldown_datetime" = "2999-06-18 00:00:00"
);
"""

Expand Down Expand Up @@ -113,7 +113,6 @@ suite("modify_partition_storage_policy") {
def tablet_result = sql """SHOW TABLETS FROM ${tblName} PARTITION ${tblName};"""
def metadata_url = tablet_result[0][17]
def (code, out, err) = curl("GET", metadata_url)
logger.info(" get be meta data: code=" + code + ", out=" + out + ", err=" + err)
def json = parseJson(out)
def be__policy_id1 = Integer.parseInt(json.storage_policy_id.toString())

Expand Down Expand Up @@ -142,8 +141,7 @@ suite("modify_partition_storage_policy") {
def tablet_result2 = sql """SHOW TABLETS FROM ${tblName} PARTITION ${tblName};"""
def metadata_url2 = tablet_result2[0][17]
def (code2, out2, err2) = curl("GET", metadata_url2)
logger.info(" get be meta data: code=" + code2 + ", out=" + out2 + ", err=" + err2)
def json2 = parseJson(out)
def json2 = parseJson(out2)
def be_policy_id2 = Integer.parseInt(json2.storage_policy_id.toString())
assertEquals(be_policy_id2, policy_id)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// 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("set_table_policy_after_modify_partition_policy") {
// 1. set table storage policy after modify partition storage policy
String tableName = "test_set_policy_table"

sql """
CREATE TABLE `${tableName}` (
`ddate` date NULL,
`dhour` varchar(*) NULL,
`server_time` datetime NULL,
`log_type` varchar(*) NULL,
`source_flag` varchar(*) NULL
) ENGINE=OLAP
DUPLICATE KEY(`ddate`)
PARTITION BY RANGE(`ddate`)
(PARTITION p202403 VALUES [('2024-03-01'), ('2024-04-01')),
PARTITION p202404 VALUES [('2024-04-01'), ('2024-05-01')))
DISTRIBUTED BY RANDOM BUCKETS 3
PROPERTIES (
"replication_allocation" = "tag.location.default: 1"
);
"""

String resource1 = "test_set_resource1"
String resource2 = "test_set_resource2"

sql """
CREATE RESOURCE IF NOT EXISTS "${resource1}"
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"
);
"""

sql """
CREATE RESOURCE IF NOT EXISTS "${resource2}"
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"
);
"""

String policy_name1 = "test_set_policy1"
String policy_name2 = "test_set_policy2"

sql """
CREATE STORAGE POLICY IF NOT EXISTS `${policy_name1}`
PROPERTIES(
"storage_resource" = "${resource1}",
"cooldown_datetime" = "2999-06-18 00:00:00"
);
"""
sql """
CREATE STORAGE POLICY IF NOT EXISTS `${policy_name2}`
PROPERTIES(
"storage_resource" = "${resource2}",
"cooldown_datetime" = "2999-06-18 00:00:00"
);
"""

// modify partition's storage policy
String partitionName = "p202403"
sql """
ALTER TABLE `${tableName}` MODIFY PARTITION (${partitionName}) SET("storage_policy"="${policy_name1}");
"""

// set table storage policy, with exception
test {
sql """
ALTER TABLE `${tableName}` set ("storage_policy" = "${policy_name2}");
"""
exception "but partition [${partitionName}] already has storage policy"
}

//clean resource
sql""" DROP TABLE IF EXISTS ${tableName} """

}

0 comments on commit 07160b7

Please sign in to comment.