[GitHub] [doris] englefly commented on a diff in pull request #18901: [feat](stats) Support to kill analyze

["englefly (via GitHub)" <gi...@apache.org>]

englefly commented on code in PR #18901:
URL: https://github.com/apache/doris/pull/18901#discussion_r1177359381


##########
fe/fe-core/src/main/cup/sql_parser.cup:
##########
@@ -4753,6 +4755,13 @@ kill_stmt ::=
     :}
     ;
 
+kill_analysis_job_stmt ::=
+    KW_KILL KW_ANALYZE INTEGER_LITERAL:value

Review Comment:
   after executing `analyze table XXX`, how can I get the INTEGER_LETEAL(job_id)?



##########
fe/fe-core/src/main/java/org/apache/doris/statistics/AnalysisManager.java:
##########
@@ -329,4 +331,91 @@ public void dropStats(DropStatsStmt dropStatsStmt) throws DdlException {
         }
     }
 
+    public void handleKillAnalyzeStmt(KillAnalysisJobStmt killAnalysisJobStmt) throws DdlException {
+        Map<Long, BaseAnalysisTask> analysisTaskInfoMap = analysisJobIdToTaskMap.remove(killAnalysisJobStmt.jobId);
+        if (analysisTaskInfoMap == null) {
+            throw new DdlException("Job not exists or already finished");
+        }
+        BaseAnalysisTask anyTask = analysisTaskInfoMap.values().stream().findFirst().orElse(null);
+        if (anyTask == null) {
+            return;
+        }
+        checkPriv(anyTask);
+        for (BaseAnalysisTask taskInfo : analysisTaskInfoMap.values()) {
+            taskInfo.markAsKilled();
+        }
+        Map<String, String> params = new HashMap<>();
+        params.put("jobState", AnalysisState.FAILED.toString());
+        params.put("message", ", message = 'Killed by user : " + ConnectContext.get().getQualifiedUser() + "'");
+        params.put("updateExecTime", ", last_exec_time_in_ms=" + String.valueOf(System.currentTimeMillis()));
+        params.put("jobId", String.valueOf(killAnalysisJobStmt.jobId));
+        params.put("taskId", "'-1'");
+        params.put("isAllTask", "true");
+        try {
+            StatisticsUtil.execUpdate(new StringSubstitutor(params).replace(UPDATE_JOB_STATE_SQL_TEMPLATE));
+        } catch (Exception e) {
+            LOG.warn("Failed to update status", e);
+        }
+    }
+
+    private void checkPriv(BaseAnalysisTask analysisTask) {
+        String dbName = analysisTask.db.getFullName();
+        String tblName = analysisTask.tbl.getName();
+        if (!Env.getCurrentEnv().getAccessManager()
+                .checkTblPriv(ConnectContext.get(), dbName, tblName, PrivPredicate.SELECT)) {
+            throw new RuntimeException("You need at least SELECT PRIV to corresponding table to kill this analyze"

Review Comment:
   this message is not clear enough. 
   In my understanding, it means user does not have privilege to run this command.
   1. SELECT PRIV means read privilege, but here we need write privilege. why SELECT privilege is enough?
   2. `SELECT PRIV` is more like a internal term, but not user friendly. how about 'select privilege'



##########
fe/fe-core/src/main/java/org/apache/doris/statistics/AnalysisManager.java:
##########
@@ -76,12 +79,14 @@ public class AnalysisManager {
     // The time field that needs to be displayed
     private static final String LAST_EXEC_TIME_IN_MS = "last_exec_time_in_ms";
 
-    private final ConcurrentMap<Long, Map<Long, AnalysisTaskInfo>> analysisJobIdToTaskMap;
+    private final ConcurrentMap<Long, Map<Long, BaseAnalysisTask>> analysisJobIdToTaskMap;

Review Comment:
   what is the meaning of the inner map's key? (the outer map key means job id)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org