[fix](sql-block-rule)Fix sql block rule bug (#8738)

1. Check properties' effectiveness of sql_block_rule, can't set limitations of sql_block_rule to be negative.
2. Optimize the judgment conditions when a query hits a sql_block_rule
3. Check if sql_block_rule has already exist when exec set property for "user" "sql_block_rule" = "xxx"
4. Add UT ad SqlBlockRuleMgrTest.java
This commit is contained in:
Xujian Duan
2022-03-31 13:51:13 +08:00
committed by GitHub
parent 82792726ab
commit d17ee7b476
3 changed files with 64 additions and 4 deletions

View File

@ -79,6 +79,19 @@ public class SqlBlockRuleMgr implements Writable {
return Lists.newArrayList(nameToSqlBlockRuleMap.values());
}
// check limitation's effectiveness of a sql_block_rule
public static void verifyLimitations(SqlBlockRule sqlBlockRule) throws DdlException {
if (sqlBlockRule.getPartitionNum() < 0){
throw new DdlException("the value of partition_num can't be a negative");
}
if (sqlBlockRule.getTabletNum() < 0){
throw new DdlException("the value of tablet_num can't be a negative");
}
if (sqlBlockRule.getCardinality() < 0){
throw new DdlException("the value of cardinality can't be a negative");
}
}
public void createSqlBlockRule(CreateSqlBlockRuleStmt stmt) throws UserException {
writeLock();
try {
@ -87,6 +100,7 @@ public class SqlBlockRuleMgr implements Writable {
if (existRule(ruleName)) {
throw new DdlException("the sql block rule " + ruleName + " already create");
}
verifyLimitations(sqlBlockRule);
unprotectedAdd(sqlBlockRule);
Catalog.getCurrentCatalog().getEditLog().logCreateSqlBlockRule(sqlBlockRule);
} finally {
@ -107,6 +121,7 @@ public class SqlBlockRuleMgr implements Writable {
if (!existRule(ruleName)) {
throw new DdlException("the sql block rule " + ruleName + " not exist");
}
verifyLimitations(sqlBlockRule);
SqlBlockRule originRule = nameToSqlBlockRuleMap.get(ruleName);
SqlBlockUtil.checkAlterValidate(sqlBlockRule, originRule);
if (StringUtils.isEmpty(sqlBlockRule.getSql())) {
@ -231,11 +246,11 @@ public class SqlBlockRuleMgr implements Writable {
|| (rule.getTabletNum() != 0 && rule.getTabletNum() < tabletNum)
|| (rule.getCardinality() != 0 && rule.getCardinality() < cardinality)) {
MetricRepo.COUNTER_HIT_SQL_BLOCK_RULE.increase(1L);
if (rule.getPartitionNum() < partitionNum) {
if (rule.getPartitionNum() < partitionNum && rule.getPartitionNum() != 0) {
throw new AnalysisException("sql hits sql block rule: " + rule.getName() + ", reach partition_num : " + rule.getPartitionNum());
} else if (rule.getTabletNum() < tabletNum) {
} else if (rule.getTabletNum() < tabletNum && rule.getTabletNum() != 0) {
throw new AnalysisException("sql hits sql block rule: " + rule.getName() + ", reach tablet_num : " + rule.getTabletNum());
} else if (rule.getCardinality() < cardinality) {
} else if (rule.getCardinality() < cardinality && rule.getCardinality() != 0) {
throw new AnalysisException("sql hits sql block rule: " + rule.getName() + ", reach cardinality : " + rule.getCardinality());
}
}

View File

@ -274,6 +274,13 @@ public class UserProperty implements Writable {
if (keyArr.length != 1) {
throw new DdlException(PROP_SQL_BLOCK_RULES + " format error");
}
// check if sql_block_rule has already exist
for (String ruleName : value.replaceAll(" ","").split(",")){
if (!ruleName.equals("") && !Catalog.getCurrentCatalog().getSqlBlockRuleMgr().existRule(ruleName)){
throw new DdlException("the sql block rule " + ruleName + " not exist");
}
}
sqlBlockRules = value;
} else if (keyArr[0].equalsIgnoreCase(PROP_CPU_RESOURCE_LIMIT)) {
// set property "cpu_resource_limit" = "2";

View File

@ -109,7 +109,7 @@ public class SqlBlockRuleMgrTest {
String sql = "select * from table1 limit 10";
String sqlHash = DigestUtils.md5Hex(sql);
SqlBlockRule sqlRule = new SqlBlockRule("test_rule1", null, sqlHash, 0L, 0L, 0L, false, true);
SqlBlockRuleMgr mgr = new SqlBlockRuleMgr();
SqlBlockRuleMgr mgr = Catalog.getCurrentCatalog().getSqlBlockRuleMgr();
mgr.replayCreate(sqlRule);
// sql block rules
String setPropertyStr = "set property for \"root\" \"sql_block_rules\" = \"test_rule1\"";
@ -287,4 +287,42 @@ public class SqlBlockRuleMgrTest {
Assert.assertEquals(1, mgr.getSqlBlockRule(showStmt).size());
Assert.assertEquals("select \\* from test_table", mgr.getSqlBlockRule(showStmt).get(0).getSql());
}
@Test
public void testLimitationsInvalid() throws Exception {
SqlBlockRuleMgr mgr = new SqlBlockRuleMgr();
// create sql_block_rule with partition_num = -1
// DdlException: the value of partition_num can't be a negative
String createSql = "CREATE SQL_BLOCK_RULE test_rule PROPERTIES(\"partition_num\"=\"-1\",\"enable\"=\"true\")";
CreateSqlBlockRuleStmt stmt = (CreateSqlBlockRuleStmt) UtFrameUtils.parseAndAnalyzeStmt(createSql, connectContext);
ExceptionChecker.expectThrowsWithMsg(DdlException.class, "the value of partition_num can't be a negative",
() -> mgr.createSqlBlockRule(stmt));
// create sql_block_rule with tablet_num = -1
// DdlException: the value of tablet_num can't be a negative
String createSql1 = "CREATE SQL_BLOCK_RULE test_rule PROPERTIES(\"tablet_num\"=\"-1\",\"enable\"=\"true\")";
CreateSqlBlockRuleStmt stmt1 = (CreateSqlBlockRuleStmt) UtFrameUtils.parseAndAnalyzeStmt(createSql1, connectContext);
ExceptionChecker.expectThrowsWithMsg(DdlException.class, "the value of tablet_num can't be a negative",
() -> mgr.createSqlBlockRule(stmt1));
// create sql_block_rule with cardinality = -1
// DdlException: the value of cardinality can't be a negative
String createSql2 = "CREATE SQL_BLOCK_RULE test_rule PROPERTIES(\"cardinality\"=\"-1\",\"enable\"=\"true\")";
CreateSqlBlockRuleStmt stmt2 = (CreateSqlBlockRuleStmt) UtFrameUtils.parseAndAnalyzeStmt(createSql2, connectContext);
ExceptionChecker.expectThrowsWithMsg(DdlException.class, "the value of cardinality can't be a negative",
() -> mgr.createSqlBlockRule(stmt2));
}
@Test
public void testUserPropertyInvalid() throws Exception {
// sql block rules
String ruleName = "test_rule_name";
String setPropertyStr = String.format("set property for \"root\" \"sql_block_rules\" = \"%s\"", ruleName);
SetUserPropertyStmt setUserPropertyStmt = (SetUserPropertyStmt) UtFrameUtils.parseAndAnalyzeStmt(setPropertyStr, connectContext);
ExceptionChecker.expectThrowsWithMsg(DdlException.class, String.format("the sql block rule %s not exist", ruleName),
() -> Catalog.getCurrentCatalog().getAuth().updateUserProperty(setUserPropertyStmt));
}
}