morningman commented on code in PR #21054:
URL: https://github.com/apache/doris/pull/21054#discussion_r1237179417


##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVJobManager.java:
##########
@@ -470,56 +317,75 @@ public void replayDropJobs(List<Long> jobIds) {
     }
 
     public void replayUpdateJob(ChangeMTMVJob changeJob) {
-        updateJob(changeJob, true);
+        MTMVJob mtmvJob = idToJobMap.get(changeJob.getJobId());
+        if (mtmvJob != null) {
+            mtmvJob.updateJob(changeJob, true);
+        }
     }
 
     public void replayCreateJobTask(MTMVTask task) {
         taskManager.replayCreateJobTask(task);
     }
 
     public void replayDropJobTasks(List<String> taskIds) {
-        taskManager.dropTasks(taskIds, true);
+        taskManager.dropHistoryTasks(taskIds, true);
     }
 
     public void removeExpiredJobs() {
         long currentTimeSeconds = MTMVUtils.getNowTimeStamp();
-
         List<Long> jobIdsToDelete = Lists.newArrayList();
-        if (!tryLock()) {
-            return;
-        }
-        try {
-            List<MTMVJob> jobs = showJobs(null);
-            for (MTMVJob job : jobs) {
-                // active job should not clean
-                if (job.getState() == MTMVUtils.JobState.ACTIVE) {
-                    continue;
-                }
-                if (job.getTriggerMode() == MTMVUtils.TriggerMode.PERIODICAL) {
-                    JobSchedule jobSchedule = job.getSchedule();
-                    if (jobSchedule == null) {
-                        jobIdsToDelete.add(job.getId());
-                        LOG.warn("clean up a null schedule periodical Task 
[{}]", job.getName());
-                        continue;
-                    }
-
-                }
-                long expireTime = job.getExpireTime();
-                if (expireTime > 0 && currentTimeSeconds > expireTime) {
-                    jobIdsToDelete.add(job.getId());
-                }
+        List<MTMVJob> jobs = getAllJobsWithLock();
+        for (MTMVJob job : jobs) {
+            // active job should not clean
+            if (job.getState() == MTMVUtils.JobState.ACTIVE) {

Review Comment:
   I think only `COMPLETE` job can be removed.



##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVJobManager.java:
##########
@@ -470,56 +317,75 @@ public void replayDropJobs(List<Long> jobIds) {
     }
 
     public void replayUpdateJob(ChangeMTMVJob changeJob) {
-        updateJob(changeJob, true);
+        MTMVJob mtmvJob = idToJobMap.get(changeJob.getJobId());
+        if (mtmvJob != null) {
+            mtmvJob.updateJob(changeJob, true);
+        }
     }
 
     public void replayCreateJobTask(MTMVTask task) {
         taskManager.replayCreateJobTask(task);
     }
 
     public void replayDropJobTasks(List<String> taskIds) {
-        taskManager.dropTasks(taskIds, true);
+        taskManager.dropHistoryTasks(taskIds, true);
     }
 
     public void removeExpiredJobs() {

Review Comment:
   change to private.
   And check other method, if it only be used inside this class, change to 
private



-- 
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-unsubscr...@doris.apache.org

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


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

Reply via email to