From 5bd8081e92e355cf4080e49454fd8b225d1bacee Mon Sep 17 00:00:00 2001
From: zhangdong <493738387@qq.com>
Date: Wed, 25 Sep 2024 10:38:31 +0800
Subject: [PATCH] [enhance](mtmv)Optimize the logic of mtmv lock (#41010)

- When deleting a job, do not query and delete it first. Instead, call
the method to delete the job directly. If the job does not exist(When
the materialized view only creates the table and the job is not yet
created, the materialized view is concurrently deleted), throw an
exception
- When changing mtmv, narrow down the scope of the lock and place it in
each sub method
---
 .../java/org/apache/doris/alter/Alter.java    |  8 --------
 .../java/org/apache/doris/catalog/MTMV.java   |  2 ++
 .../org/apache/doris/mtmv/MTMVJobManager.java | 20 +++++++++----------
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
index 51ed811cc06336..7bfb3e3747ca91 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
@@ -968,8 +968,6 @@ public void processAlterMTMV(AlterMTMV alterMTMV, boolean isReplay) {
         try {
             Database db = Env.getCurrentInternalCatalog().getDbOrDdlException(tbl.getDb());
             mtmv = (MTMV) db.getTableOrMetaException(tbl.getTbl(), TableType.MATERIALIZED_VIEW);
-
-            mtmv.writeMvLock();
             switch (alterMTMV.getOpType()) {
                 case ALTER_REFRESH_INFO:
                     mtmv.alterRefreshInfo(alterMTMV.getRefreshInfo());
@@ -982,8 +980,6 @@ public void processAlterMTMV(AlterMTMV alterMTMV, boolean isReplay) {
                     break;
                 case ADD_TASK:
                     mtmv.addTaskResult(alterMTMV.getTask(), alterMTMV.getRelation(), alterMTMV.getPartitionSnapshots());
-                    Env.getCurrentEnv().getMtmvService()
-                            .refreshComplete(mtmv, alterMTMV.getRelation(), alterMTMV.getTask());
                     break;
                 default:
                     throw new RuntimeException("Unknown type value: " + alterMTMV.getOpType());
@@ -996,10 +992,6 @@ public void processAlterMTMV(AlterMTMV alterMTMV, boolean isReplay) {
         } catch (UserException e) {
             // if MTMV has been dropped, ignore this exception
             LOG.warn(e);
-        } finally {
-            if (mtmv != null) {
-                mtmv.writeMvUnlock();
-            }
         }
     }
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java
index c3d36ea39714c0..988f6646339db6 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java
@@ -196,6 +196,8 @@ public void addTaskResult(MTMVTask task, MTMVRelation relation,
             }
             this.jobInfo.addHistoryTask(task);
             this.refreshSnapshot.updateSnapshots(partitionSnapshots, getPartitionNames());
+            Env.getCurrentEnv().getMtmvService()
+                    .refreshComplete(this, relation, task);
         } finally {
             writeMvUnlock();
         }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVJobManager.java b/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVJobManager.java
index 11089899b309a8..0312cd33829e54 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVJobManager.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVJobManager.java
@@ -48,6 +48,8 @@
 import com.google.common.collect.Lists;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.util.List;
 
@@ -55,6 +57,8 @@
  * when do some operation, do something about job
  */
 public class MTMVJobManager implements MTMVHookService {
+    private static final Logger LOG = LogManager.getLogger(MTMVJobManager.class);
+
     public static final String MTMV_JOB_PREFIX = "inner_mtmv_";
 
     /**
@@ -124,16 +128,12 @@ private void setScheduleJobConfig(JobExecutionConfiguration jobExecutionConfigur
      */
     @Override
     public void dropMTMV(MTMV mtmv) throws DdlException {
-        List<MTMVJob> jobs = Env.getCurrentEnv().getJobManager()
-                .queryJobs(JobType.MV, mtmv.getJobInfo().getJobName());
-        if (!CollectionUtils.isEmpty(jobs)) {
-            try {
-                Env.getCurrentEnv().getJobManager()
-                        .unregisterJob(jobs.get(0).getJobId());
-            } catch (JobException e) {
-                e.printStackTrace();
-                throw new DdlException(e.getMessage());
-            }
+        try {
+            Env.getCurrentEnv().getJobManager()
+                    .unregisterJob(mtmv.getJobInfo().getJobName(), false);
+        } catch (JobException e) {
+            LOG.warn("drop mtmv job failed, mtmvName: {}", mtmv.getName(), e);
+            throw new DdlException(e.getMessage());
         }
     }