From c9f2ac2de9565d9126d5320b84209f5b8c643aa1 Mon Sep 17 00:00:00 2001
From: zhangdong <493738387@qq.com>
Date: Fri, 6 Sep 2024 11:34:23 +0800
Subject: [PATCH] [enhance](mtmv) During cache generation, no longer hold the
 write lock for mtmv (#40402)

During cache generation, no longer hold the write Lock for mv to avoid
changes in the logic of cache generation in the future, internal calls
to other locks, and deadlocks
---
 .../java/org/apache/doris/catalog/MTMV.java   | 46 ++++++++++++-------
 1 file changed, 30 insertions(+), 16 deletions(-)

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 e0beca8a1626f2..b4da3b3ab7e5b4 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
@@ -182,6 +182,18 @@ public MTMVStatus alterStatus(MTMVStatus newStatus) {
 
     public void addTaskResult(MTMVTask task, MTMVRelation relation,
             Map<String, MTMVRefreshPartitionSnapshot> partitionSnapshots) {
+        MTMVCache mtmvCache = null;
+        boolean needUpdateCache = false;
+        if (task.getStatus() == TaskStatus.SUCCESS && !Env.isCheckpointThread()) {
+            needUpdateCache = true;
+            try {
+                // shouldn't do this while holding mvWriteLock
+                mtmvCache = MTMVCache.from(this, MTMVPlanUtil.createMTMVContext(this), true);
+            } catch (Throwable e) {
+                mtmvCache = null;
+                LOG.warn("generate cache failed", e);
+            }
+        }
         writeMvLock();
         try {
             if (task.getStatus() == TaskStatus.SUCCESS) {
@@ -189,13 +201,8 @@ public void addTaskResult(MTMVTask task, MTMVRelation relation,
                 this.status.setSchemaChangeDetail(null);
                 this.status.setRefreshState(MTMVRefreshState.SUCCESS);
                 this.relation = relation;
-                if (!Env.isCheckpointThread()) {
-                    try {
-                        this.cache = MTMVCache.from(this, MTMVPlanUtil.createMTMVContext(this), true);
-                    } catch (Throwable e) {
-                        this.cache = null;
-                        LOG.warn("generate cache failed", e);
-                    }
+                if (needUpdateCache) {
+                    this.cache = mtmvCache;
                 }
             } else {
                 this.status.setRefreshState(MTMVRefreshState.FAIL);
@@ -274,17 +281,24 @@ public Set<String> getExcludedTriggerTables() {
      * Called when in query, Should use one connection context in query
      */
     public MTMVCache getOrGenerateCache(ConnectContext connectionContext) throws AnalysisException {
-        if (cache == null) {
-            writeMvLock();
-            try {
-                if (cache == null) {
-                    this.cache = MTMVCache.from(this, connectionContext, true);
-                }
-            } finally {
-                writeMvUnlock();
+        readMvLock();
+        try {
+            if (cache != null) {
+                return cache;
             }
+        } finally {
+            readMvUnlock();
+        }
+        // Concurrent situations may result in duplicate cache generation,
+        // but we tolerate this in order to prevent nested use of readLock and write MvLock for the table
+        MTMVCache mtmvCache = MTMVCache.from(this, connectionContext, true);
+        writeMvLock();
+        try {
+            this.cache = mtmvCache;
+            return cache;
+        } finally {
+            writeMvUnlock();
         }
-        return cache;
     }
 
     public Map<String, String> getMvProperties() {