You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by sh...@apache.org on 2018/06/16 06:52:32 UTC

[kylin] 02/04: KYLIN-3400 wipeCache and createCubeDesc make deadlock

This is an automated email from the ASF dual-hosted git repository.

shaofengshi pushed a commit to branch v2.4.0-release
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit 111cb284d04ddb7574a4630ee6a97b94e63a02e1
Author: shaofengshi <sh...@apache.org>
AuthorDate: Sat Jun 16 14:10:53 2018 +0800

    KYLIN-3400 wipeCache and createCubeDesc make deadlock
---
 .../org/apache/kylin/cube/CubeDescManager.java     |  2 +-
 .../java/org/apache/kylin/cube/CubeManager.java    | 22 ++++---
 .../kylin/metadata/TableMetadataManager.java       | 22 +++----
 .../kylin/metadata/project/ProjectL2Cache.java     |  4 ++
 .../kylin/metadata/project/ProjectManager.java     | 68 ++++++++++++----------
 .../apache/kylin/rest/service/CacheService.java    |  2 +-
 6 files changed, 66 insertions(+), 54 deletions(-)

diff --git a/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java b/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
index 78c71b7..67c54f5 100644
--- a/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
+++ b/core-cube/src/main/java/org/apache/kylin/cube/CubeDescManager.java
@@ -152,7 +152,7 @@ public class CubeDescManager {
     }
     
     public CubeDesc reloadCubeDescQuietly(String name) {
-        try (AutoLock lock = descMapLock.lockForWrite()) {
+        try {
             return reloadCubeDescLocal(name);
         } catch (Exception e) {
             logger.error("Failed to reload CubeDesc " + name, e);
diff --git a/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java b/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java
index 3f4c576..fcec14e 100755
--- a/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java
+++ b/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java
@@ -32,7 +32,6 @@ import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
-import com.google.common.collect.Maps;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.kylin.common.KylinConfig;
 import org.apache.kylin.common.persistence.JsonSerializer;
@@ -82,6 +81,7 @@ import org.slf4j.LoggerFactory;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
 /**
@@ -205,18 +205,16 @@ public class CubeManager implements IRealizationProvider {
      * @return
      */
     public List<CubeInstance> getCubesByDesc(String descName) {
-        try (AutoLock lock = cubeMapLock.lockForRead()) {
-            List<CubeInstance> list = listAllCubes();
-            List<CubeInstance> result = new ArrayList<CubeInstance>();
-            Iterator<CubeInstance> it = list.iterator();
-            while (it.hasNext()) {
-                CubeInstance ci = it.next();
-                if (descName.equalsIgnoreCase(ci.getDescName())) {
-                    result.add(ci);
-                }
+        List<CubeInstance> list = listAllCubes();
+        List<CubeInstance> result = new ArrayList<CubeInstance>();
+        Iterator<CubeInstance> it = list.iterator();
+        while (it.hasNext()) {
+            CubeInstance ci = it.next();
+            if (descName.equalsIgnoreCase(ci.getDescName())) {
+                result.add(ci);
             }
-            return result;
         }
+        return result;
     }
 
     public CubeInstance createCube(String cubeName, String projectName, CubeDesc desc, String owner)
@@ -401,7 +399,7 @@ public class CubeManager implements IRealizationProvider {
         }
 
         //this is a duplicate call to take care of scenarios where REST cache service unavailable
-        ProjectManager.getInstance(cube.getConfig()).clearL2Cache();
+        ProjectManager.getInstance(config).clearL2Cache(cube.getProject());
 
         return cube;
     }
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/TableMetadataManager.java b/core-metadata/src/main/java/org/apache/kylin/metadata/TableMetadataManager.java
index 116e210..19adcb9 100644
--- a/core-metadata/src/main/java/org/apache/kylin/metadata/TableMetadataManager.java
+++ b/core-metadata/src/main/java/org/apache/kylin/metadata/TableMetadataManager.java
@@ -158,7 +158,7 @@ public class TableMetadataManager {
     }
 
     public List<TableDesc> listAllTables(String prj) {
-        try (AutoLock lock = srcTableMapLock.lockForWrite()) {
+        try (AutoLock lock = srcTableMapLock.lockForRead()) {
             return Lists.newArrayList(getAllTablesMap(prj).values());
         }
     }
@@ -166,22 +166,24 @@ public class TableMetadataManager {
     public Map<String, TableDesc> getAllTablesMap(String prj) {
         // avoid cyclic locks
         ProjectInstance project = (prj == null) ? null : ProjectManager.getInstance(config).getProject(prj);
-        
-        try (AutoLock lock = srcTableMapLock.lockForWrite()) {
-            //TODO prj == null case is now only used by test case and CubeMetaIngester
-            //should refactor these test case and tool ASAP and stop supporting null case
-            if (prj == null) {
-                Map<String, TableDesc> globalTables = new LinkedHashMap<>();
 
+        //TODO prj == null case is now only used by test case and CubeMetaIngester
+        //should refactor these test case and tool ASAP and stop supporting null case
+        if (prj == null) {
+            Map<String, TableDesc> globalTables = new LinkedHashMap<>();
+
+            try (AutoLock lock = srcTableMapLock.lockForRead()) {
                 for (TableDesc t : srcTableMap.values()) {
                     globalTables.put(t.getIdentity(), t);
                 }
-                return globalTables;
             }
+            return globalTables;
+        }
 
-            Set<String> prjTableNames = project.getTables();
+        Set<String> prjTableNames = project.getTables();
 
-            Map<String, TableDesc> ret = new LinkedHashMap<>();
+        Map<String, TableDesc> ret = new LinkedHashMap<>();
+        try (AutoLock lock = srcTableMapLock.lockForWrite()) {
             for (String tableName : prjTableNames) {
                 String tableIdentity = getTableIdentity(tableName);
                 ret.put(tableIdentity, getProjectSpecificTableDesc(tableIdentity, prj));
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/project/ProjectL2Cache.java b/core-metadata/src/main/java/org/apache/kylin/metadata/project/ProjectL2Cache.java
index 1663c8d..cb31107 100644
--- a/core-metadata/src/main/java/org/apache/kylin/metadata/project/ProjectL2Cache.java
+++ b/core-metadata/src/main/java/org/apache/kylin/metadata/project/ProjectL2Cache.java
@@ -63,6 +63,10 @@ class ProjectL2Cache {
         projectCaches.clear();
     }
 
+    public void clear(String projectName) {
+        projectCaches.remove(projectName);
+    }
+
     public ExternalFilterDesc getExternalFilterDesc(String project, String extFilterName) {
         ProjectCache prjCache = getCache(project);
         return prjCache.extFilters.get(extFilterName);
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/project/ProjectManager.java b/core-metadata/src/main/java/org/apache/kylin/metadata/project/ProjectManager.java
index aae692d..545875c 100644
--- a/core-metadata/src/main/java/org/apache/kylin/metadata/project/ProjectManager.java
+++ b/core-metadata/src/main/java/org/apache/kylin/metadata/project/ProjectManager.java
@@ -113,10 +113,16 @@ public class ProjectManager {
         }
     }
 
+    public void clearL2Cache(String projectName) {
+        l2Cache.clear(projectName);
+    }
+
+
     public void clearL2Cache() {
         l2Cache.clear();
     }
 
+    @Deprecated
     public void reloadProjectL2Cache(String project) {
         l2Cache.reloadCacheByProject(project);
     }
@@ -124,7 +130,7 @@ public class ProjectManager {
     public ProjectInstance reloadProjectQuietly(String project) throws IOException {
         try (AutoLock lock = prjMapLock.lockForWrite()) {
             ProjectInstance prj = crud.reloadQuietly(project);
-            reloadProjectL2Cache(project);
+            clearL2Cache(project);
             return prj;
         }
     }
@@ -157,17 +163,17 @@ public class ProjectManager {
 
     public ProjectInstance createProject(String projectName, String owner, String description,
             LinkedHashMap<String, String> overrideProps) throws IOException {
-        try (AutoLock lock = prjMapLock.lockForWrite()) {
-            logger.info("Creating project " + projectName);
 
-            ProjectInstance currentProject = getProject(projectName);
-            if (currentProject == null) {
-                currentProject = ProjectInstance.create(projectName, owner, description, overrideProps, null, null);
-            } else {
-                throw new IllegalStateException("The project named " + projectName + "already exists");
-            }
-            checkOverrideProps(currentProject);
+        ProjectInstance currentProject = getProject(projectName);
+        if (currentProject != null) {
+            throw new IllegalStateException("The project named " + projectName + "already exists");
+        }
+
+        currentProject = ProjectInstance.create(projectName, owner, description, overrideProps, null, null);
+        checkOverrideProps(currentProject);
 
+        try (AutoLock lock = prjMapLock.lockForWrite()) {
+            logger.info("Creating project " + projectName);
             return save(currentProject);
         }
     }
@@ -189,27 +195,27 @@ public class ProjectManager {
     }
 
     public ProjectInstance dropProject(String projectName) throws IOException {
-        try (AutoLock lock = prjMapLock.lockForWrite()) {
-            if (projectName == null)
-                throw new IllegalArgumentException("Project name not given");
+        if (projectName == null)
+            throw new IllegalArgumentException("Project name not given");
 
-            ProjectInstance projectInstance = getProject(projectName);
+        ProjectInstance projectInstance = getProject(projectName);
 
-            if (projectInstance == null) {
-                throw new IllegalStateException("The project named " + projectName + " does not exist");
-            }
+        if (projectInstance == null) {
+            throw new IllegalStateException("The project named " + projectName + " does not exist");
+        }
 
-            if (projectInstance.getRealizationCount(null) != 0) {
-                throw new IllegalStateException("The project named " + projectName
-                        + " can not be deleted because there's still realizations in it. Delete them first.");
-            }
+        if (projectInstance.getRealizationCount(null) != 0) {
+            throw new IllegalStateException("The project named " + projectName
+                    + " can not be deleted because there's still realizations in it. Delete them first.");
+        }
 
+        try (AutoLock lock = prjMapLock.lockForWrite()) {
             logger.info("Dropping project '" + projectInstance.getName() + "'");
 
             crud.delete(projectInstance);
             BadQueryHistoryManager.getInstance(config).removeBadQueryHistory(projectName);
 
-            clearL2Cache();
+            clearL2Cache(projectInstance.getName());
             return projectInstance;
         }
     }
@@ -217,8 +223,8 @@ public class ProjectManager {
     // update project itself
     public ProjectInstance updateProject(ProjectInstance project, String newName, String newDesc,
             LinkedHashMap<String, String> overrideProps) throws IOException {
+        Preconditions.checkArgument(project.getName().equals(newName));
         try (AutoLock lock = prjMapLock.lockForWrite()) {
-            Preconditions.checkArgument(project.getName().equals(newName));
             project.setName(newName);
             project.setDescription(newDesc);
             project.setOverrideKylinProps(overrideProps);
@@ -233,7 +239,7 @@ public class ProjectManager {
     public void removeProjectLocal(String proj) {
         try (AutoLock lock = prjMapLock.lockForWrite()) {
             projectMap.removeLocal(proj);
-            clearL2Cache();
+            clearL2Cache(proj);
         }
     }
 
@@ -252,8 +258,9 @@ public class ProjectManager {
     }
 
     public void removeModelFromProjects(String modelName) throws IOException {
+        final List<ProjectInstance> projects = findProjectsByModel(modelName);
         try (AutoLock lock = prjMapLock.lockForWrite()) {
-            for (ProjectInstance projectInstance : findProjectsByModel(modelName)) {
+            for (ProjectInstance projectInstance : projects) {
                 projectInstance.removeModel(modelName);
                 save(projectInstance);
             }
@@ -287,8 +294,9 @@ public class ProjectManager {
     }
 
     public void removeRealizationsFromProjects(RealizationType type, String realizationName) throws IOException {
+        final List<ProjectInstance> projects = findProjects(type, realizationName);
         try (AutoLock lock = prjMapLock.lockForWrite()) {
-            for (ProjectInstance projectInstance : findProjects(type, realizationName)) {
+            for (ProjectInstance projectInstance : projects) {
                 projectInstance.removeRealization(type, realizationName);
                 save(projectInstance);
             }
@@ -358,7 +366,7 @@ public class ProjectManager {
     
     private ProjectInstance save(ProjectInstance prj) throws IOException {
         crud.save(prj);
-        clearL2Cache();
+        clearL2Cache(prj.getName());
         return prj;
     }
 
@@ -373,7 +381,7 @@ public class ProjectManager {
     }
 
     public List<ProjectInstance> findProjects(RealizationType type, String realizationName) {
-        try (AutoLock lock = prjMapLock.lockForWrite()) {
+        try (AutoLock lock = prjMapLock.lockForRead()) {
             List<ProjectInstance> result = Lists.newArrayList();
             for (ProjectInstance prj : projectMap.values()) {
                 for (RealizationEntry entry : prj.getRealizationEntries()) {
@@ -388,7 +396,7 @@ public class ProjectManager {
     }
 
     public List<ProjectInstance> findProjectsByModel(String modelName) {
-        try (AutoLock lock = prjMapLock.lockForWrite()) {
+        try (AutoLock lock = prjMapLock.lockForRead()) {
             List<ProjectInstance> projects = new ArrayList<ProjectInstance>();
             for (ProjectInstance projectInstance : projectMap.values()) {
                 if (projectInstance.containsModel(modelName)) {
@@ -400,7 +408,7 @@ public class ProjectManager {
     }
 
     public List<ProjectInstance> findProjectsByTable(String tableIdentity) {
-        try (AutoLock lock = prjMapLock.lockForWrite()) {
+        try (AutoLock lock = prjMapLock.lockForRead()) {
             List<ProjectInstance> projects = new ArrayList<ProjectInstance>();
             for (ProjectInstance projectInstance : projectMap.values()) {
                 if (projectInstance.containsTable(tableIdentity)) {
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/CacheService.java b/server-base/src/main/java/org/apache/kylin/rest/service/CacheService.java
index 10ab90b..4fc0913 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/CacheService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/CacheService.java
@@ -161,7 +161,7 @@ public class CacheService extends BasicService implements InitializingBean {
         logger.info("reload cube cache done");
 
         //reload project l2cache again after cube cache, because the project L2 cache relay on latest cube cache
-        getProjectManager().reloadProjectL2Cache(project);
+        getProjectManager().clearL2Cache(project);
         logger.info("reload project l2cache done");
     }
 

-- 
To stop receiving notification emails like this one, please contact
shaofengshi@apache.org.