Repository: ambari Updated Branches: refs/heads/branch-2.1 e8fdf38b2 -> 88fdd26e5
AMBARI-11744. Ambari Server deadlocks when adding service / adding host. (swagle) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/88fdd26e Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/88fdd26e Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/88fdd26e Branch: refs/heads/branch-2.1 Commit: 88fdd26e5d7116f56bc52095c43d9001f153bcde Parents: e8fdf38 Author: Siddharth Wagle <swa...@hortonworks.com> Authored: Sat Jun 6 15:47:03 2015 -0700 Committer: Siddharth Wagle <swa...@hortonworks.com> Committed: Sat Jun 6 15:47:03 2015 -0700 ---------------------------------------------------------------------- .../internal/ConfigGroupResourceProvider.java | 4 +- .../state/configgroup/ConfigGroupImpl.java | 113 ++++++++++--------- 2 files changed, 61 insertions(+), 56 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/88fdd26e/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java index 68bad38..2bbb9e7 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java @@ -535,9 +535,7 @@ public class ConfigGroupResourceProvider extends return configGroupResponses; } - private synchronized void updateConfigGroups - (Set<ConfigGroupRequest> requests) throws AmbariException { - + private synchronized void updateConfigGroups (Set<ConfigGroupRequest> requests) throws AmbariException { if (requests.isEmpty()) { LOG.warn("Received an empty requests set"); return; http://git-wip-us.apache.org/repos/asf/ambari/blob/88fdd26e/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java index 656c452..6bab661 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java @@ -317,18 +317,23 @@ public class ConfigGroupImpl implements ConfigGroup { @Override public void persist() { - readWriteLock.writeLock().lock(); + cluster.getClusterGlobalLock().writeLock().lock(); try { - if (!isPersisted) { - persistEntities(); - refresh(); - cluster.refresh(); - isPersisted = true; - } else { - saveIfPersisted(); + readWriteLock.writeLock().lock(); + try { + if (!isPersisted) { + persistEntities(); + refresh(); + cluster.refresh(); + isPersisted = true; + } else { + saveIfPersisted(); + } + } finally { + readWriteLock.writeLock().unlock(); } } finally { - readWriteLock.writeLock().unlock(); + cluster.getClusterGlobalLock().writeLock().unlock(); } } @@ -420,19 +425,11 @@ public class ConfigGroupImpl implements ConfigGroup { clusterConfigEntity.setAttributes(gson.toJson(config.getPropertiesAttributes())); } clusterConfigEntity.setTimestamp(System.currentTimeMillis()); - - - // TODO: Is locking necessary and functional ? - cluster.getClusterGlobalLock().writeLock().lock(); - try { - clusterDAO.createConfig(clusterConfigEntity); - clusterEntity.getClusterConfigEntities().add(clusterConfigEntity); - cluster.addConfig(config); - clusterDAO.merge(clusterEntity); - cluster.refresh(); - } finally { - cluster.getClusterGlobalLock().writeLock().unlock(); - } + clusterDAO.createConfig(clusterConfigEntity); + clusterEntity.getClusterConfigEntities().add(clusterConfigEntity); + cluster.addConfig(config); + clusterDAO.merge(clusterEntity); + cluster.refresh(); } ConfigGroupConfigMappingEntity configMappingEntity = @@ -467,15 +464,20 @@ public class ConfigGroupImpl implements ConfigGroup { @Override @Transactional public void delete() { - readWriteLock.writeLock().lock(); + cluster.getClusterGlobalLock().writeLock().lock(); try { - configGroupConfigMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); - configGroupHostMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); - configGroupDAO.removeByPK(configGroupEntity.getGroupId()); - cluster.refresh(); - isPersisted = false; + readWriteLock.writeLock().lock(); + try { + configGroupConfigMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); + configGroupHostMappingDAO.removeAllByGroup(configGroupEntity.getGroupId()); + configGroupDAO.removeByPK(configGroupEntity.getGroupId()); + cluster.refresh(); + isPersisted = false; + } finally { + readWriteLock.writeLock().unlock(); + } } finally { - readWriteLock.writeLock().unlock(); + cluster.getClusterGlobalLock().writeLock().unlock(); } } @@ -523,35 +525,40 @@ public class ConfigGroupImpl implements ConfigGroup { @Override public ConfigGroupResponse convertToResponse() throws AmbariException { - readWriteLock.readLock().lock(); + cluster.getClusterGlobalLock().readLock().lock(); try { - Set<Map<String, Object>> hostnames = new HashSet<Map<String, Object>>(); - for (Host host : hosts.values()) { - Map<String, Object> hostMap = new HashMap<String, Object>(); - hostMap.put("host_name", host.getHostName()); - hostnames.add(hostMap); - } + readWriteLock.readLock().lock(); + try { + Set<Map<String, Object>> hostnames = new HashSet<Map<String, Object>>(); + for (Host host : hosts.values()) { + Map<String, Object> hostMap = new HashMap<String, Object>(); + hostMap.put("host_name", host.getHostName()); + hostnames.add(hostMap); + } - Set<Map<String, Object>> configObjMap = new HashSet<Map<String, - Object>>(); + Set<Map<String, Object>> configObjMap = new HashSet<Map<String, + Object>>(); - for (Config config : configurations.values()) { - Map<String, Object> configMap = new HashMap<String, Object>(); - configMap.put(ConfigurationResourceProvider - .CONFIGURATION_CONFIG_TYPE_PROPERTY_ID, config.getType()); - configMap.put(ConfigurationResourceProvider - .CONFIGURATION_CONFIG_TAG_PROPERTY_ID, config.getTag()); - configObjMap.add(configMap); - } + for (Config config : configurations.values()) { + Map<String, Object> configMap = new HashMap<String, Object>(); + configMap.put(ConfigurationResourceProvider + .CONFIGURATION_CONFIG_TYPE_PROPERTY_ID, config.getType()); + configMap.put(ConfigurationResourceProvider + .CONFIGURATION_CONFIG_TAG_PROPERTY_ID, config.getTag()); + configObjMap.add(configMap); + } - ConfigGroupResponse configGroupResponse = new ConfigGroupResponse( - configGroupEntity.getGroupId(), cluster.getClusterName(), - configGroupEntity.getGroupName(), configGroupEntity.getTag(), - configGroupEntity.getDescription(), - hostnames, configObjMap); - return configGroupResponse; + ConfigGroupResponse configGroupResponse = new ConfigGroupResponse( + configGroupEntity.getGroupId(), cluster.getClusterName(), + configGroupEntity.getGroupName(), configGroupEntity.getTag(), + configGroupEntity.getDescription(), + hostnames, configObjMap); + return configGroupResponse; + } finally { + readWriteLock.readLock().unlock(); + } } finally { - readWriteLock.readLock().unlock(); + cluster.getClusterGlobalLock().readLock().unlock(); } }