Skip to content

Commit

Permalink
[BugFix](StoragePolicy) fix modify partition's storage policy with di…
Browse files Browse the repository at this point in the history
…fferent resource issue succeed with execption (#35270)

## Proposed changes

Issue Number: close #34229

when we try to modify a partition's storage policy which have different
resource with the previous one, we get error message "currently do not
support change origin storage policy to another one with different
resource", but the modification same also succeed!

expect:
the modification fail with "currently do not support change origin
storage policy to another one with different resource".



## Further comments

If this is a relatively large or complex change, kick off the discussion
at [[email protected]](mailto:[email protected]) by explaining why
you chose the solution you did and what alternatives you considered,
etc...

---------

Co-authored-by: shenshoucheng <[email protected]>
  • Loading branch information
Johnnyssc and shenshoucheng authored Jul 9, 2024
1 parent 6182e4f commit ec9800a
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 43 deletions.
4 changes: 2 additions & 2 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 @@ -499,12 +499,12 @@ public void processAlterTable(AlterTableStmt stmt) throws UserException {
// currently, only in memory and storage policy property could reach here
Preconditions.checkState(properties.containsKey(PropertyAnalyzer.PROPERTIES_INMEMORY)
|| properties.containsKey(PropertyAnalyzer.PROPERTIES_STORAGE_POLICY));
((SchemaChangeHandler) schemaChangeHandler).updatePartitionsProperties(
db, tableName, partitionNames, properties);
OlapTable olapTable = (OlapTable) table;
olapTable.writeLockOrDdlException();
try {
modifyPartitionsProperty(db, olapTable, partitionNames, properties, clause.isTempPartition());
((SchemaChangeHandler) schemaChangeHandler).updatePartitionsProperties(
db, tableName, partitionNames, properties);
} finally {
olapTable.writeUnlock();
}
Expand Down
80 changes: 40 additions & 40 deletions regression-test/suites/account_p0/test_alter_user.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -20,51 +20,51 @@ suite("test_alter_user", "account") {
sql """drop user if exists test_auth_user2"""
sql """drop user if exists test_auth_user3"""
sql """drop user if exists test_auth_user4"""

// 2. test password history
sql """set global password_history=0""" // disabled
sql """create user test_auth_user2 identified by '12345' password_history default"""
sql """create user test_auth_user2 identified by 'qwe678^&*' password_history default"""
sql """grant all on *.* to test_auth_user2"""
sql """alter user test_auth_user2 identified by '12345'"""
sql """set password for 'test_auth_user2' = password('12345')"""
sql """alter user test_auth_user2 identified by 'qwe678^&*'"""
sql """set password for 'test_auth_user2' = password('qwe678^&*')"""

sql """set global password_history=1""" // set to 1
test {
sql """alter user test_auth_user2 identified by '12345'"""
sql """alter user test_auth_user2 identified by 'qwe678^&*'"""
exception "Cannot use these credentials for 'default_cluster:test_auth_user2'@'%' because they contradict the password history policy"
}

sql """alter user test_auth_user2 password_history 0"""
sql """set password for 'test_auth_user2' = password('12345')"""
def result1 = connect(user = 'test_auth_user2', password = '12345', url = context.config.jdbcUrl) {
sql """set password for 'test_auth_user2' = password('qwe678^&*')"""

def result1 = connect(user = 'test_auth_user2', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}

sql """alter user test_auth_user2 password_history 2"""
sql """alter user test_auth_user2 identified by 'abc12345'"""
sql """alter user test_auth_user2 identified by 'abc123456'"""
sql """alter user test_auth_user2 identified by 'abc12345***'"""
sql """alter user test_auth_user2 identified by 'abc123456***'"""
test {
sql """alter user test_auth_user2 identified by 'abc12345'"""
sql """alter user test_auth_user2 identified by 'abc12345***'"""
exception "Cannot use these credentials for 'default_cluster:test_auth_user2'@'%' because they contradict the password history policy"
}
result1 = connect(user = 'test_auth_user2', password = 'abc123456', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user2', password = 'abc123456***', url = context.config.jdbcUrl) {
sql 'select 1'
}
sql """set global password_history=0""" // set to disabled

// 3. test FAILED_LOGIN_ATTEMPTS and PASSWORD_LOCK_TIME
sql """create user test_auth_user3 identified by '12345' FAILED_LOGIN_ATTEMPTS 2 PASSWORD_LOCK_TIME 1 DAY"""
sql """create user test_auth_user3 identified by 'qwe678^&*' FAILED_LOGIN_ATTEMPTS 2 PASSWORD_LOCK_TIME 1 DAY"""
sql """grant all on *.* to test_auth_user3"""

// login success in multi times
result1 = connect(user = 'test_auth_user3', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user3', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}
result1 = connect(user = 'test_auth_user3', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user3', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}
result1 = connect(user = 'test_auth_user3', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user3', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}
// login failed in 2 times
Expand All @@ -73,24 +73,24 @@ suite("test_alter_user", "account") {
assertTrue(false. "should not be able to login")
} catch (Exception e) {
assertTrue(e.getMessage().contains("Access denied for user 'default_cluster:test_auth_user3"), e.getMessage())
}
}
try {
connect(user = 'test_auth_user3', password = 'wrong', url = context.config.jdbcUrl) {}
assertTrue(false. "should not be able to login")
} catch (Exception e) {
assertTrue(e.getMessage().contains("Access denied for user 'default_cluster:test_auth_user3"), e.getMessage())
}
}
// login with correct password but also failed
try {
connect(user = 'test_auth_user3', password = '12345', url = context.config.jdbcUrl) {}
connect(user = 'test_auth_user3', password = 'qwe678^&*', url = context.config.jdbcUrl) {}
assertTrue(false. "should not be able to login")
} catch (Exception e) {
assertTrue(e.getMessage().contains("Access denied for user 'default_cluster:test_auth_user3'@'%'. Account is blocked for 86400 second(s) (86400 second(s) remaining) due to 2 consecutive failed logins."), e.getMessage())
}
}

// unlock user and login again
sql """alter user test_auth_user3 account_unlock"""
result1 = connect(user = 'test_auth_user3', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user3', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}

Expand All @@ -102,23 +102,23 @@ suite("test_alter_user", "account") {
assertTrue(false. "should not be able to login")
} catch (Exception e) {
assertTrue(e.getMessage().contains("Access denied for user 'default_cluster:test_auth_user3"), e.getMessage())
}
}
try {
connect(user = 'test_auth_user3', password = 'wrong', url = context.config.jdbcUrl) {}
assertTrue(false. "should not be able to login")
} catch (Exception e) {
assertTrue(e.getMessage().contains("Access denied for user 'default_cluster:test_auth_user3"), e.getMessage())
}
}
// login with correct password but also failed
try {
connect(user = 'test_auth_user3', password = '12345', url = context.config.jdbcUrl) {}
connect(user = 'test_auth_user3', password = 'qwe678^&*', url = context.config.jdbcUrl) {}
assertTrue(false. "should not be able to login")
} catch (Exception e) {
assertTrue(e.getMessage().contains("Access denied for user 'default_cluster:test_auth_user3'@'%'. Account is blocked for 5 second(s) (5 second(s) remaining) due to 2 consecutive failed logins."), e.getMessage())
}
}
// sleep 5 second to unlock account
sleep(5000)
result1 = connect(user = 'test_auth_user3', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user3', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}

Expand All @@ -140,32 +140,32 @@ suite("test_alter_user", "account") {
sql """set global validate_password_policy=NONE"""

// 5. test expire
sql """create user test_auth_user4 identified by '12345' PASSWORD_EXPIRE INTERVAL 5 SECOND"""
sql """create user test_auth_user4 identified by 'qwe678^&*' PASSWORD_EXPIRE INTERVAL 5 SECOND"""
sql """grant all on *.* to test_auth_user4"""
result1 = connect(user = 'test_auth_user4', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user4', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}
sleep(6000)
try {
connect(user = 'test_auth_user4', password = '12345', url = context.config.jdbcUrl) {}
connect(user = 'test_auth_user4', password = 'qwe678^&*', url = context.config.jdbcUrl) {}
assertTrue(false. "should not be able to login")
} catch (Exception e) {
assertTrue(e.getMessage().contains("Your password has expired. To log in you must change it using a client that supports expired passwords."), e.getMessage())
}

// 6. drop user and create again, new user with same name can login
sql """drop user test_auth_user4"""
sql """create user test_auth_user4 identified by '12345'"""
sql """create user test_auth_user4 identified by 'qwe678^&*'"""
sql """grant all on *.* to test_auth_user4"""
result1 = connect(user = 'test_auth_user4', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user4', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}

// 7. test after expire, reset password
sql """drop user test_auth_user4"""
sql """create user test_auth_user4 identified by '12345' PASSWORD_EXPIRE INTERVAL 5 SECOND"""
sql """create user test_auth_user4 identified by 'qwe678^&*' PASSWORD_EXPIRE INTERVAL 5 SECOND"""
sql """grant all on *.* to test_auth_user4"""
result1 = connect(user = 'test_auth_user4', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user4', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}
sleep(6000)
Expand All @@ -183,21 +183,21 @@ suite("test_alter_user", "account") {

// 8. test password not expiration
sql """drop user test_auth_user4"""
sql """create user test_auth_user4 identified by '12345'"""
sql """create user test_auth_user4 identified by 'qwe678^&*'"""
sql """grant all on *.* to test_auth_user4"""
result1 = connect(user = 'test_auth_user4', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user4', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}
sleep(1000)
result2 = connect(user = 'test_auth_user4', password = '12345', url = context.config.jdbcUrl) {
result2 = connect(user = 'test_auth_user4', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
}

// 9. test user default database privileges
sql """drop user if exists test_auth_user4"""
sql """create user test_auth_user4 identified by '12345'"""
sql """create user test_auth_user4 identified by 'qwe678^&*'"""
sql """grant SELECT_PRIV on regression_test.* to test_auth_user4"""
result1 = connect(user = 'test_auth_user4', password = '12345', url = context.config.jdbcUrl) {
result1 = connect(user = 'test_auth_user4', password = 'qwe678^&*', url = context.config.jdbcUrl) {
sql 'select 1'
sql 'use information_schema'
sql 'use mysql'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
// 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("modify_partition_storage_policy") {
// 1. create a non-partitioned table
String tblName = "test_modify_table"

sql """DROP TABLE IF EXISTS ${tblName} FORCE; """

sql """
CREATE TABLE `${tblName}`
(
k1 BIGINT,
k2 LARGEINT,
v1 VARCHAR(2048)
)
UNIQUE KEY(k1)
DISTRIBUTED BY HASH (k1) BUCKETS 3
PROPERTIES(
"replication_num" = "1"
);
"""

// 2.create resource1, resource2, and policy1 and policy2.
String resource1 = "test_modify_resource1"
String resource2 = "test_modify_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_modify_policy1"
String policy_name2 = "test_modify_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"
);
"""

// 3. add storage policy to the current partition,make sure that the storage information in the metadata of FE and BE be the same
try_sql """
ALTER TABLE ${tblName} MODIFY PARTITION (${tblName}) SET("storage_policy"="${policy_name1}");
"""

def get_policy_id_from_name = { name ->
def show_storage_policy = sql """SHOW STORAGE POLICY;"""

for (iter in show_storage_policy) {
if (name == iter[0]) {
return iter[1];
}
}
return -1;
}

// 3.1 check fe meta
def partitions = sql """show partitions from ${tblName}"""
for (par in partitions) {
// 12th is storage policy name
assertTrue(par[12] == "${policy_name1}")
}


// 3.2 get storage policy from be metadata
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)
def json = parseJson(out)
def be__policy_id1 = Integer.parseInt(json.storage_policy_id.toString())

// assert is equal: storage id in fe & be meta
def policy_id = Integer.parseInt(get_policy_id_from_name.call(policy_name1).toString())
assertEquals(be__policy_id1, policy_id)


// 4. alter policy with different resources
try {
sql """
ALTER TABLE ${tblName} MODIFY PARTITION (${tblName}) SET("storage_policy"="${policy_name2}");
"""
}
catch (Exception e) {
logger.info(e.getMessage())
}
// 4.1 check fe meta
def partitions2 = sql "show partitions from ${tblName}"
for (par in partitions2) {
// 12th is storage policy name
assertTrue(par[12] == "${policy_name1}")
}

// 4.2 check be meta
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)
def json2 = parseJson(out2)
def be_policy_id2 = Integer.parseInt(json2.storage_policy_id.toString())
assertEquals(be_policy_id2, policy_id)
}
2 changes: 1 addition & 1 deletion regression-test/suites/javaudf_p0/test_javaudf_auth.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ suite("test_javaudf_auth") {
def url=tokens[0] + "//" + tokens[2] + "/" + "information_schema" + "?"

def user = 'udf_auth_user'
def pwd = '123456'
def pwd = 'qwe678^&*'
def dbName = 'udf_auth_db'

try_sql("DROP USER ${user}")
Expand Down

0 comments on commit ec9800a

Please sign in to comment.