You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ma...@apache.org on 2013/02/15 23:55:21 UTC

svn commit: r1446799 - in /incubator/ambari/trunk: ./ ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ ambari-server/src/main/java/org/apache/ambari/server/state/host/

Author: mahadev
Date: Fri Feb 15 22:55:21 2013
New Revision: 1446799

URL: http://svn.apache.org/r1446799
Log:
AMBARI-1436. Threads blocking on ClustersImpl.getHost for several minutes. (Sid Wagle via mahadev)

Modified:
    incubator/ambari/trunk/CHANGES.txt
    incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
    incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
    incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java

Modified: incubator/ambari/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/incubator/ambari/trunk/CHANGES.txt?rev=1446799&r1=1446798&r2=1446799&view=diff
==============================================================================
--- incubator/ambari/trunk/CHANGES.txt (original)
+++ incubator/ambari/trunk/CHANGES.txt Fri Feb 15 22:55:21 2013
@@ -286,6 +286,9 @@ Trunk (unreleased changes):
  AMBARI-1435. L2 Cache does not work due to Eclipse Link exception.
  (Sid Wagle via mahadev)
 
+ AMBARI-1436. Threads blocking on ClustersImpl.getHost for several minutes.
+ (Sid Wagle via mahadev)
+
  BUG FIXES
 
  AMBARI-1431. Hosts table no longer allows sorting. (yusaku)

Modified: incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
URL: http://svn.apache.org/viewvc/incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java?rev=1446799&r1=1446798&r2=1446799&view=diff
==============================================================================
--- incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java (original)
+++ incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java Fri Feb 15 22:55:21 2013
@@ -138,54 +138,62 @@ public class ClusterImpl implements Clus
    * Make sure we load all the service host components.
    * We need this for live status checks.
    */
-  public synchronized void loadServiceHostComponents() {
+  public void loadServiceHostComponents() {
     loadServices();
-    LOG.info("Loading Service Host Components");
     if (svcHostsLoaded) return;
-    if (services != null) {
-      for (Map.Entry<String, Service> serviceKV: services.entrySet()) {
-        /* get all the service component hosts **/
-        Service service = serviceKV.getValue();
-        if (!serviceComponentHosts.containsKey(service.getName())) {
-          serviceComponentHosts.put(service.getName(), new HashMap<String, 
-              Map<String, ServiceComponentHost>>());
-        }
-        for (Map.Entry<String, ServiceComponent> svcComponent:
-            service.getServiceComponents().entrySet()) {
-          ServiceComponent comp = svcComponent.getValue();
-          String componentName = svcComponent.getKey();
-          if (!serviceComponentHosts.get(service.getName()).containsKey(componentName)) {
-            serviceComponentHosts.get(service.getName()).put(componentName, 
-                new HashMap<String, ServiceComponentHost>());
+    writeLock.lock();
+    try {
+      if (svcHostsLoaded) return;
+      LOG.info("Loading Service Host Components");
+      if (svcHostsLoaded) return;
+      if (services != null) {
+        for (Entry<String, Service> serviceKV: services.entrySet()) {
+          /* get all the service component hosts **/
+          Service service = serviceKV.getValue();
+          if (!serviceComponentHosts.containsKey(service.getName())) {
+            serviceComponentHosts.put(service.getName(), new HashMap<String,
+                Map<String, ServiceComponentHost>>());
+          }
+          for (Entry<String, ServiceComponent> svcComponent:
+              service.getServiceComponents().entrySet()) {
+            ServiceComponent comp = svcComponent.getValue();
+            String componentName = svcComponent.getKey();
+            if (!serviceComponentHosts.get(service.getName()).containsKey(componentName)) {
+              serviceComponentHosts.get(service.getName()).put(componentName,
+                  new HashMap<String, ServiceComponentHost>());
+            }
+            /** Get Service Host Components **/
+            for (Entry<String, ServiceComponentHost> svchost:
+                comp.getServiceComponentHosts().entrySet()) {
+                String hostname = svchost.getKey();
+                ServiceComponentHost svcHostComponent = svchost.getValue();
+                if (!serviceComponentHostsByHost.containsKey(hostname)) {
+                  serviceComponentHostsByHost.put(hostname,
+                      new ArrayList<ServiceComponentHost>());
+                }
+                List<ServiceComponentHost> compList =  serviceComponentHostsByHost.get(hostname);
+                compList.add(svcHostComponent);
+
+                if (!serviceComponentHosts.get(service.getName()).get(componentName)
+                    .containsKey(hostname)) {
+                  serviceComponentHosts.get(service.getName()).get(componentName)
+                  .put(hostname, svcHostComponent);
+                }
+            }
           }
-          /** Get Service Host Components **/
-          for (Map.Entry<String, ServiceComponentHost> svchost:
-              comp.getServiceComponentHosts().entrySet()) {
-              String hostname = svchost.getKey();
-              ServiceComponentHost svcHostComponent = svchost.getValue();
-              if (!serviceComponentHostsByHost.containsKey(hostname)) {
-                serviceComponentHostsByHost.put(hostname, 
-                    new ArrayList<ServiceComponentHost>());
-              }
-              List<ServiceComponentHost> compList =  serviceComponentHostsByHost.get(hostname);
-              compList.add(svcHostComponent);
-              
-              if (!serviceComponentHosts.get(service.getName()).get(componentName)
-                  .containsKey(hostname)) {
-                serviceComponentHosts.get(service.getName()).get(componentName)
-                .put(hostname, svcHostComponent);
-              }
-          }       
         }
       }
+      svcHostsLoaded = true;
+    } finally {
+      writeLock.unlock();
     }
-    svcHostsLoaded = true;
   }
 
   private void loadServices() {
     LOG.info("clusterEntity " + clusterEntity.getClusterServiceEntities() );
     if (services == null) {
-      synchronized (this) {
+      writeLock.lock();
+      try {
         if (services == null) {
           services = new TreeMap<String, Service>();
           if (!clusterEntity.getClusterServiceEntities().isEmpty()) {
@@ -194,6 +202,8 @@ public class ClusterImpl implements Clus
             }
           }
         }
+      } finally {
+        writeLock.unlock();
       }
     }
   }
@@ -201,22 +211,27 @@ public class ClusterImpl implements Clus
   public ServiceComponentHost getServiceComponentHost(String serviceName,
       String serviceComponentName, String hostname) throws AmbariException {
     loadServiceHostComponents();
-    if (!serviceComponentHosts.containsKey(serviceName)
-        || !serviceComponentHosts.get(serviceName)
-            .containsKey(serviceComponentName)
-        || !serviceComponentHosts.get(serviceName).get(serviceComponentName)
-            .containsKey(hostname)) {
-      throw new ServiceComponentHostNotFoundException(getClusterName(), serviceName,
-          serviceComponentName, hostname);
+    readLock.lock();
+    try {
+      if (!serviceComponentHosts.containsKey(serviceName)
+          || !serviceComponentHosts.get(serviceName)
+              .containsKey(serviceComponentName)
+          || !serviceComponentHosts.get(serviceName).get(serviceComponentName)
+              .containsKey(hostname)) {
+        throw new ServiceComponentHostNotFoundException(getClusterName(), serviceName,
+            serviceComponentName, hostname);
+      }
+      return serviceComponentHosts.get(serviceName).get(serviceComponentName)
+          .get(hostname);
+    } finally {
+      readLock.unlock();
     }
-    return serviceComponentHosts.get(serviceName).get(serviceComponentName)
-        .get(hostname);
   }
 
   @Override
   public String getClusterName() {
+    readLock.lock();
     try {
-      readLock.lock();
       return clusterEntity.getClusterName();
     } finally {
       readLock.unlock();
@@ -225,8 +240,8 @@ public class ClusterImpl implements Clus
 
   @Override
   public void setClusterName(String clusterName) {
+    writeLock.lock();
     try {
-      writeLock.lock();
       String oldName = clusterEntity.getClusterName();
       clusterEntity.setClusterName(clusterName);
       clusterDAO.merge(clusterEntity); //RollbackException possibility if UNIQUE constraint violated
@@ -236,35 +251,37 @@ public class ClusterImpl implements Clus
     }
   }
 
-  public synchronized void addServiceComponentHost(
+  public void addServiceComponentHost(
       ServiceComponentHost svcCompHost) throws AmbariException {
     loadServiceHostComponents();
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Trying to add ServiceComponentHost to ClusterHostMap cache"
-          + ", serviceName=" + svcCompHost.getServiceName()
-          + ", componentName=" + svcCompHost.getServiceComponentName()
-          + ", hostname=" + svcCompHost.getHostName());
-    }
-
-    final String hostname = svcCompHost.getHostName();
-    final String serviceName = svcCompHost.getServiceName();
-    final String componentName = svcCompHost.getServiceComponentName();
-    Set<Cluster> cs = clusters.getClustersForHost(hostname);
-    boolean clusterFound = false;
-    Iterator<Cluster> iter = cs.iterator();
-    while (iter.hasNext()) {
-      Cluster c = iter.next();
-      if (c.getClusterId() == this.getClusterId()) {
-        clusterFound = true;
-        break;
-      }
-    }
-    if (!clusterFound) {
-      throw new AmbariException("Host does not belong this cluster"
-              + ", hostname=" + hostname
-              + ", clusterName=" + getClusterName()
-              + ", clusterId=" + getClusterId());
-    }
+    writeLock.lock();
+    try {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Trying to add ServiceComponentHost to ClusterHostMap cache"
+            + ", serviceName=" + svcCompHost.getServiceName()
+            + ", componentName=" + svcCompHost.getServiceComponentName()
+            + ", hostname=" + svcCompHost.getHostName());
+      }
+
+      final String hostname = svcCompHost.getHostName();
+      final String serviceName = svcCompHost.getServiceName();
+      final String componentName = svcCompHost.getServiceComponentName();
+      Set<Cluster> cs = clusters.getClustersForHost(hostname);
+      boolean clusterFound = false;
+      Iterator<Cluster> iter = cs.iterator();
+      while (iter.hasNext()) {
+        Cluster c = iter.next();
+        if (c.getClusterId() == this.getClusterId()) {
+          clusterFound = true;
+          break;
+        }
+      }
+      if (!clusterFound) {
+        throw new AmbariException("Host does not belong this cluster"
+                + ", hostname=" + hostname
+                + ", clusterName=" + getClusterName()
+                + ", clusterId=" + getClusterId());
+      }
 
     if (!serviceComponentHosts.containsKey(serviceName)) {
       serviceComponentHosts.put(serviceName,
@@ -275,278 +292,400 @@ public class ClusterImpl implements Clus
           new HashMap<String, ServiceComponentHost>());
     }
 
-    if (serviceComponentHosts.get(serviceName).get(componentName).
-        containsKey(hostname)) {
-      throw new AmbariException("Duplicate entry for ServiceComponentHost"
-          + ", serviceName=" + serviceName
-          + ", serviceComponentName" + componentName
-          + ", hostname= " + hostname);
-    }
+      if (serviceComponentHosts.get(serviceName).get(componentName).
+          containsKey(hostname)) {
+        throw new AmbariException("Duplicate entry for ServiceComponentHost"
+            + ", serviceName=" + serviceName
+            + ", serviceComponentName" + componentName
+            + ", hostname= " + hostname);
+      }
 
-    if (!serviceComponentHostsByHost.containsKey(hostname)) {
-      serviceComponentHostsByHost.put(hostname,
-          new ArrayList<ServiceComponentHost>());
-    }
+      if (!serviceComponentHostsByHost.containsKey(hostname)) {
+        serviceComponentHostsByHost.put(hostname,
+            new ArrayList<ServiceComponentHost>());
+      }
 
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Adding a new ServiceComponentHost"
-          + ", clusterName=" + getClusterName()
-          + ", clusterId=" + getClusterId()
-          + ", serviceName=" + serviceName
-          + ", serviceComponentName" + componentName
-          + ", hostname= " + hostname);
-    }
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Adding a new ServiceComponentHost"
+            + ", clusterName=" + getClusterName()
+            + ", clusterId=" + getClusterId()
+            + ", serviceName=" + serviceName
+            + ", serviceComponentName" + componentName
+            + ", hostname= " + hostname);
+      }
 
-    serviceComponentHosts.get(serviceName).get(componentName).put(hostname,
-        svcCompHost);
-    serviceComponentHostsByHost.get(hostname).add(svcCompHost);
+      serviceComponentHosts.get(serviceName).get(componentName).put(hostname,
+          svcCompHost);
+      serviceComponentHostsByHost.get(hostname).add(svcCompHost);
+    } finally {
+      writeLock.unlock();
+    }
   }
 
   @Override
   public long getClusterId() {
-    return clusterEntity.getClusterId();
+    readLock.lock();
+    try {
+      return clusterEntity.getClusterId();
+    } finally {
+      readLock.unlock();
+    }
   }
 
   @Override
-  public synchronized List<ServiceComponentHost> getServiceComponentHosts(
+  public List<ServiceComponentHost> getServiceComponentHosts(
       String hostname) {
     loadServiceHostComponents();
-    if (serviceComponentHostsByHost.containsKey(hostname)) {
-      return Collections.unmodifiableList(
-          serviceComponentHostsByHost.get(hostname));
+    readLock.lock();
+    try {
+      if (serviceComponentHostsByHost.containsKey(hostname)) {
+        return Collections.unmodifiableList(
+            serviceComponentHostsByHost.get(hostname));
+      }
+      return new ArrayList<ServiceComponentHost>();
+    } finally {
+      readLock.unlock();
     }
-    return new ArrayList<ServiceComponentHost>();
   }
 
   @Override
-  public synchronized void addService(Service service)
+  public void addService(Service service)
       throws AmbariException {
     loadServices();
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Adding a new Service"
-          + ", clusterName=" + getClusterName()
-          + ", clusterId=" + getClusterId()
-          + ", serviceName=" + service.getName());
-    }
-    if (services.containsKey(service.getName())) {
-      throw new AmbariException("Service already exists"
-          + ", clusterName=" + getClusterName()
-          + ", clusterId=" + getClusterId()
-          + ", serviceName=" + service.getName());
+    writeLock.lock();
+    try {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Adding a new Service"
+            + ", clusterName=" + getClusterName()
+            + ", clusterId=" + getClusterId()
+            + ", serviceName=" + service.getName());
+      }
+      if (services.containsKey(service.getName())) {
+        throw new AmbariException("Service already exists"
+            + ", clusterName=" + getClusterName()
+            + ", clusterId=" + getClusterId()
+            + ", serviceName=" + service.getName());
+      }
+      this.services.put(service.getName(), service);
+    } finally {
+      writeLock.unlock();
     }
-    this.services.put(service.getName(), service);
   }
 
   @Override
-  public synchronized Service addService(String serviceName) throws AmbariException{
+  public Service addService(String serviceName) throws AmbariException{
     loadServices();
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Adding a new Service"
-          + ", clusterName=" + getClusterName()
-          + ", clusterId=" + getClusterId()
-          + ", serviceName=" + serviceName);
-    }
-    if (services.containsKey(serviceName)) {
-      throw new AmbariException("Service already exists"
-          + ", clusterName=" + getClusterName()
-          + ", clusterId=" + getClusterId()
-          + ", serviceName=" + serviceName);
+    writeLock.lock();
+    try {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Adding a new Service"
+            + ", clusterName=" + getClusterName()
+            + ", clusterId=" + getClusterId()
+            + ", serviceName=" + serviceName);
+      }
+      if (services.containsKey(serviceName)) {
+        throw new AmbariException("Service already exists"
+            + ", clusterName=" + getClusterName()
+            + ", clusterId=" + getClusterId()
+            + ", serviceName=" + serviceName);
+      }
+      Service s = serviceFactory.createNew(this, serviceName);
+      this.services.put(s.getName(), s);
+      return s;
+    } finally {
+      writeLock.unlock();
     }
-    Service s = serviceFactory.createNew(this, serviceName);
-    this.services.put(s.getName(), s);
-    return s;
   }
 
   @Override
-  public synchronized Service getService(String serviceName)
+  public Service getService(String serviceName)
       throws AmbariException {
     loadServices();
-    if (!services.containsKey(serviceName)) {
-      throw new ServiceNotFoundException(getClusterName(), serviceName);
+    readLock.lock();
+    try {
+      if (!services.containsKey(serviceName)) {
+        throw new ServiceNotFoundException(getClusterName(), serviceName);
+      }
+      return services.get(serviceName);
+    } finally {
+      readLock.unlock();
     }
-    return services.get(serviceName);
   }
 
   @Override
-  public synchronized Map<String, Service> getServices() {
+  public Map<String, Service> getServices() {
     loadServices();
-    return Collections.unmodifiableMap(services);
+    readLock.lock();
+    try {
+      return Collections.unmodifiableMap(services);
+    } finally {
+      readLock.unlock();
+    }
   }
 
   @Override
-  public synchronized StackId getDesiredStackVersion() {
-    return desiredStackVersion;
+  public StackId getDesiredStackVersion() {
+    readLock.lock();
+    try {
+      return desiredStackVersion;
+    } finally {
+      readLock.unlock();
+    }
   }
 
   @Override
-  public synchronized void setDesiredStackVersion(StackId stackVersion) {
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Changing DesiredStackVersion of Cluster"
-        + ", clusterName=" + getClusterName()
-        + ", clusterId=" + getClusterId()
-        + ", currentDesiredStackVersion=" + this.desiredStackVersion
-        + ", newDesiredStackVersion=" + stackVersion);
+  public void setDesiredStackVersion(StackId stackVersion) {
+    readWriteLock.writeLock().lock();
+    try {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Changing DesiredStackVersion of Cluster"
+            + ", clusterName=" + getClusterName()
+            + ", clusterId=" + getClusterId()
+            + ", currentDesiredStackVersion=" + this.desiredStackVersion
+            + ", newDesiredStackVersion=" + stackVersion);
+      }
+      this.desiredStackVersion = stackVersion;
+      clusterEntity.setDesiredStackVersion(gson.toJson(stackVersion));
+      clusterDAO.merge(clusterEntity);
+    } finally {
+      readWriteLock.writeLock().unlock();
     }
-    this.desiredStackVersion = stackVersion;
-    clusterEntity.setDesiredStackVersion(gson.toJson(stackVersion));
-    clusterDAO.merge(clusterEntity);
+
   }
 
-  public synchronized StackId getDesiredState() {
+  public StackId getDesiredState() {
     //TODO separate implementation, mapped to StackVersion for now
 //    return desiredState; for separate implementation
-    return getDesiredStackVersion();
+    readWriteLock.readLock().lock();
+    try {
+      return getDesiredStackVersion();
+    } finally {
+      readWriteLock.readLock().unlock();
+    }
+
   }
 
-  public synchronized void setDesiredState(StackId desiredState) {
+  public void setDesiredState(StackId desiredState) {
     //TODO separate implementation, mapped to StackVersion for now
 //    LOG.debug("Changing desired state of cluster, clusterName={}, clusterId={}, oldState={}, newState={}",
 //        getClusterName(), getClusterId(), this.desiredState, desiredState);
 //    clusterEntity.setDesiredClusterState(gson.toJson(desiredState));
 //    clusterDAO.merge(clusterEntity);
 //    this.desiredState = desiredState;
-    setDesiredStackVersion(desiredState);
+    readWriteLock.writeLock().lock();
+    try {
+      setDesiredStackVersion(desiredState);
+    } finally {
+      readWriteLock.writeLock().unlock();
+    }
+
   }
 
 
   @Override
-  public synchronized Map<String, Config> getDesiredConfigsByType(String configType) {
-    if (!configs.containsKey(configType))
-      return null;
+  public Map<String, Config> getDesiredConfigsByType(String configType) {
+    readWriteLock.writeLock().lock();
+    try {
+      if (!configs.containsKey(configType))
+        return null;
+
+      return Collections.unmodifiableMap(configs.get(configType));
+    } finally {
+      readWriteLock.writeLock().unlock();
+    }
 
-    return Collections.unmodifiableMap(configs.get(configType));
   }
 
   @Override
-  public synchronized Config getDesiredConfig(String configType, String versionTag) {
-    if (!configs.containsKey(configType)
-        || !configs.get(configType).containsKey(versionTag)) {
-      return null;
+  public Config getDesiredConfig(String configType, String versionTag) {
+    readWriteLock.readLock().lock();
+    try {
+      if (!configs.containsKey(configType)
+          || !configs.get(configType).containsKey(versionTag)) {
+        return null;
+      }
+      return configs.get(configType).get(versionTag);
+    } finally {
+      readWriteLock.readLock().unlock();
     }
-    return configs.get(configType).get(versionTag);
+
   }
 
   @Override
-  public synchronized void addDesiredConfig(Config config) {
-    if (config.getType() == null
-        || config.getType().isEmpty()
-        || config.getVersionTag() == null
-        || config.getVersionTag().isEmpty()) {
-      // TODO throw error
-    }
-    if (!configs.containsKey(config.getType())) {
-      configs.put(config.getType(), new HashMap<String, Config>());
+  public void addDesiredConfig(Config config) {
+    readWriteLock.writeLock().lock();
+    try {
+      if (config.getType() == null
+          || config.getType().isEmpty()
+          || config.getVersionTag() == null
+          || config.getVersionTag().isEmpty()) {
+        // TODO throw error
+      }
+      if (!configs.containsKey(config.getType())) {
+        configs.put(config.getType(), new HashMap<String, Config>());
+      }
+
+      configs.get(config.getType()).put(config.getVersionTag(), config);
+    } finally {
+      readWriteLock.writeLock().unlock();
     }
 
-    configs.get(config.getType()).put(config.getVersionTag(), config);
   }
 
-  public synchronized Collection<Config> getAllConfigs() {
-    List<Config> list = new ArrayList<Config>();
-    for (Entry<String,Map<String,Config>> entry : configs.entrySet()) {
-      for (Config config : entry.getValue().values()) {
-        list.add(config);
+  public Collection<Config> getAllConfigs() {
+    readWriteLock.readLock().lock();
+    try {
+      List<Config> list = new ArrayList<Config>();
+      for (Entry<String, Map<String, Config>> entry : configs.entrySet()) {
+        for (Config config : entry.getValue().values()) {
+          list.add(config);
+        }
       }
+      return Collections.unmodifiableList(list);
+    } finally {
+      readWriteLock.readLock().unlock();
     }
-    return Collections.unmodifiableList(list);
+
   }
 
   @Override
-  public synchronized ClusterResponse convertToResponse()
+  public ClusterResponse convertToResponse()
       throws AmbariException {
-    ClusterResponse r = new ClusterResponse(getClusterId(), getClusterName(),
-        clusters.getHostsForCluster(getClusterName()).keySet(),
-        getDesiredStackVersion().getStackId());
-    return r;
+    readWriteLock.readLock().lock();
+    try {
+      ClusterResponse r = new ClusterResponse(getClusterId(), getClusterName(),
+          clusters.getHostsForCluster(getClusterName()).keySet(),
+          getDesiredStackVersion().getStackId());
+      return r;
+    } finally {
+      readWriteLock.readLock().unlock();
+    }
+
   }
 
-  public synchronized void debugDump(StringBuilder sb) {
+  public void debugDump(StringBuilder sb) {
     loadServices();
-    sb.append("Cluster={ clusterName=" + getClusterName()
-        + ", clusterId=" + getClusterId()
-        + ", desiredStackVersion=" + desiredStackVersion.getStackId()
-        + ", services=[ ");
-    boolean first = true;
-    for(Service s : services.values()) {
-      if (!first) {
-        sb.append(" , ");
-        first = false;
-      }
-      sb.append("\n    ");
-      s.debugDump(sb);
-      sb.append(" ");
+    readWriteLock.readLock().lock();
+    try {
+      sb.append("Cluster={ clusterName=" + getClusterName()
+          + ", clusterId=" + getClusterId()
+          + ", desiredStackVersion=" + desiredStackVersion.getStackId()
+          + ", services=[ ");
+      boolean first = true;
+      for (Service s : services.values()) {
+        if (!first) {
+          sb.append(" , ");
+          first = false;
+        }
+        sb.append("\n    ");
+        s.debugDump(sb);
+        sb.append(" ");
+      }
+      sb.append(" ] }");
+    } finally {
+      readWriteLock.readLock().unlock();
     }
-    sb.append(" ] }");
+
   }
 
   @Override
   @Transactional
-  public synchronized void refresh() {
-    clusterEntity = clusterDAO.findById(clusterEntity.getClusterId());
-    clusterDAO.refresh(clusterEntity);
+  public void refresh() {
+    readWriteLock.writeLock().lock();
+    try {
+      clusterEntity = clusterDAO.findById(clusterEntity.getClusterId());
+      clusterDAO.refresh(clusterEntity);
+    } finally {
+      readWriteLock.writeLock().unlock();
+    }
+
   }
 
   @Override
   @Transactional
-  public synchronized void deleteAllServices() throws AmbariException {
+  public void deleteAllServices() throws AmbariException {
     loadServices();
-    LOG.info("Deleting all services for cluster"
-        + ", clusterName=" + getClusterName());
-    for (Service service : services.values()) {
-      if (!service.canBeRemoved()) {
-        throw new AmbariException("Found non removable service when trying to"
-            + " all services from cluster"
-            + ", clusterName=" + getClusterName()
-            + ", serviceName=" + service.getName());
+    readWriteLock.writeLock().lock();
+    try {
+      LOG.info("Deleting all services for cluster"
+          + ", clusterName=" + getClusterName());
+      for (Service service : services.values()) {
+        if (!service.canBeRemoved()) {
+          throw new AmbariException("Found non removable service when trying to"
+              + " all services from cluster"
+              + ", clusterName=" + getClusterName()
+              + ", serviceName=" + service.getName());
+        }
       }
-    }
 
-    for (Service service : services.values()) {
-      service.delete();
+      for (Service service : services.values()) {
+        service.delete();
+      }
+
+      services.clear();
+    } finally {
+      readWriteLock.writeLock().unlock();
     }
 
-    services.clear();
   }
 
   @Override
-  public synchronized void deleteService(String serviceName)
+  public void deleteService(String serviceName)
       throws AmbariException {
     loadServices();
-    Service service = getService(serviceName);
-    LOG.info("Deleting service for cluster"
-        + ", clusterName=" + getClusterName()
-        + ", serviceName=" + service.getName());
-    // FIXME check dependencies from meta layer
-    if (!service.canBeRemoved()) {
-      throw new AmbariException("Could not delete service from cluster"
+    readWriteLock.writeLock().lock();
+    try {
+      Service service = getService(serviceName);
+      LOG.info("Deleting service for cluster"
           + ", clusterName=" + getClusterName()
           + ", serviceName=" + service.getName());
+      // FIXME check dependencies from meta layer
+      if (!service.canBeRemoved()) {
+        throw new AmbariException("Could not delete service from cluster"
+            + ", clusterName=" + getClusterName()
+            + ", serviceName=" + service.getName());
+      }
+      service.delete();
+      services.remove(serviceName);
+    } finally {
+      readWriteLock.writeLock().unlock();
     }
-    service.delete();
-    services.remove(serviceName);
+
   }
 
   @Override
   public boolean canBeRemoved() {
     loadServices();
-    boolean safeToRemove = true;
-    for (Service service : services.values()) {
-      if (!service.canBeRemoved()) {
-        safeToRemove = false;
-        LOG.warn("Found non removable service"
-            + ", clusterName=" + getClusterName()
-            + ", serviceName=" + service.getName());
+    readWriteLock.readLock().lock();
+    try {
+      boolean safeToRemove = true;
+      for (Service service : services.values()) {
+        if (!service.canBeRemoved()) {
+          safeToRemove = false;
+          LOG.warn("Found non removable service"
+              + ", clusterName=" + getClusterName()
+              + ", serviceName=" + service.getName());
+        }
       }
+      return safeToRemove;
+    } finally {
+      readWriteLock.readLock().unlock();
     }
-    return safeToRemove;
+
   }
 
   @Override
   @Transactional
   public void delete() throws AmbariException {
-    deleteAllServices();
-    removeEntities();
-    configs.clear();
+    readWriteLock.writeLock().lock();
+    try {
+      deleteAllServices();
+      removeEntities();
+      configs.clear();
+    } finally {
+      readWriteLock.writeLock().unlock();
+    }
+
   }
 
   @Transactional

Modified: incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
URL: http://svn.apache.org/viewvc/incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java?rev=1446799&r1=1446798&r2=1446799&view=diff
==============================================================================
--- incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java (original)
+++ incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java Fri Feb 15 22:55:21 2013
@@ -25,6 +25,9 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import javax.persistence.RollbackException;
 
@@ -56,11 +59,17 @@ public class ClustersImpl implements Clu
   private static final Logger LOG = LoggerFactory.getLogger(
       ClustersImpl.class);
 
-  private Map<String, Cluster> clusters;
-  private Map<Long, Cluster> clustersById;
-  private Map<String, Host> hosts;
-  private Map<String, Set<Cluster>> hostClusterMap;
-  private Map<String, Set<Host>> clusterHostMap;
+  private ConcurrentHashMap<String, Cluster> clusters;
+  private ConcurrentHashMap<Long, Cluster> clustersById;
+  private ConcurrentHashMap<String, Host> hosts;
+  private ConcurrentHashMap<String, Set<Cluster>> hostClusterMap;
+  private ConcurrentHashMap<String, Set<Host>> clusterHostMap;
+
+  private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();
+  private final Lock r = rwl.readLock();
+  private final Lock w = rwl.writeLock();
+
+  volatile boolean clustersLoaded = false;
 
   @Inject
   ClusterDAO clusterDAO;
@@ -77,159 +86,194 @@ public class ClustersImpl implements Clu
 
   @Inject
   public ClustersImpl() {
-    clusters = new HashMap<String, Cluster>();
-    clustersById = new HashMap<Long, Cluster>();
-    hosts = new HashMap<String, Host>();
-    hostClusterMap = new HashMap<String, Set<Cluster>>();
-    clusterHostMap = new HashMap<String, Set<Host>>();
+    clusters = new ConcurrentHashMap<String, Cluster>();
+    clustersById = new ConcurrentHashMap<Long, Cluster>();
+    hosts = new ConcurrentHashMap<String, Host>();
+    hostClusterMap = new ConcurrentHashMap<String, Set<Cluster>>();
+    clusterHostMap = new ConcurrentHashMap<String, Set<Host>>();
+
     LOG.info("Initializing the ClustersImpl");
   }
 
+  @Transactional
+  void loadClustersAndHosts() {
+    if (!clustersLoaded) {
+      w.lock();
+      try {
+        if (!clustersLoaded) {
+          for (ClusterEntity clusterEntity : clusterDAO.findAll()) {
+            Cluster currentCluster = clusterFactory.create(clusterEntity);
+            clusters.put(clusterEntity.getClusterName(), currentCluster);
+            clustersById.put(currentCluster.getClusterId(), currentCluster);
+            clusterHostMap.put(currentCluster.getClusterName(), Collections.newSetFromMap(new ConcurrentHashMap<Host, Boolean>()));
+          }
+
+          for (HostEntity hostEntity : hostDAO.findAll()) {
+            Host host = hostFactory.create(hostEntity, true);
+            hosts.put(hostEntity.getHostName(), host);
+            Set<Cluster> cSet = Collections.newSetFromMap(new ConcurrentHashMap<Cluster, Boolean>());
+            hostClusterMap.put(hostEntity.getHostName(), cSet);
+
+            for (ClusterEntity clusterEntity : hostEntity.getClusterEntities()) {
+              clusterHostMap.get(clusterEntity.getClusterName()).add(host);
+              cSet.add(clusters.get(clusterEntity.getClusterName()));
+            }
+          }
+        }
+        clustersLoaded = true;
+      } finally {
+        w.unlock();
+      }
+    }
+  }
+
   @Override
-  public synchronized void addCluster(String clusterName)
+  public void addCluster(String clusterName)
       throws AmbariException {
+    loadClustersAndHosts();
+
     if (clusters.containsKey(clusterName)) {
       throw new DuplicateResourceException("Attempted to create a Cluster which already exists"
           + ", clusterName=" + clusterName);
     }
 
-    // retrieve new cluster id
-    // add cluster id -> cluster mapping into clustersById
-    ClusterEntity clusterEntity = new ClusterEntity();
-    clusterEntity.setClusterName(clusterName);
-    clusterEntity.setDesiredStackVersion(gson.toJson(new StackId()));
+    w.lock();
     try {
-      clusterDAO.create(clusterEntity);
-      clusterEntity = clusterDAO.merge(clusterEntity);
-      Cluster cluster = clusterFactory.create(clusterEntity);
+      if (clusters.containsKey(clusterName)) {
+        throw new DuplicateResourceException("Attempted to create a Cluster which already exists"
+            + ", clusterName=" + clusterName);
+      }
+      // retrieve new cluster id
+      // add cluster id -> cluster mapping into clustersById
+      ClusterEntity clusterEntity = new ClusterEntity();
+      clusterEntity.setClusterName(clusterName);
+      clusterEntity.setDesiredStackVersion(gson.toJson(new StackId()));
 
+      try {
+        clusterDAO.create(clusterEntity);
+        clusterEntity = clusterDAO.merge(clusterEntity);
+      } catch (RollbackException e) {
+        LOG.warn("Unable to create cluster " + clusterName, e);
+        throw new AmbariException("Unable to create cluster " + clusterName, e);
+      }
+
+      Cluster cluster = clusterFactory.create(clusterEntity);
       clusters.put(clusterName, cluster);
       clustersById.put(cluster.getClusterId(), cluster);
       clusterHostMap.put(clusterName, new HashSet<Host>());
-    } catch (RollbackException e) {
-      LOG.warn("Unable to create cluster " + clusterName, e);
-      throw new AmbariException("Unable to create cluster " + clusterName, e);
+    } finally {
+      w.unlock();
     }
   }
 
   @Override
-  @Transactional
-  public synchronized Cluster getCluster(String clusterName)
+  public Cluster getCluster(String clusterName)
       throws AmbariException {
-    if (!clusters.containsKey(clusterName)) {
-      ClusterEntity clusterEntity = clusterDAO.findByName(clusterName);
-      if (clusterEntity != null) {
-        Cluster cl = getClusterById(clusterEntity.getClusterId());
-        clustersById.put(cl.getClusterId(), cl);
-        clusters.put(cl.getClusterName(), cl);
-        if (!clusterHostMap.containsKey(clusterEntity.getClusterName()))
-          clusterHostMap.put(clusterEntity.getClusterName(), new HashSet<Host>());
-      } else {
+    loadClustersAndHosts();
+    r.lock();
+    try {
+      if (!clusters.containsKey(clusterName)) {
         throw new ClusterNotFoundException(clusterName);
       }
+      return clusters.get(clusterName);
+    } finally {
+      r.unlock();
     }
-    return clusters.get(clusterName);
   }
 
   @Override
-  @Transactional
-  public synchronized Cluster getClusterById(long id) throws AmbariException {
-    if (!clustersById.containsKey(id)) {
-      ClusterEntity clusterEntity = clusterDAO.findById(id);
-      if (clusterEntity != null) {
-        Cluster cluster = clusterFactory.create(clusterEntity);
-        clustersById.put(cluster.getClusterId(), cluster);
-        clusters.put(clusterEntity.getClusterName(), cluster);
-        if (!clusterHostMap.containsKey(clusterEntity.getClusterName()))
-          clusterHostMap.put(clusterEntity.getClusterName(), new HashSet<Host>());
-      } else {
+  public Cluster getClusterById(long id) throws AmbariException {
+    loadClustersAndHosts();
+    r.lock();
+    try {
+      if (!clustersById.containsKey(id)) {
         throw new ClusterNotFoundException("clusterID=" + id);
       }
+      return clustersById.get(id);
+    } finally {
+      r.unlock();
     }
-    return clustersById.get(id);
   }
 
   @Override
   @Transactional
-  public synchronized List<Host> getHosts() {
-    List<Host> hostList = new ArrayList<Host>(hosts.size());
-    hostList.addAll(hosts.values());
-
-    for (HostEntity hostEntity : hostDAO.findAll()) {
-      if (!hosts.containsKey(hostEntity.getHostName())) {
-        try {
-          hostList.add(getHost(hostEntity.getHostName()));
-        } catch (AmbariException ignored) {
-          LOG.error("Database externally modified?");
-        }
-      }
-    }
+  public List<Host> getHosts() {
+    loadClustersAndHosts();
+    r.lock();
 
-    return hostList;
+    try {
+      List<Host> hostList = new ArrayList<Host>(hosts.size());
+      hostList.addAll(hosts.values());
+      return hostList;
+    } finally {
+      r.unlock();
+    }
   }
 
   @Override
-  public synchronized Set<Cluster> getClustersForHost(String hostname)
+  public Set<Cluster> getClustersForHost(String hostname)
       throws AmbariException {
-    if (!hostClusterMap.containsKey(hostname)) {
-      getHost(hostname);
-    }
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Looking up clusters for hostname"
-          + ", hostname=" + hostname
-          + ", mappedClusters=" + hostClusterMap.get(hostname).size());
+    loadClustersAndHosts();
+    r.lock();
+    try {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Looking up clusters for hostname"
+            + ", hostname=" + hostname
+            + ", mappedClusters=" + hostClusterMap.get(hostname).size());
+      }
+      return Collections.unmodifiableSet(hostClusterMap.get(hostname));
+    } finally {
+      r.unlock();
     }
-    return Collections.unmodifiableSet(hostClusterMap.get(hostname));
   }
 
   @Override
   @Transactional
-  public synchronized Host getHost(String hostname) throws AmbariException {
-    if (!hosts.containsKey(hostname)) {
-      HostEntity hostEntity = hostDAO.findByName(hostname);
-      if (hostEntity != null) {
-        Host host = hostFactory.create(hostEntity, true);
-        Set<Cluster> cSet = new HashSet<Cluster>();
-        hosts.put(hostname, host);
-        hostClusterMap.put(hostname, cSet);
-
-        for (ClusterEntity clusterEntity : hostEntity.getClusterEntities()) {
-          if (clustersById.containsKey(clusterEntity.getClusterId())) {
-            cSet.add(clustersById.get(clusterEntity.getClusterId()));
-          } else {
-            cSet.add(getClusterById(clusterEntity.getClusterId()));
-          }
-        }
-      } else {
+  public Host getHost(String hostname) throws AmbariException {
+    loadClustersAndHosts();
+    r.lock();
+    try {
+      if (!hosts.containsKey(hostname)) {
         throw new HostNotFoundException(hostname);
       }
+      return hosts.get(hostname);
+    } finally {
+      r.unlock();
     }
-    return hosts.get(hostname);
   }
 
   @Override
-  public synchronized void addHost(String hostname) throws AmbariException {
+  public void addHost(String hostname) throws AmbariException {
+    loadClustersAndHosts();
+    String duplicateMessage = "Duplicate entry for Host"
+        + ", hostName= " + hostname;
+
     if (hosts.containsKey(hostname)) {
-      throw new AmbariException("Duplicate entry for Host"
-          + ", hostName= " + hostname);
+      throw new AmbariException(duplicateMessage);
     }
-    HostEntity hostEntity = new HostEntity();
-    hostEntity.setHostName(hostname);
-    hostEntity.setClusterEntities(new ArrayList<ClusterEntity>());
-    //not stored to DB
-    Host host = hostFactory.create(hostEntity, false);
-    host.setAgentVersion(new AgentVersion(""));
-    List<DiskInfo> emptyDiskList = new ArrayList<DiskInfo>();
-    host.setDisksInfo(emptyDiskList);
-    host.setHealthStatus(new HostHealthStatus(HealthStatus.UNKNOWN, ""));
-    host.setHostAttributes(new HashMap<String, String>());
-    host.setState(HostState.INIT);
-
-    hosts.put(hostname, host);
-    hostClusterMap.put(hostname, new HashSet<Cluster>());
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Adding a host to Clusters"
-          + ", hostname=" + hostname);
+    r.lock();
+
+    try {
+      HostEntity hostEntity = new HostEntity();
+      hostEntity.setHostName(hostname);
+      hostEntity.setClusterEntities(new ArrayList<ClusterEntity>());
+      //not stored to DB
+      Host host = hostFactory.create(hostEntity, false);
+      host.setAgentVersion(new AgentVersion(""));
+      List<DiskInfo> emptyDiskList = new ArrayList<DiskInfo>();
+      host.setDisksInfo(emptyDiskList);
+      host.setHealthStatus(new HostHealthStatus(HealthStatus.UNKNOWN, ""));
+      host.setHostAttributes(new HashMap<String, String>());
+      host.setState(HostState.INIT);
+      hosts.put(hostname, host);
+      hostClusterMap.put(hostname, Collections.newSetFromMap(new ConcurrentHashMap<Cluster, Boolean>()));
+
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Adding a host to Clusters"
+            + ", hostname=" + hostname);
+      }
+    } finally {
+      r.unlock();
     }
   }
 
@@ -244,46 +288,46 @@ public class ClustersImpl implements Clu
   }
 
   @Override
-  public synchronized void mapHostToCluster(String hostname,
-      String clusterName) throws AmbariException {
-    Cluster cluster = getCluster(clusterName);
-    HostImpl host = (HostImpl) getHost(hostname);
+  public void mapHostToCluster(String hostname,
+                               String clusterName) throws AmbariException {
+    loadClustersAndHosts();
+    w.lock();
 
-    if (!hostClusterMap.containsKey(hostname)) {
-      throw new HostNotFoundException(hostname);
-    }
+    try {
+      Host host = getHost(hostname);
+      Cluster cluster = getCluster(clusterName);
 
-    for (Cluster c : hostClusterMap.get(hostname)) {
-      if (c.getClusterName().equals(clusterName)) {
-        throw new DuplicateResourceException("Attempted to create a host which already exists: clusterName=" +
-            clusterName + ", hostName=" + hostname);
+      for (Cluster c : hostClusterMap.get(hostname)) {
+        if (c.getClusterName().equals(clusterName)) {
+          throw new DuplicateResourceException("Attempted to create a host which already exists: clusterName=" +
+              clusterName + ", hostName=" + hostname);
+        }
       }
-    }
 
-    if (!isOsSupportedByClusterStack(cluster, host)) {
-      String message = "Trying to map host to cluster where stack does not"
-          + " support host's os type"
-          + ", clusterName=" + clusterName
-          + ", clusterStackId=" + cluster.getDesiredStackVersion().getStackId()
-          + ", hostname=" + hostname
-          + ", hostOsType=" + host.getOsType();
-      LOG.warn(message);
-      throw new AmbariException(message);
-    }
-
-    mapHostClusterEntities(hostname, cluster.getClusterId());
+      if (!isOsSupportedByClusterStack(cluster, host)) {
+        String message = "Trying to map host to cluster where stack does not"
+            + " support host's os type"
+            + ", clusterName=" + clusterName
+            + ", clusterStackId=" + cluster.getDesiredStackVersion().getStackId()
+            + ", hostname=" + hostname
+            + ", hostOsType=" + host.getOsType();
+        LOG.warn(message);
+        throw new AmbariException(message);
+      }
 
-    hostClusterMap.get(hostname).add(cluster);
-    clusterHostMap.get(clusterName).add(host);
+      mapHostClusterEntities(hostname, cluster.getClusterId());
 
-    cluster.refresh();
-    host.refresh();
+      hostClusterMap.get(hostname).add(cluster);
+      clusterHostMap.get(clusterName).add(host);
 
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Mapping a host to a cluster"
-          + ", clusterName=" + clusterName
-          + ", clusterId=" + cluster.getClusterId()
-          + ", hostname=" + hostname);
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Mapping a host to a cluster"
+            + ", clusterName=" + clusterName
+            + ", clusterId=" + cluster.getClusterId()
+            + ", hostname=" + hostname);
+      }
+    } finally {
+      w.unlock();
     }
   }
 
@@ -301,81 +345,105 @@ public class ClustersImpl implements Clu
 
   @Override
   @Transactional
-  public synchronized Map<String, Cluster> getClusters() {
-    for (ClusterEntity clusterEntity : clusterDAO.findAll()) {
-      try {
-        if (!clustersById.containsKey(clusterEntity.getClusterId())) {
-          getClusterById(clusterEntity.getClusterId());
-        }
-      } catch (AmbariException ignored) {
-
-      }
+  public Map<String, Cluster> getClusters() {
+    loadClustersAndHosts();
+    r.lock();
+    try {
+      return Collections.unmodifiableMap(clusters);
+    } finally {
+      r.unlock();
     }
-    return Collections.unmodifiableMap(clusters);
   }
 
   @Override
-  public synchronized void mapHostsToCluster(Set<String> hostnames,
-      String clusterName) throws AmbariException {
-    for (String hostname : hostnames) {
-      mapHostToCluster(hostname, clusterName);
+  public void mapHostsToCluster(Set<String> hostnames,
+                                             String clusterName) throws AmbariException {
+    loadClustersAndHosts();
+    w.lock();
+    try {
+      for (String hostname : hostnames) {
+        mapHostToCluster(hostname, clusterName);
+      }
+    } finally {
+      w.unlock();
     }
   }
 
   @Override
-  public synchronized void updateClusterName(String oldName, String newName) {
-    clusters.put(newName, clusters.remove(oldName));
+  public void updateClusterName(String oldName, String newName) {
+    w.lock();
+    try {
+      clusters.put(newName, clusters.remove(oldName));
+      clusterHostMap.put(newName, clusterHostMap.remove(oldName));
+    } finally {
+      w.unlock();
+    }
   }
 
 
   public void debugDump(StringBuilder sb) {
-    sb.append("Clusters=[ ");
-    boolean first = true;
-    for(Cluster c : clusters.values()) {
-      if (!first) {
-        sb.append(" , ");
-        first = false;
-      }
-      sb.append("\n  ");
-      c.debugDump(sb);
-      sb.append(" ");
+    r.lock();
+    try {
+      sb.append("Clusters=[ ");
+      boolean first = true;
+      for (Cluster c : clusters.values()) {
+        if (!first) {
+          sb.append(" , ");
+          first = false;
+        }
+        sb.append("\n  ");
+        c.debugDump(sb);
+        sb.append(" ");
+      }
+      sb.append(" ]");
+    } finally {
+      r.unlock();
     }
-    sb.append(" ]");
   }
 
   @Override
   @Transactional
   public Map<String, Host> getHostsForCluster(String clusterName)
       throws AmbariException {
+    loadClustersAndHosts();
+    r.lock();
 
-    getCluster(clusterName);
+    try {
+      Map<String, Host> hosts = new HashMap<String, Host>();
 
-    Map<String, Host> hosts = new HashMap<String, Host>();
+      for (Host h : clusterHostMap.get(clusterName)) {
+        hosts.put(h.getHostName(), h);
+      }
 
-    for (Host h : clusterHostMap.get(clusterName)) {
-      hosts.put(h.getHostName(), h);
+      return hosts;
+    } finally {
+      r.unlock();
     }
-
-    return hosts;
   }
 
   @Override
   public synchronized void deleteCluster(String clusterName)
       throws AmbariException {
-    Cluster cluster = getCluster(clusterName);
-    if (!cluster.canBeRemoved()) {
-      throw new AmbariException("Could not delete cluster"
-          + ", clusterName=" + clusterName);
-    }
-    LOG.info("Deleting cluster "+ cluster.getClusterName());
-    cluster.delete();
+    loadClustersAndHosts();
+    w.lock();
+    try {
+      Cluster cluster = getCluster(clusterName);
+      if (!cluster.canBeRemoved()) {
+        throw new AmbariException("Could not delete cluster"
+            + ", clusterName=" + clusterName);
+      }
+      LOG.info("Deleting cluster " + cluster.getClusterName());
+      cluster.delete();
 
-    //clear maps
-    for (Set<Cluster> clusterSet : hostClusterMap.values()) {
-      clusterSet.remove(cluster);
+      //clear maps
+      for (Set<Cluster> clusterSet : hostClusterMap.values()) {
+        clusterSet.remove(cluster);
+      }
+      clusterHostMap.remove(cluster.getClusterName());
+      clusters.remove(clusterName);
+    } finally {
+      w.unlock();
     }
-    clusterHostMap.remove(cluster.getClusterName());
-    clusters.remove(clusterName);
   }
 
 }

Modified: incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java
URL: http://svn.apache.org/viewvc/incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java?rev=1446799&r1=1446798&r2=1446799&view=diff
==============================================================================
--- incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java (original)
+++ incubator/ambari/trunk/ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java Fri Feb 15 22:55:21 2013
@@ -61,6 +61,7 @@ public class HostImpl implements Host {
   private static final Type hostAttributesType =
       new TypeToken<Map<String, String>>() {}.getType();
 
+  ReadWriteLock rwLock;
   private final Lock readLock;
   private final Lock writeLock;
 
@@ -185,7 +186,7 @@ public class HostImpl implements Host {
   public HostImpl(@Assisted HostEntity hostEntity,
       @Assisted boolean persisted, Injector injector) {
     this.stateMachine = stateMachineFactory.make(this);
-    ReadWriteLock rwLock = new ReentrantReadWriteLock();
+    rwLock = new ReentrantReadWriteLock();
     this.readLock = rwLock.readLock();
     this.writeLock = rwLock.writeLock();
 
@@ -961,8 +962,8 @@ public class HostImpl implements Host {
    */
   @Override
   public void persist() {
+    writeLock.lock();
     try {
-      writeLock.lock();
       if (!persisted) {
         persistEntities();
         refresh();
@@ -984,7 +985,7 @@ public class HostImpl implements Host {
   }
 
   @Transactional
-  protected void persistEntities() {
+  void persistEntities() {
     hostDAO.create(hostEntity);
     hostStateDAO.create(hostStateEntity);
     if (!hostEntity.getClusterEntities().isEmpty()) {
@@ -998,8 +999,8 @@ public class HostImpl implements Host {
   @Override
   @Transactional
   public void refresh() {
+    writeLock.lock();
     try {
-      writeLock.lock();
       if (isPersisted()) {
         hostEntity = hostDAO.findByName(hostEntity.getHostName());
         hostStateEntity = hostEntity.getHostStateEntity();
@@ -1012,7 +1013,7 @@ public class HostImpl implements Host {
   }
 
   @Transactional
-  private void saveIfPersisted() {
+  void saveIfPersisted() {
     if (isPersisted()) {
       hostDAO.merge(hostEntity);
       hostStateDAO.merge(hostStateEntity);