diff --git a/fe/fe-core/src/main/java/com/starrocks/meta/SqlBlackList.java b/fe/fe-core/src/main/java/com/starrocks/meta/SqlBlackList.java index 2a24579c92541..92e7cf61b5883 100644 --- a/fe/fe-core/src/main/java/com/starrocks/meta/SqlBlackList.java +++ b/fe/fe-core/src/main/java/com/starrocks/meta/SqlBlackList.java @@ -19,8 +19,8 @@ import com.starrocks.common.AnalysisException; import com.starrocks.common.ErrorCode; import com.starrocks.common.ErrorReport; -import com.starrocks.persist.AddSqlBlackList; import com.starrocks.persist.ImageWriter; +import com.starrocks.persist.SqlBlackListPersistInfo; import com.starrocks.persist.metablock.SRMetaBlockEOFException; import com.starrocks.persist.metablock.SRMetaBlockException; import com.starrocks.persist.metablock.SRMetaBlockID; @@ -63,8 +63,8 @@ public void load(SRMetaBlockReader reader) throws IOException, SRMetaBlockExcept try (LockCloseable ignored = new LockCloseable(rwLock.writeLock())) { int cnt = reader.readInt(); for (int i = 0; i < cnt; i++) { - AddSqlBlackList addSqlBlackList = reader.readJson(AddSqlBlackList.class); - put(addSqlBlackList.id, Pattern.compile(addSqlBlackList.pattern)); + SqlBlackListPersistInfo sqlBlackListPersistInfo = reader.readJson(SqlBlackListPersistInfo.class); + put(sqlBlackListPersistInfo.id, Pattern.compile(sqlBlackListPersistInfo.pattern)); } LOG.info("loaded {} SQL blacklist patterns", sqlBlackListMap.size()); } @@ -105,6 +105,12 @@ public void delete(long id) { } } + public void delete(List ids) { + for (Long id : ids) { + this.delete(id); + } + } + public void save(ImageWriter imageWriter) throws IOException, SRMetaBlockException { try (LockCloseable ignored = new LockCloseable(rwLock.readLock())) { // one for self and N for patterns @@ -114,7 +120,7 @@ public void save(ImageWriter imageWriter) throws IOException, SRMetaBlockExcepti // write patterns writer.writeInt(sqlBlackListMap.size()); for (BlackListSql p : sqlBlackListMap.values()) { - writer.writeJson(new AddSqlBlackList(p.id, p.pattern.pattern())); + writer.writeJson(new SqlBlackListPersistInfo(p.id, p.pattern.pattern())); } writer.close(); } diff --git a/fe/fe-core/src/main/java/com/starrocks/persist/EditLog.java b/fe/fe-core/src/main/java/com/starrocks/persist/EditLog.java index af24587a391d2..e8230bf721932 100644 --- a/fe/fe-core/src/main/java/com/starrocks/persist/EditLog.java +++ b/fe/fe-core/src/main/java/com/starrocks/persist/EditLog.java @@ -1113,16 +1113,14 @@ public void loadJournal(GlobalStateMgr globalStateMgr, JournalEntity journal) break; } case OperationType.OP_ADD_SQL_QUERY_BLACK_LIST: { - AddSqlBlackList addBlacklistRequest = (AddSqlBlackList) journal.data(); + SqlBlackListPersistInfo addBlacklistRequest = (SqlBlackListPersistInfo) journal.data(); GlobalStateMgr.getCurrentState().getSqlBlackList() .put(addBlacklistRequest.id, Pattern.compile(addBlacklistRequest.pattern)); break; } case OperationType.OP_DELETE_SQL_QUERY_BLACK_LIST: { DeleteSqlBlackLists deleteBlackListsRequest = (DeleteSqlBlackLists) journal.data(); - for (int i = 0; i < deleteBlackListsRequest.ids.size(); i++) { - GlobalStateMgr.getCurrentState().getSqlBlackList().delete(deleteBlackListsRequest.ids.get(i)); - } + GlobalStateMgr.getCurrentState().getSqlBlackList().delete(deleteBlackListsRequest.ids); break; } default: { @@ -1809,7 +1807,7 @@ public void logAlterMaterializedViewProperties(ModifyTablePropertyOperationLog l logEdit(OperationType.OP_ALTER_MATERIALIZED_VIEW_PROPERTIES, log); } - public void logAddSQLBlackList(AddSqlBlackList addBlackList) { + public void logAddSQLBlackList(SqlBlackListPersistInfo addBlackList) { logEdit(OperationType.OP_ADD_SQL_QUERY_BLACK_LIST, addBlackList); } diff --git a/fe/fe-core/src/main/java/com/starrocks/persist/EditLogDeserializer.java b/fe/fe-core/src/main/java/com/starrocks/persist/EditLogDeserializer.java index b0e0039f3f20d..40359c2a9b056 100644 --- a/fe/fe-core/src/main/java/com/starrocks/persist/EditLogDeserializer.java +++ b/fe/fe-core/src/main/java/com/starrocks/persist/EditLogDeserializer.java @@ -243,7 +243,7 @@ public class EditLogDeserializer { .put(OperationType.OP_ALTER_WAREHOUSE, Warehouse.class) .put(OperationType.OP_DROP_WAREHOUSE, DropWarehouseLog.class) .put(OperationType.OP_CLUSTER_SNAPSHOT_LOG, ClusterSnapshotLog.class) - .put(OperationType.OP_ADD_SQL_QUERY_BLACK_LIST, AddSqlBlackList.class) + .put(OperationType.OP_ADD_SQL_QUERY_BLACK_LIST, SqlBlackListPersistInfo.class) .put(OperationType.OP_DELETE_SQL_QUERY_BLACK_LIST, DeleteSqlBlackLists.class) .build(); diff --git a/fe/fe-core/src/main/java/com/starrocks/persist/OperationType.java b/fe/fe-core/src/main/java/com/starrocks/persist/OperationType.java index ef85bdd3b0d6c..b64b4600f5d59 100644 --- a/fe/fe-core/src/main/java/com/starrocks/persist/OperationType.java +++ b/fe/fe-core/src/main/java/com/starrocks/persist/OperationType.java @@ -592,9 +592,9 @@ public class OperationType { public static final short OP_CLUSTER_SNAPSHOT_LOG = 13513; @IgnorableOnReplayFailed - public static final short OP_ADD_SQL_QUERY_BLACK_LIST = 13700; + public static final short OP_ADD_SQL_QUERY_BLACK_LIST = 13520; @IgnorableOnReplayFailed - public static final short OP_DELETE_SQL_QUERY_BLACK_LIST = 13701; + public static final short OP_DELETE_SQL_QUERY_BLACK_LIST = 13521; /** * NOTICE: OperationType cannot use a value exceeding 20000, please follow the above sequence number diff --git a/fe/fe-core/src/main/java/com/starrocks/persist/AddSqlBlackList.java b/fe/fe-core/src/main/java/com/starrocks/persist/SqlBlackListPersistInfo.java similarity index 87% rename from fe/fe-core/src/main/java/com/starrocks/persist/AddSqlBlackList.java rename to fe/fe-core/src/main/java/com/starrocks/persist/SqlBlackListPersistInfo.java index d03764c4b7602..4a32fe7251574 100644 --- a/fe/fe-core/src/main/java/com/starrocks/persist/AddSqlBlackList.java +++ b/fe/fe-core/src/main/java/com/starrocks/persist/SqlBlackListPersistInfo.java @@ -19,8 +19,8 @@ import java.util.Objects; -public class AddSqlBlackList extends JsonWriter { - public AddSqlBlackList(long id, String pattern) { +public class SqlBlackListPersistInfo extends JsonWriter { + public SqlBlackListPersistInfo(long id, String pattern) { this.id = id; this.pattern = pattern; } @@ -39,7 +39,7 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) { return false; } - AddSqlBlackList that = (AddSqlBlackList) o; + SqlBlackListPersistInfo that = (SqlBlackListPersistInfo) o; return id == that.id && Objects.equals(pattern, that.pattern); } diff --git a/fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java b/fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java index 294061d19fa42..b20c5fabf0ac9 100644 --- a/fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java +++ b/fe/fe-core/src/main/java/com/starrocks/qe/StmtExecutor.java @@ -106,9 +106,9 @@ import com.starrocks.mysql.MysqlCommand; import com.starrocks.mysql.MysqlEofPacket; import com.starrocks.mysql.MysqlSerializer; -import com.starrocks.persist.AddSqlBlackList; import com.starrocks.persist.CreateInsertOverwriteJobLog; import com.starrocks.persist.DeleteSqlBlackLists; +import com.starrocks.persist.SqlBlackListPersistInfo; import com.starrocks.persist.gson.GsonUtils; import com.starrocks.planner.FileScanNode; import com.starrocks.planner.HiveTableSink; @@ -446,11 +446,6 @@ private boolean initForwardToLeaderState() { } } - // always redirect sql blacklist related queries to the leader - if (parsedStmt instanceof AddSqlBlackListStmt || parsedStmt instanceof DelSqlBlackListStmt) { - return true; - } - if (redirectStatus == null) { return false; } else { @@ -1699,7 +1694,7 @@ private void handleAddSqlBlackListStmt() { AddSqlBlackListStmt addSqlBlackListStmt = (AddSqlBlackListStmt) parsedStmt; long id = GlobalStateMgr.getCurrentState().getSqlBlackList().put(addSqlBlackListStmt.getSqlPattern()); GlobalStateMgr.getCurrentState().getEditLog() - .logAddSQLBlackList(new AddSqlBlackList(id, addSqlBlackListStmt.getSqlPattern().pattern())); + .logAddSQLBlackList(new SqlBlackListPersistInfo(id, addSqlBlackListStmt.getSqlPattern().pattern())); } private void handleDelSqlBlackListStmt() { diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/ast/AddSqlBlackListStmt.java b/fe/fe-core/src/main/java/com/starrocks/sql/ast/AddSqlBlackListStmt.java index 5b9720b885164..078a31cb58ad6 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/ast/AddSqlBlackListStmt.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/ast/AddSqlBlackListStmt.java @@ -65,7 +65,7 @@ public R accept(AstVisitor visitor, C context) { @Override public RedirectStatus getRedirectStatus() { - return RedirectStatus.NO_FORWARD; + return RedirectStatus.FORWARD_NO_SYNC; } } diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/ast/DelSqlBlackListStmt.java b/fe/fe-core/src/main/java/com/starrocks/sql/ast/DelSqlBlackListStmt.java index 51c3b04e218f3..34d4c367b57b5 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/ast/DelSqlBlackListStmt.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/ast/DelSqlBlackListStmt.java @@ -46,7 +46,7 @@ public R accept(AstVisitor visitor, C context) { @Override public RedirectStatus getRedirectStatus() { - return RedirectStatus.NO_FORWARD; + return RedirectStatus.FORWARD_NO_SYNC; } } diff --git a/fe/fe-core/src/test/java/com/starrocks/server/SqlBlacklistTest.java b/fe/fe-core/src/test/java/com/starrocks/server/SqlBlacklistTest.java index f34b8f8177ebe..e648669040f31 100644 --- a/fe/fe-core/src/test/java/com/starrocks/server/SqlBlacklistTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/server/SqlBlacklistTest.java @@ -14,17 +14,20 @@ package com.starrocks.server; -import com.starrocks.ha.FrontendNodeType; +import com.starrocks.analysis.RedirectStatus; import com.starrocks.meta.BlackListSql; import com.starrocks.meta.SqlBlackList; -import com.starrocks.persist.AddSqlBlackList; import com.starrocks.persist.DeleteSqlBlackLists; import com.starrocks.persist.EditLog; +import com.starrocks.persist.SqlBlackListPersistInfo; import com.starrocks.qe.ConnectContext; +import com.starrocks.qe.ShowExecutor; +import com.starrocks.qe.ShowResultSet; import com.starrocks.qe.StmtExecutor; import com.starrocks.sql.analyzer.AnalyzeTestUtil; +import com.starrocks.sql.ast.AddSqlBlackListStmt; import com.starrocks.sql.ast.DelSqlBlackListStmt; -import com.starrocks.sql.ast.StatementBase; +import com.starrocks.sql.ast.ShowSqlBlackListStmt; import com.starrocks.utframe.UtFrameUtils; import mockit.Expectations; import mockit.Mock; @@ -68,7 +71,7 @@ public EditLog getEditLog() { new Expectations(editLog) { { - editLog.logAddSQLBlackList(new AddSqlBlackList(0, ".+")); + editLog.logAddSQLBlackList(new SqlBlackListPersistInfo(0, ".+")); times = 1; } }; @@ -76,7 +79,8 @@ public EditLog getEditLog() { ConnectContext connectContext = UtFrameUtils.createDefaultCtx(); connectContext.setQueryId(UUID.randomUUID()); - StatementBase addStatement = parseSql("ADD SQLBLACKLIST \".+\";"); + AddSqlBlackListStmt addStatement = (AddSqlBlackListStmt) parseSql("ADD SQLBLACKLIST \".+\";"); + Assert.assertEquals(addStatement.getSql(), ".+"); StmtExecutor addStatementExecutor = new StmtExecutor(connectContext, addStatement); addStatementExecutor.execute(); @@ -86,6 +90,37 @@ public EditLog getEditLog() { Assert.assertEquals(".+", blackLists.get(0).pattern.pattern()); } + @Test + public void testShowBlacklist() throws Exception { + GlobalStateMgr.getCurrentState().waitForReady(); + + SqlBlackList sqlBlackList = new SqlBlackList(); + + sqlBlackList.put(Pattern.compile("qwert")); + sqlBlackList.put(Pattern.compile("abcde")); + + MockUp mockUp = new MockUp() { + @Mock + public SqlBlackList getSqlBlackList() { + return sqlBlackList; + } + }; + + ConnectContext connectContext = UtFrameUtils.createDefaultCtx(); + connectContext.setQueryId(UUID.randomUUID()); + + ShowSqlBlackListStmt showSqlStatement = (ShowSqlBlackListStmt) parseSql("SHOW SQLBLACKLIST"); + + ShowResultSet resultSet = ShowExecutor.execute(showSqlStatement, connectContext); + Assert.assertTrue(resultSet.next()); + Assert.assertEquals(0L, resultSet.getLong(0)); + Assert.assertEquals("qwert", resultSet.getString(1)); + Assert.assertTrue(resultSet.next()); + Assert.assertEquals(1L, resultSet.getLong(0)); + Assert.assertEquals("abcde", resultSet.getString(1)); + Assert.assertFalse(resultSet.next()); + } + @Test public void testDeleteSqlBlacklist(@Mocked EditLog editLog) throws Exception { SqlBlackList sqlBlackList = new SqlBlackList(); @@ -120,27 +155,15 @@ public EditLog getEditLog() { } @Test - public void testRedirectBlacklistOperationIfNotLeader() { - MockUp mockUp = new MockUp() { - @Mock - public FrontendNodeType getFeType() { - return FrontendNodeType.FOLLOWER; - } - - @Mock - public boolean isLeader() { - return false; - } - }; - ConnectContext connectContext = UtFrameUtils.createDefaultCtx(); - - StatementBase addStatement = parseSql("ADD SQLBLACKLIST \".+\";"); - StmtExecutor addStatementExecutor = new StmtExecutor(connectContext, addStatement); - Assert.assertTrue(addStatementExecutor.isForwardToLeader()); - - StatementBase deleteStatement = parseSql("DELETE SQLBLACKLIST 1,2,3;"); - StmtExecutor deleteStatementExecutor = new StmtExecutor(connectContext, deleteStatement); - Assert.assertTrue(deleteStatementExecutor.isForwardToLeader()); + public void testRedirectStatus() { + Assert.assertEquals( + new AddSqlBlackListStmt("ADD SQLBLACKLIST \".+\";").getRedirectStatus(), + RedirectStatus.FORWARD_NO_SYNC + ); + Assert.assertEquals( + new DelSqlBlackListStmt(List.of(1L, 2L)).getRedirectStatus(), + RedirectStatus.FORWARD_NO_SYNC + ); } @Test @@ -164,8 +187,8 @@ public void testSqlBlacklistJournalOperations() throws Exception { // add blacklists - GlobalStateMgr.getCurrentState().getEditLog().logAddSQLBlackList(new AddSqlBlackList(123, "p1")); - GlobalStateMgr.getCurrentState().getEditLog().logAddSQLBlackList(new AddSqlBlackList(1234, "p2")); + GlobalStateMgr.getCurrentState().getEditLog().logAddSQLBlackList(new SqlBlackListPersistInfo(123, "p1")); + GlobalStateMgr.getCurrentState().getEditLog().logAddSQLBlackList(new SqlBlackListPersistInfo(1234, "p2")); UtFrameUtils.PseudoJournalReplayer.replayJournalToEnd(); Assertions.assertIterableEquals(