AMBARI-2195. Ambari has a deadlock when re-installing after reboot of cluster nodes. (Myroslav Papirkovskyy via smohanty)

git-svn-id: https://svn.apache.org/repos/asf/incubator/ambari/trunk@1489550 13f79535-47bb-0310-9956-ffa450edef68

ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java

@@ -21,6 +21,7 @@ package org.apache.ambari.server.state;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.locks.ReadWriteLock;
 
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.controller.ClusterResponse;
@@ -199,4 +200,9 @@ public interface Cluster {
    */
   Service addService(String serviceName) throws AmbariException;
 
+  /**
+   * Get lock to control access to cluster structure
+   * @return cluster-global lock
+   */
+  ReadWriteLock getClusterGlobalLock();
 }

ambari-server/src/main/java/org/apache/ambari/server/state/Service.java

@@ -19,6 +19,7 @@
 package org.apache.ambari.server.state;
 
 import java.util.Map;
+import java.util.concurrent.locks.ReadWriteLock;
 
 import com.google.inject.persist.Transactional;
 import org.apache.ambari.server.AmbariException;
@@ -85,6 +86,12 @@ public interface Service {
 
   public void delete() throws AmbariException;
 
+  /**
+   * Get lock to control access to cluster structure
+   * @return cluster-global lock
+   */
+  ReadWriteLock getClusterGlobalLock();
+
   public enum Type {
     HDFS,
     MAPREDUCE,

ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponent.java

@@ -20,6 +20,7 @@ package org.apache.ambari.server.state;
 
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.locks.ReadWriteLock;
 
 import com.google.inject.persist.Transactional;
 import org.apache.ambari.server.AmbariException;
@@ -84,4 +85,10 @@ public interface ServiceComponent {
       String hostName) throws AmbariException;
 
   public void delete() throws AmbariException;
+
+  /**
+   * Get lock to control access to cluster structure
+   * @return cluster-global lock
+   */
+  ReadWriteLock getClusterGlobalLock();
 }

ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java

@@ -44,9 +44,9 @@ public class ServiceComponentImpl implements ServiceComponent {
   private final static Logger LOG =
       LoggerFactory.getLogger(ServiceComponentImpl.class);
   
-  ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
-
   private final Service service;
+  private final ReadWriteLock clusterGlobalLock;
+  private final ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
 
   @Inject
   private Gson gson;
@@ -84,6 +84,7 @@ public class ServiceComponentImpl implements ServiceComponent {
   public ServiceComponentImpl(@Assisted Service service,
       @Assisted String componentName, Injector injector) throws AmbariException {
     injector.injectMembers(this);
+    this.clusterGlobalLock = service.getClusterGlobalLock();
     this.service = service;
     this.desiredStateEntity = new ServiceComponentDesiredStateEntity();
     desiredStateEntity.setComponentName(componentName);
@@ -116,6 +117,7 @@ public class ServiceComponentImpl implements ServiceComponent {
                               @Assisted ServiceComponentDesiredStateEntity serviceComponentDesiredStateEntity,
                               Injector injector) throws AmbariException {
     injector.injectMembers(this);
+    this.clusterGlobalLock = service.getClusterGlobalLock();
     this.service = service;
     this.desiredStateEntity = serviceComponentDesiredStateEntity;
 
@@ -157,375 +159,494 @@ public class ServiceComponentImpl implements ServiceComponent {
     persisted = true;
   }
 
+  @Override
+  public ReadWriteLock getClusterGlobalLock() {
+    return clusterGlobalLock;
+  }
+
   @Override
   public String getName() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return desiredStateEntity.getComponentName();
+      readWriteLock.readLock().lock();
+      try {
+        return desiredStateEntity.getComponentName();
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public String getServiceName() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return service.getName();
+      readWriteLock.readLock().lock();
+      try {
+        return service.getName();
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public long getClusterId() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return this.service.getClusterId();
+      readWriteLock.readLock().lock();
+      try {
+        return this.service.getClusterId();
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public Map<String, ServiceComponentHost>
       getServiceComponentHosts() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return Collections.unmodifiableMap(hostComponents);
+      readWriteLock.readLock().lock();
+      try {
+        return Collections.unmodifiableMap(hostComponents);
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public void addServiceComponentHosts(
       Map<String, ServiceComponentHost> hostComponents) throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      // TODO validation
-      for (Entry<String, ServiceComponentHost> entry :
-          hostComponents.entrySet()) {
-        if (!entry.getKey().equals(entry.getValue().getHostName())) {
-          throw new AmbariException("Invalid arguments in map"
-              + ", hostname does not match the key in map");
+      readWriteLock.writeLock().lock();
+      try {
+        // TODO validation
+        for (Entry<String, ServiceComponentHost> entry :
+            hostComponents.entrySet()) {
+          if (!entry.getKey().equals(entry.getValue().getHostName())) {
+            throw new AmbariException("Invalid arguments in map"
+                + ", hostname does not match the key in map");
+          }
         }
-      }
-      for (ServiceComponentHost sch : hostComponents.values()) {
-        addServiceComponentHost(sch);
+        for (ServiceComponentHost sch : hostComponents.values()) {
+          addServiceComponentHost(sch);
+        }
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
 
+
   }
 
   @Override
   public void addServiceComponentHost(
       ServiceComponentHost hostComponent) throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      // TODO validation
-      // TODO ensure host belongs to cluster
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Adding a ServiceComponentHost to ServiceComponent"
-            + ", clusterName=" + service.getCluster().getClusterName()
-            + ", clusterId=" + service.getCluster().getClusterId()
-            + ", serviceName=" + service.getName()
-            + ", serviceComponentName=" + getName()
-            + ", hostname=" + hostComponent.getHostName());
-      }
-      if (hostComponents.containsKey(hostComponent.getHostName())) {
-        throw new AmbariException("Cannot add duplicate ServiceComponentHost"
-            + ", clusterName=" + service.getCluster().getClusterName()
-            + ", clusterId=" + service.getCluster().getClusterId()
-            + ", serviceName=" + service.getName()
-            + ", serviceComponentName=" + getName()
-            + ", hostname=" + hostComponent.getHostName());
+      readWriteLock.writeLock().lock();
+      try {
+        // TODO validation
+        // TODO ensure host belongs to cluster
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Adding a ServiceComponentHost to ServiceComponent"
+              + ", clusterName=" + service.getCluster().getClusterName()
+              + ", clusterId=" + service.getCluster().getClusterId()
+              + ", serviceName=" + service.getName()
+              + ", serviceComponentName=" + getName()
+              + ", hostname=" + hostComponent.getHostName());
+        }
+        if (hostComponents.containsKey(hostComponent.getHostName())) {
+          throw new AmbariException("Cannot add duplicate ServiceComponentHost"
+              + ", clusterName=" + service.getCluster().getClusterName()
+              + ", clusterId=" + service.getCluster().getClusterId()
+              + ", serviceName=" + service.getName()
+              + ", serviceComponentName=" + getName()
+              + ", hostname=" + hostComponent.getHostName());
+        }
+        // FIXME need a better approach of caching components by host
+        ClusterImpl clusterImpl = (ClusterImpl) service.getCluster();
+        clusterImpl.addServiceComponentHost(hostComponent);
+        this.hostComponents.put(hostComponent.getHostName(), hostComponent);
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
-      // FIXME need a better approach of caching components by host
-      ClusterImpl clusterImpl = (ClusterImpl) service.getCluster();
-      clusterImpl.addServiceComponentHost(hostComponent);
-      this.hostComponents.put(hostComponent.getHostName(), hostComponent);
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
 
+
   }
 
   @Override
   public ServiceComponentHost addServiceComponentHost(
       String hostName) throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      // TODO validation
-      // TODO ensure host belongs to cluster
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Adding a ServiceComponentHost to ServiceComponent"
-            + ", clusterName=" + service.getCluster().getClusterName()
-            + ", clusterId=" + service.getCluster().getClusterId()
-            + ", serviceName=" + service.getName()
-            + ", serviceComponentName=" + getName()
-            + ", hostname=" + hostName);
-      }
-      if (hostComponents.containsKey(hostName)) {
-        throw new AmbariException("Cannot add duplicate ServiceComponentHost"
-            + ", clusterName=" + service.getCluster().getClusterName()
-            + ", clusterId=" + service.getCluster().getClusterId()
-            + ", serviceName=" + service.getName()
-            + ", serviceComponentName=" + getName()
-            + ", hostname=" + hostName);
-      }
-      ServiceComponentHost hostComponent =
-          serviceComponentHostFactory.createNew(this, hostName, this.isClientComponent());
-      // FIXME need a better approach of caching components by host
-      ClusterImpl clusterImpl = (ClusterImpl) service.getCluster();
-      clusterImpl.addServiceComponentHost(hostComponent);
+      readWriteLock.writeLock().lock();
+      try {
+        // TODO validation
+        // TODO ensure host belongs to cluster
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Adding a ServiceComponentHost to ServiceComponent"
+              + ", clusterName=" + service.getCluster().getClusterName()
+              + ", clusterId=" + service.getCluster().getClusterId()
+              + ", serviceName=" + service.getName()
+              + ", serviceComponentName=" + getName()
+              + ", hostname=" + hostName);
+        }
+        if (hostComponents.containsKey(hostName)) {
+          throw new AmbariException("Cannot add duplicate ServiceComponentHost"
+              + ", clusterName=" + service.getCluster().getClusterName()
+              + ", clusterId=" + service.getCluster().getClusterId()
+              + ", serviceName=" + service.getName()
+              + ", serviceComponentName=" + getName()
+              + ", hostname=" + hostName);
+        }
+        ServiceComponentHost hostComponent =
+            serviceComponentHostFactory.createNew(this, hostName, this.isClientComponent());
+        // FIXME need a better approach of caching components by host
+        ClusterImpl clusterImpl = (ClusterImpl) service.getCluster();
+        clusterImpl.addServiceComponentHost(hostComponent);
 
-      this.hostComponents.put(hostComponent.getHostName(), hostComponent);
+        this.hostComponents.put(hostComponent.getHostName(), hostComponent);
 
-      return hostComponent;
+        return hostComponent;
+      } finally {
+        readWriteLock.writeLock().unlock();
+      }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
 
+
   }
 
   @Override
   public ServiceComponentHost getServiceComponentHost(String hostname)
     throws AmbariException {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (!hostComponents.containsKey(hostname)) {
-        throw new ServiceComponentHostNotFoundException(getClusterName(),
-            getServiceName(), getName(), hostname);
+      readWriteLock.readLock().lock();
+      try {
+        if (!hostComponents.containsKey(hostname)) {
+          throw new ServiceComponentHostNotFoundException(getClusterName(),
+              getServiceName(), getName(), hostname);
+        }
+        return this.hostComponents.get(hostname);
+      } finally {
+        readWriteLock.readLock().unlock();
       }
-      return this.hostComponents.get(hostname);
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public State getDesiredState() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return desiredStateEntity.getDesiredState();
+      readWriteLock.readLock().lock();
+      try {
+        return desiredStateEntity.getDesiredState();
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public void setDesiredState(State state) {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Setting DesiredState of Service"
-            + ", clusterName=" + service.getCluster().getClusterName()
-            + ", clusterId=" + service.getCluster().getClusterId()
-            + ", serviceName=" + service.getName()
-            + ", serviceComponentName=" + getName()
-            + ", oldDesiredState=" + getDesiredState()
-            + ", newDesiredState=" + state);
+      readWriteLock.writeLock().lock();
+      try {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Setting DesiredState of Service"
+              + ", clusterName=" + service.getCluster().getClusterName()
+              + ", clusterId=" + service.getCluster().getClusterId()
+              + ", serviceName=" + service.getName()
+              + ", serviceComponentName=" + getName()
+              + ", oldDesiredState=" + getDesiredState()
+              + ", newDesiredState=" + state);
+        }
+        desiredStateEntity.setDesiredState(state);
+        saveIfPersisted();
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
-      desiredStateEntity.setDesiredState(state);
-      saveIfPersisted();
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public Map<String, Config> getDesiredConfigs() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      Map<String, Config> map = new HashMap<String, Config>();
-      for (Entry<String, String> entry : desiredConfigs.entrySet()) {
-        Config config = service.getCluster().getConfig(entry.getKey(), entry.getValue());
-        if (null != config) {
-          map.put(entry.getKey(), config);
+      readWriteLock.readLock().lock();
+      try {
+        Map<String, Config> map = new HashMap<String, Config>();
+        for (Entry<String, String> entry : desiredConfigs.entrySet()) {
+          Config config = service.getCluster().getConfig(entry.getKey(), entry.getValue());
+          if (null != config) {
+            map.put(entry.getKey(), config);
+          }
         }
-      }
 
-      Map<String, Config> svcConfigs = service.getDesiredConfigs();
-      for (Entry<String, Config> entry : svcConfigs.entrySet()) {
-        if (!map.containsKey(entry.getKey())) {
-          map.put(entry.getKey(), entry.getValue());
+        Map<String, Config> svcConfigs = service.getDesiredConfigs();
+        for (Entry<String, Config> entry : svcConfigs.entrySet()) {
+          if (!map.containsKey(entry.getKey())) {
+            map.put(entry.getKey(), entry.getValue());
+          }
         }
-      }
 
-      return Collections.unmodifiableMap(map);
+        return Collections.unmodifiableMap(map);
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public void updateDesiredConfigs(Map<String, Config> configs) {
 
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      for (Entry<String, Config> entry : configs.entrySet()) {
-        boolean contains = false;
-
-        for (ComponentConfigMappingEntity componentConfigMappingEntity : desiredStateEntity.getComponentConfigMappingEntities()) {
-          if (entry.getKey().equals(componentConfigMappingEntity.getConfigType())) {
-            contains = true;
-            componentConfigMappingEntity.setTimestamp(new Date().getTime());
-            componentConfigMappingEntity.setVersionTag(entry.getValue().getVersionTag());
-            if (persisted) {
-              componentConfigMappingDAO.merge(componentConfigMappingEntity);
+      readWriteLock.writeLock().lock();
+      try {
+        for (Entry<String, Config> entry : configs.entrySet()) {
+          boolean contains = false;
+
+          for (ComponentConfigMappingEntity componentConfigMappingEntity : desiredStateEntity.getComponentConfigMappingEntities()) {
+            if (entry.getKey().equals(componentConfigMappingEntity.getConfigType())) {
+              contains = true;
+              componentConfigMappingEntity.setTimestamp(new Date().getTime());
+              componentConfigMappingEntity.setVersionTag(entry.getValue().getVersionTag());
+              if (persisted) {
+                componentConfigMappingDAO.merge(componentConfigMappingEntity);
+              }
             }
           }
-        }
 
-        if (!contains) {
-          ComponentConfigMappingEntity newEntity = new ComponentConfigMappingEntity();
-          newEntity.setClusterId(desiredStateEntity.getClusterId());
-          newEntity.setServiceName(desiredStateEntity.getServiceName());
-          newEntity.setComponentName(desiredStateEntity.getComponentName());
-          newEntity.setConfigType(entry.getKey());
-          newEntity.setVersionTag(entry.getValue().getVersionTag());
-          newEntity.setTimestamp(new Date().getTime());
-          newEntity.setServiceComponentDesiredStateEntity(desiredStateEntity);
-          desiredStateEntity.getComponentConfigMappingEntities().add(newEntity);
+          if (!contains) {
+            ComponentConfigMappingEntity newEntity = new ComponentConfigMappingEntity();
+            newEntity.setClusterId(desiredStateEntity.getClusterId());
+            newEntity.setServiceName(desiredStateEntity.getServiceName());
+            newEntity.setComponentName(desiredStateEntity.getComponentName());
+            newEntity.setConfigType(entry.getKey());
+            newEntity.setVersionTag(entry.getValue().getVersionTag());
+            newEntity.setTimestamp(new Date().getTime());
+            newEntity.setServiceComponentDesiredStateEntity(desiredStateEntity);
+            desiredStateEntity.getComponentConfigMappingEntities().add(newEntity);
 
-        }
+          }
 
 
-        this.desiredConfigs.put(entry.getKey(), entry.getValue().getVersionTag());
-      }
+          this.desiredConfigs.put(entry.getKey(), entry.getValue().getVersionTag());
+        }
 
-      saveIfPersisted();
+        saveIfPersisted();
+      } finally {
+        readWriteLock.writeLock().unlock();
+      }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public StackId getDesiredStackVersion() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return gson.fromJson(desiredStateEntity.getDesiredStackVersion(), StackId.class);
+      readWriteLock.readLock().lock();
+      try {
+        return gson.fromJson(desiredStateEntity.getDesiredStackVersion(), StackId.class);
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public void setDesiredStackVersion(StackId stackVersion) {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Setting DesiredStackVersion of Service"
-            + ", clusterName=" + service.getCluster().getClusterName()
-            + ", clusterId=" + service.getCluster().getClusterId()
-            + ", serviceName=" + service.getName()
-            + ", serviceComponentName=" + getName()
-            + ", oldDesiredStackVersion=" + getDesiredStackVersion()
-            + ", newDesiredStackVersion=" + stackVersion);
+      readWriteLock.writeLock().lock();
+      try {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Setting DesiredStackVersion of Service"
+              + ", clusterName=" + service.getCluster().getClusterName()
+              + ", clusterId=" + service.getCluster().getClusterId()
+              + ", serviceName=" + service.getName()
+              + ", serviceComponentName=" + getName()
+              + ", oldDesiredStackVersion=" + getDesiredStackVersion()
+              + ", newDesiredStackVersion=" + stackVersion);
+        }
+        desiredStateEntity.setDesiredStackVersion(gson.toJson(stackVersion));
+        saveIfPersisted();
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
-      desiredStateEntity.setDesiredStackVersion(gson.toJson(stackVersion));
-      saveIfPersisted();
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public ServiceComponentResponse convertToResponse() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      ServiceComponentResponse r = new ServiceComponentResponse(
-          getClusterId(), service.getCluster().getClusterName(),
-          service.getName(), getName(), this.desiredConfigs,
-          getDesiredStackVersion().getStackId(),
-          getDesiredState().toString());
-      return r;
+      readWriteLock.readLock().lock();
+      try {
+        ServiceComponentResponse r = new ServiceComponentResponse(
+            getClusterId(), service.getCluster().getClusterName(),
+            service.getName(), getName(), this.desiredConfigs,
+            getDesiredStackVersion().getStackId(),
+            getDesiredState().toString());
+        return r;
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public String getClusterName() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return service.getCluster().getClusterName();
+      readWriteLock.readLock().lock();
+      try {
+        return service.getCluster().getClusterName();
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public void debugDump(StringBuilder sb) {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      sb.append("ServiceComponent={ serviceComponentName=" + getName()
-          + ", clusterName=" + service.getCluster().getClusterName()
-          + ", clusterId=" + service.getCluster().getClusterId()
-          + ", serviceName=" + service.getName()
-          + ", desiredStackVersion=" + getDesiredStackVersion()
-          + ", desiredState=" + getDesiredState().toString()
-          + ", hostcomponents=[ ");
-      boolean first = true;
-      for (ServiceComponentHost sch : hostComponents.values()) {
-        if (!first) {
-          sb.append(" , ");
-          first = false;
+      readWriteLock.readLock().lock();
+      try {
+        sb.append("ServiceComponent={ serviceComponentName=" + getName()
+            + ", clusterName=" + service.getCluster().getClusterName()
+            + ", clusterId=" + service.getCluster().getClusterId()
+            + ", serviceName=" + service.getName()
+            + ", desiredStackVersion=" + getDesiredStackVersion()
+            + ", desiredState=" + getDesiredState().toString()
+            + ", hostcomponents=[ ");
+        boolean first = true;
+        for (ServiceComponentHost sch : hostComponents.values()) {
+          if (!first) {
+            sb.append(" , ");
+            first = false;
+          }
+          sb.append("\n        ");
+          sch.debugDump(sb);
+          sb.append(" ");
         }
-        sb.append("\n        ");
-        sch.debugDump(sb);
-        sb.append(" ");
+        sb.append(" ] }");
+      } finally {
+        readWriteLock.readLock().unlock();
       }
-      sb.append(" ] }");
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public boolean isPersisted() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return persisted;
+      readWriteLock.readLock().lock();
+      try {
+        return persisted;
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public void persist() {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (!persisted) {
-        persistEntities();
-        refresh();
-        service.refresh();
-        persisted = true;
-      } else {
-        saveIfPersisted();
+      readWriteLock.writeLock().lock();
+      try {
+        if (!persisted) {
+          persistEntities();
+          refresh();
+          service.refresh();
+          persisted = true;
+        } else {
+          saveIfPersisted();
+        }
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Transactional
@@ -543,34 +664,46 @@ public class ServiceComponentImpl implements ServiceComponent {
   @Override
   @Transactional
   public void refresh() {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (isPersisted()) {
-        ServiceComponentDesiredStateEntityPK pk = new ServiceComponentDesiredStateEntityPK();
-        pk.setComponentName(getName());
-        pk.setClusterId(getClusterId());
-        pk.setServiceName(getServiceName());
-        // TODO: desiredStateEntity is assigned in unway, may be a bug
-        desiredStateEntity = serviceComponentDesiredStateDAO.findByPK(pk);
-        serviceComponentDesiredStateDAO.refresh(desiredStateEntity);
+      readWriteLock.writeLock().lock();
+      try {
+        if (isPersisted()) {
+          ServiceComponentDesiredStateEntityPK pk = new ServiceComponentDesiredStateEntityPK();
+          pk.setComponentName(getName());
+          pk.setClusterId(getClusterId());
+          pk.setServiceName(getServiceName());
+          // TODO: desiredStateEntity is assigned in unway, may be a bug
+          desiredStateEntity = serviceComponentDesiredStateDAO.findByPK(pk);
+          serviceComponentDesiredStateDAO.refresh(desiredStateEntity);
+        }
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Transactional
   private void saveIfPersisted() {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (isPersisted()) {
-        serviceComponentDesiredStateDAO.merge(desiredStateEntity);
+      readWriteLock.writeLock().lock();
+      try {
+        if (isPersisted()) {
+          serviceComponentDesiredStateDAO.merge(desiredStateEntity);
+        }
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
@@ -580,124 +713,154 @@ public class ServiceComponentImpl implements ServiceComponent {
 
   @Override
   public boolean canBeRemoved() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (!getDesiredState().isRemovableState()) {
-        return false;
-      }
-
-      for (ServiceComponentHost sch : hostComponents.values()) {
-        if (!sch.canBeRemoved()) {
-          LOG.warn("Found non removable hostcomponent when trying to"
-              + " delete service component"
-              + ", clusterName=" + getClusterName()
-              + ", serviceName=" + getServiceName()
-              + ", componentName=" + getName()
-              + ", hostname=" + sch.getHostName());
+      readWriteLock.readLock().lock();
+      try {
+        if (!getDesiredState().isRemovableState()) {
           return false;
         }
+
+        for (ServiceComponentHost sch : hostComponents.values()) {
+          if (!sch.canBeRemoved()) {
+            LOG.warn("Found non removable hostcomponent when trying to"
+                + " delete service component"
+                + ", clusterName=" + getClusterName()
+                + ", serviceName=" + getServiceName()
+                + ", componentName=" + getName()
+                + ", hostname=" + sch.getHostName());
+            return false;
+          }
+        }
+        return true;
+      } finally {
+        readWriteLock.readLock().unlock();
       }
-      return true;
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   @Transactional
   public void deleteAllServiceComponentHosts()
       throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      LOG.info("Deleting all servicecomponenthosts for component"
-          + ", clusterName=" + getClusterName()
-          + ", serviceName=" + getServiceName()
-          + ", componentName=" + getName());
-      for (ServiceComponentHost sch : hostComponents.values()) {
-        if (!sch.canBeRemoved()) {
-          throw new AmbariException("Found non removable hostcomponent "
-              + " when trying to delete"
-              + " all hostcomponents from servicecomponent"
-              + ", clusterName=" + getClusterName()
-              + ", serviceName=" + getServiceName()
-              + ", componentName=" + getName()
-              + ", hostname=" + sch.getHostName());
+      readWriteLock.writeLock().lock();
+      try {
+        LOG.info("Deleting all servicecomponenthosts for component"
+            + ", clusterName=" + getClusterName()
+            + ", serviceName=" + getServiceName()
+            + ", componentName=" + getName());
+        for (ServiceComponentHost sch : hostComponents.values()) {
+          if (!sch.canBeRemoved()) {
+            throw new AmbariException("Found non removable hostcomponent "
+                + " when trying to delete"
+                + " all hostcomponents from servicecomponent"
+                + ", clusterName=" + getClusterName()
+                + ", serviceName=" + getServiceName()
+                + ", componentName=" + getName()
+                + ", hostname=" + sch.getHostName());
+          }
         }
-      }
 
-      for (ServiceComponentHost serviceComponentHost : hostComponents.values()) {
-        serviceComponentHost.delete();
-      }
+        for (ServiceComponentHost serviceComponentHost : hostComponents.values()) {
+          serviceComponentHost.delete();
+        }
 
-      hostComponents.clear();
+        hostComponents.clear();
+      } finally {
+        readWriteLock.writeLock().unlock();
+      }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
 
+
   }
 
   @Override
   public void deleteServiceComponentHosts(String hostname)
       throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      ServiceComponentHost sch = getServiceComponentHost(hostname);
-      LOG.info("Deleting servicecomponenthost for cluster"
-          + ", clusterName=" + getClusterName()
-          + ", serviceName=" + getServiceName()
-          + ", componentName=" + getName()
-          + ", hostname=" + sch.getHostName());
-      if (!sch.canBeRemoved()) {
-        throw new AmbariException("Could not delete hostcomponent from cluster"
+      readWriteLock.writeLock().lock();
+      try {
+        ServiceComponentHost sch = getServiceComponentHost(hostname);
+        LOG.info("Deleting servicecomponenthost for cluster"
             + ", clusterName=" + getClusterName()
             + ", serviceName=" + getServiceName()
             + ", componentName=" + getName()
             + ", hostname=" + sch.getHostName());
+        if (!sch.canBeRemoved()) {
+          throw new AmbariException("Could not delete hostcomponent from cluster"
+              + ", clusterName=" + getClusterName()
+              + ", serviceName=" + getServiceName()
+              + ", componentName=" + getName()
+              + ", hostname=" + sch.getHostName());
+        }
+        sch.delete();
+        hostComponents.remove(hostname);
+
+        // FIXME need a better approach of caching components by host
+        ClusterImpl clusterImpl = (ClusterImpl) service.getCluster();
+        clusterImpl.removeServiceComponentHost(sch);
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
-      sch.delete();
-      hostComponents.remove(hostname);
-
-      // FIXME need a better approach of caching components by host
-      ClusterImpl clusterImpl = (ClusterImpl) service.getCluster();
-      clusterImpl.removeServiceComponentHost(sch);
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
 
+
   }
 
   @Override
   public void deleteDesiredConfigs(Set<String> configTypes) {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      componentConfigMappingDAO.removeByType(configTypes);
-      for (String configType : configTypes) {
-        desiredConfigs.remove(configType);
+      readWriteLock.writeLock().lock();
+      try {
+        componentConfigMappingDAO.removeByType(configTypes);
+        for (String configType : configTypes) {
+          desiredConfigs.remove(configType);
+        }
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   @Transactional
   public void delete() throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      deleteAllServiceComponentHosts();
+      readWriteLock.writeLock().lock();
+      try {
+        deleteAllServiceComponentHosts();
 
-      if (persisted) {
-        removeEntities();
-        persisted = false;
-      }
+        if (persisted) {
+          removeEntities();
+          persisted = false;
+        }
 
-      desiredConfigs.clear();
+        desiredConfigs.clear();
+      } finally {
+        readWriteLock.writeLock().unlock();
+      }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
 
+
   }
 
   @Transactional

ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java

@@ -41,6 +41,7 @@ import com.google.inject.persist.Transactional;
 
 
 public class ServiceImpl implements Service {
+  private final ReadWriteLock clusterGlobalLock;
   private ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
 
   private ClusterServiceEntity serviceEntity;
@@ -83,6 +84,7 @@ public class ServiceImpl implements Service {
   public ServiceImpl(@Assisted Cluster cluster, @Assisted String serviceName,
       Injector injector) throws AmbariException {
     injector.injectMembers(this);
+    clusterGlobalLock = cluster.getClusterGlobalLock();
     serviceEntity = new ClusterServiceEntity();
     serviceEntity.setServiceName(serviceName);
     serviceDesiredStateEntity = new ServiceDesiredStateEntity();
@@ -109,6 +111,7 @@ public class ServiceImpl implements Service {
   public ServiceImpl(@Assisted Cluster cluster, @Assisted ClusterServiceEntity
       serviceEntity, Injector injector) throws AmbariException {
     injector.injectMembers(this);
+    clusterGlobalLock = cluster.getClusterGlobalLock();
     this.serviceEntity = serviceEntity;
     this.cluster = cluster;
 
@@ -142,255 +145,336 @@ public class ServiceImpl implements Service {
     persisted = true;
   }
 
+  @Override
+  public ReadWriteLock getClusterGlobalLock() {
+    return clusterGlobalLock;
+  }
+
   @Override
   public String getName() {
-    return serviceEntity.getServiceName();
+    clusterGlobalLock.readLock().lock();
+    try {
+      readWriteLock.readLock().lock();
+      try {
+        return serviceEntity.getServiceName();
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
+    } finally {
+      clusterGlobalLock.readLock().unlock();
+    }
   }
 
   @Override
   public long getClusterId() {
-    return cluster.getClusterId();
+    clusterGlobalLock.readLock().lock();
+    try {
+      readWriteLock.readLock().lock();
+      try {
+        return cluster.getClusterId();
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
+    } finally {
+      clusterGlobalLock.readLock().unlock();
+    }
   }
 
   @Override
   public Map<String, ServiceComponent> getServiceComponents() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return Collections.unmodifiableMap(components);
+      readWriteLock.readLock().lock();
+      try {
+        return Collections.unmodifiableMap(components);
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
-
   }
 
   @Override
   public void addServiceComponents(
       Map<String, ServiceComponent> components) throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      for (ServiceComponent sc : components.values()) {
-        addServiceComponent(sc);
+      readWriteLock.writeLock().lock();
+      try {
+        for (ServiceComponent sc : components.values()) {
+          addServiceComponent(sc);
+        }
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
-
   }
 
   @Override
   public void addServiceComponent(ServiceComponent component)
       throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      // TODO validation
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Adding a ServiceComponent to Service"
-            + ", clusterName=" + cluster.getClusterName()
-            + ", clusterId=" + cluster.getClusterId()
-            + ", serviceName=" + getName()
-            + ", serviceComponentName=" + component.getName());
-      }
-      if (components.containsKey(component.getName())) {
-        throw new AmbariException("Cannot add duplicate ServiceComponent"
-            + ", clusterName=" + cluster.getClusterName()
-            + ", clusterId=" + cluster.getClusterId()
-            + ", serviceName=" + getName()
-            + ", serviceComponentName=" + component.getName());
+      readWriteLock.writeLock().lock();
+      try {
+        // TODO validation
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Adding a ServiceComponent to Service"
+              + ", clusterName=" + cluster.getClusterName()
+              + ", clusterId=" + cluster.getClusterId()
+              + ", serviceName=" + getName()
+              + ", serviceComponentName=" + component.getName());
+        }
+        if (components.containsKey(component.getName())) {
+          throw new AmbariException("Cannot add duplicate ServiceComponent"
+              + ", clusterName=" + cluster.getClusterName()
+              + ", clusterId=" + cluster.getClusterId()
+              + ", serviceName=" + getName()
+              + ", serviceComponentName=" + component.getName());
+        }
+        this.components.put(component.getName(), component);
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
-      this.components.put(component.getName(), component);
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
-
   }
 
   @Override
   public ServiceComponent addServiceComponent(
       String serviceComponentName) throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Adding a ServiceComponent to Service"
-            + ", clusterName=" + cluster.getClusterName()
-            + ", clusterId=" + cluster.getClusterId()
-            + ", serviceName=" + getName()
-            + ", serviceComponentName=" + serviceComponentName);
-      }
-      if (components.containsKey(serviceComponentName)) {
-        throw new AmbariException("Cannot add duplicate ServiceComponent"
-            + ", clusterName=" + cluster.getClusterName()
-            + ", clusterId=" + cluster.getClusterId()
-            + ", serviceName=" + getName()
-            + ", serviceComponentName=" + serviceComponentName);
+      readWriteLock.writeLock().lock();
+      try {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Adding a ServiceComponent to Service"
+              + ", clusterName=" + cluster.getClusterName()
+              + ", clusterId=" + cluster.getClusterId()
+              + ", serviceName=" + getName()
+              + ", serviceComponentName=" + serviceComponentName);
+        }
+        if (components.containsKey(serviceComponentName)) {
+          throw new AmbariException("Cannot add duplicate ServiceComponent"
+              + ", clusterName=" + cluster.getClusterName()
+              + ", clusterId=" + cluster.getClusterId()
+              + ", serviceName=" + getName()
+              + ", serviceComponentName=" + serviceComponentName);
+        }
+        ServiceComponent component = serviceComponentFactory.createNew(this, serviceComponentName);
+        this.components.put(component.getName(), component);
+        return component;
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
-      ServiceComponent component = serviceComponentFactory.createNew(this, serviceComponentName);
-      this.components.put(component.getName(), component);
-      return component;
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
 
+
   }
 
   @Override
   public ServiceComponent getServiceComponent(String componentName)
       throws AmbariException {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (!components.containsKey(componentName)) {
-        throw new ServiceComponentNotFoundException(cluster.getClusterName(),
-            getName(),
-            componentName);
+      readWriteLock.readLock().lock();
+      try {
+        if (!components.containsKey(componentName)) {
+          throw new ServiceComponentNotFoundException(cluster.getClusterName(),
+              getName(),
+              componentName);
+        }
+        return this.components.get(componentName);
+      } finally {
+        readWriteLock.readLock().unlock();
       }
-      return this.components.get(componentName);
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public State getDesiredState() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return this.serviceDesiredStateEntity.getDesiredState();
+      readWriteLock.readLock().lock();
+      try {
+        return this.serviceDesiredStateEntity.getDesiredState();
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public void setDesiredState(State state) {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Setting DesiredState of Service"
-            + ", clusterName=" + cluster.getClusterName()
-            + ", clusterId=" + cluster.getClusterId()
-            + ", serviceName=" + getName()
-            + ", oldDesiredState=" + this.getDesiredState()
-            + ", newDesiredState=" + state);
+      readWriteLock.writeLock().lock();
+      try {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Setting DesiredState of Service"
+              + ", clusterName=" + cluster.getClusterName()
+              + ", clusterId=" + cluster.getClusterId()
+              + ", serviceName=" + getName()
+              + ", oldDesiredState=" + this.getDesiredState()
+              + ", newDesiredState=" + state);
+        }
+        this.serviceDesiredStateEntity.setDesiredState(state);
+        saveIfPersisted();
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
-      this.serviceDesiredStateEntity.setDesiredState(state);
-      saveIfPersisted();
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
-
   }
 
   @Override
   public Map<String, Config> getDesiredConfigs() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      Map<String, Config> map = new HashMap<String, Config>();
-      for (Entry<String, String> entry : desiredConfigs.entrySet()) {
-        Config config = cluster.getConfig(entry.getKey(), entry.getValue());
-        if (null != config) {
-          map.put(entry.getKey(), config);
-        } else {
-          // FIXME this is an error - should throw a proper exception
-          throw new RuntimeException("Found an invalid config"
-              + ", clusterName=" + getCluster().getClusterName()
-              + ", serviceName=" + getName()
-              + ", configType=" + entry.getKey()
-              + ", configVersionTag=" + entry.getValue());
+      readWriteLock.readLock().lock();
+      try {
+        Map<String, Config> map = new HashMap<String, Config>();
+        for (Entry<String, String> entry : desiredConfigs.entrySet()) {
+          Config config = cluster.getConfig(entry.getKey(), entry.getValue());
+          if (null != config) {
+            map.put(entry.getKey(), config);
+          } else {
+            // FIXME this is an error - should throw a proper exception
+            throw new RuntimeException("Found an invalid config"
+                + ", clusterName=" + getCluster().getClusterName()
+                + ", serviceName=" + getName()
+                + ", configType=" + entry.getKey()
+                + ", configVersionTag=" + entry.getValue());
+          }
         }
+        return Collections.unmodifiableMap(map);
+      } finally {
+        readWriteLock.readLock().unlock();
       }
-      return Collections.unmodifiableMap(map);
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
-
   }
 
   @Override
   public void updateDesiredConfigs(Map<String, Config> configs) {
-
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      for (Entry<String, Config> entry : configs.entrySet()) {
-        boolean contains = false;
-
-        for (ServiceConfigMappingEntity serviceConfigMappingEntity : serviceEntity.getServiceConfigMappings()) {
-          if (entry.getKey().equals(serviceConfigMappingEntity.getConfigType())) {
-            contains = true;
-            serviceConfigMappingEntity.setTimestamp(new Date().getTime());
-            serviceConfigMappingEntity.setVersionTag(entry.getValue().getVersionTag());
+      readWriteLock.writeLock().lock();
+      try {
+        for (Entry<String, Config> entry : configs.entrySet()) {
+          boolean contains = false;
+
+          for (ServiceConfigMappingEntity serviceConfigMappingEntity : serviceEntity.getServiceConfigMappings()) {
+            if (entry.getKey().equals(serviceConfigMappingEntity.getConfigType())) {
+              contains = true;
+              serviceConfigMappingEntity.setTimestamp(new Date().getTime());
+              serviceConfigMappingEntity.setVersionTag(entry.getValue().getVersionTag());
+            }
           }
-        }
 
-        if (!contains) {
-          ServiceConfigMappingEntity newEntity = new ServiceConfigMappingEntity();
-          newEntity.setClusterId(serviceEntity.getClusterId());
-          newEntity.setServiceName(serviceEntity.getServiceName());
-          newEntity.setConfigType(entry.getKey());
-          newEntity.setVersionTag(entry.getValue().getVersionTag());
-          newEntity.setTimestamp(new Date().getTime());
-          newEntity.setServiceEntity(serviceEntity);
-          serviceEntity.getServiceConfigMappings().add(newEntity);
+          if (!contains) {
+            ServiceConfigMappingEntity newEntity = new ServiceConfigMappingEntity();
+            newEntity.setClusterId(serviceEntity.getClusterId());
+            newEntity.setServiceName(serviceEntity.getServiceName());
+            newEntity.setConfigType(entry.getKey());
+            newEntity.setVersionTag(entry.getValue().getVersionTag());
+            newEntity.setTimestamp(new Date().getTime());
+            newEntity.setServiceEntity(serviceEntity);
+            serviceEntity.getServiceConfigMappings().add(newEntity);
 
-        }
+          }
 
 
-        this.desiredConfigs.put(entry.getKey(), entry.getValue().getVersionTag());
-      }
+          this.desiredConfigs.put(entry.getKey(), entry.getValue().getVersionTag());
+        }
 
-      saveIfPersisted();
+        saveIfPersisted();
+      } finally {
+        readWriteLock.writeLock().unlock();
+      }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
-
-
   }
 
   @Override
   public StackId getDesiredStackVersion() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return gson.fromJson(serviceDesiredStateEntity.getDesiredStackVersion(), StackId.class);
+      readWriteLock.readLock().lock();
+      try {
+        return gson.fromJson(serviceDesiredStateEntity.getDesiredStackVersion(), StackId.class);
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public void setDesiredStackVersion(StackId stackVersion) {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Setting DesiredStackVersion of Service"
-            + ", clusterName=" + cluster.getClusterName()
-            + ", clusterId=" + cluster.getClusterId()
-            + ", serviceName=" + getName()
-            + ", oldDesiredStackVersion=" + getDesiredStackVersion()
-            + ", newDesiredStackVersion=" + stackVersion);
+      readWriteLock.writeLock().lock();
+      try {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Setting DesiredStackVersion of Service"
+              + ", clusterName=" + cluster.getClusterName()
+              + ", clusterId=" + cluster.getClusterId()
+              + ", serviceName=" + getName()
+              + ", oldDesiredStackVersion=" + getDesiredStackVersion()
+              + ", newDesiredStackVersion=" + stackVersion);
+        }
+        serviceDesiredStateEntity.setDesiredStackVersion(gson.toJson(stackVersion));
+        saveIfPersisted();
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
-      serviceDesiredStateEntity.setDesiredStackVersion(gson.toJson(stackVersion));
-      saveIfPersisted();
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public ServiceResponse convertToResponse() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      ServiceResponse r = new ServiceResponse(cluster.getClusterId(),
-          cluster.getClusterName(),
-          getName(),
-          desiredConfigs,
-          getDesiredStackVersion().getStackId(),
-          getDesiredState().toString());
-      return r;
+      readWriteLock.readLock().lock();
+      try {
+        ServiceResponse r = new ServiceResponse(cluster.getClusterId(),
+            cluster.getClusterName(),
+            getName(),
+            desiredConfigs,
+            getDesiredStackVersion().getStackId(),
+            getDesiredState().toString());
+        return r;
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
-
   }
 
   @Override
@@ -400,69 +484,83 @@ public class ServiceImpl implements Service {
 
   @Override
   public void debugDump(StringBuilder sb) {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      sb.append("Service={ serviceName=" + getName()
-          + ", clusterName=" + cluster.getClusterName()
-          + ", clusterId=" + cluster.getClusterId()
-          + ", desiredStackVersion=" + getDesiredStackVersion()
-          + ", desiredState=" + getDesiredState().toString()
-          + ", configs=[");
-      boolean first = true;
-      if (desiredConfigs != null) {
-        for (Entry<String, String> entry : desiredConfigs.entrySet()) {
+      readWriteLock.readLock().lock();
+      try {
+        sb.append("Service={ serviceName=" + getName()
+            + ", clusterName=" + cluster.getClusterName()
+            + ", clusterId=" + cluster.getClusterId()
+            + ", desiredStackVersion=" + getDesiredStackVersion()
+            + ", desiredState=" + getDesiredState().toString()
+            + ", configs=[");
+        boolean first = true;
+        if (desiredConfigs != null) {
+          for (Entry<String, String> entry : desiredConfigs.entrySet()) {
+            if (!first) {
+              sb.append(" , ");
+            }
+            first = false;
+            sb.append("{ Config type=" + entry.getKey()
+                + ", versionTag=" + entry.getValue() + "}");
+          }
+        }
+        sb.append("], components=[ ");
+
+        first = true;
+        for (ServiceComponent sc : components.values()) {
           if (!first) {
             sb.append(" , ");
           }
           first = false;
-          sb.append("{ Config type=" + entry.getKey()
-              + ", versionTag=" + entry.getValue() + "}");
-        }
-      }
-      sb.append("], components=[ ");
-
-      first = true;
-      for (ServiceComponent sc : components.values()) {
-        if (!first) {
-          sb.append(" , ");
+          sb.append("\n      ");
+          sc.debugDump(sb);
+          sb.append(" ");
         }
-        first = false;
-        sb.append("\n      ");
-        sc.debugDump(sb);
-        sb.append(" ");
+        sb.append(" ] }");
+      } finally {
+        readWriteLock.readLock().unlock();
       }
-      sb.append(" ] }");
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
-
   }
 
   @Override
   public boolean isPersisted() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      return persisted;
+      readWriteLock.readLock().lock();
+      try {
+        return persisted;
+      } finally {
+        readWriteLock.readLock().unlock();
+      }
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
   }
 
   @Override
   public void persist() {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (!persisted) {
-        persistEntities();
-        refresh();
-        cluster.refresh();
-        persisted = true;
-      } else {
-        saveIfPersisted();
+      readWriteLock.writeLock().lock();
+      try {
+        if (!persisted) {
+          persistEntities();
+          refresh();
+          cluster.refresh();
+          persisted = true;
+        } else {
+          saveIfPersisted();
+        }
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
   }
@@ -492,101 +590,123 @@ public class ServiceImpl implements Service {
   @Override
   @Transactional
   public void refresh() {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (isPersisted()) {
-        ClusterServiceEntityPK pk = new ClusterServiceEntityPK();
-        pk.setClusterId(getClusterId());
-        pk.setServiceName(getName());
-        serviceEntity = clusterServiceDAO.findByPK(pk);
-        serviceDesiredStateEntity = serviceEntity.getServiceDesiredStateEntity();
-        clusterServiceDAO.refresh(serviceEntity);
-        serviceDesiredStateDAO.refresh(serviceDesiredStateEntity);
+      readWriteLock.writeLock().lock();
+      try {
+        if (isPersisted()) {
+          ClusterServiceEntityPK pk = new ClusterServiceEntityPK();
+          pk.setClusterId(getClusterId());
+          pk.setServiceName(getName());
+          serviceEntity = clusterServiceDAO.findByPK(pk);
+          serviceDesiredStateEntity = serviceEntity.getServiceDesiredStateEntity();
+          clusterServiceDAO.refresh(serviceEntity);
+          serviceDesiredStateDAO.refresh(serviceDesiredStateEntity);
+        }
+      } finally {
+        readWriteLock.writeLock().unlock();
       }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   public boolean canBeRemoved() {
-    readWriteLock.readLock().lock();
+    clusterGlobalLock.readLock().lock();
     try {
-      if (!getDesiredState().isRemovableState()) {
-        return false;
-      }
-
-      for (ServiceComponent sc : components.values()) {
-        if (!sc.canBeRemoved()) {
-          LOG.warn("Found non removable component when trying to delete service"
-              + ", clusterName=" + cluster.getClusterName()
-              + ", serviceName=" + getName()
-              + ", componentName=" + sc.getName());
+      readWriteLock.readLock().lock();
+      try {
+        if (!getDesiredState().isRemovableState()) {
           return false;
         }
+
+        for (ServiceComponent sc : components.values()) {
+          if (!sc.canBeRemoved()) {
+            LOG.warn("Found non removable component when trying to delete service"
+                + ", clusterName=" + cluster.getClusterName()
+                + ", serviceName=" + getName()
+                + ", componentName=" + sc.getName());
+            return false;
+          }
+        }
+        return true;
+      } finally {
+        readWriteLock.readLock().unlock();
       }
-      return true;
     } finally {
-      readWriteLock.readLock().unlock();
+      clusterGlobalLock.readLock().unlock();
     }
 
+
   }
 
   @Override
   @Transactional
   public void deleteAllComponents() throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      LOG.info("Deleting all components for service"
-          + ", clusterName=" + cluster.getClusterName()
-          + ", serviceName=" + getName());
-      // FIXME check dependencies from meta layer
-      for (ServiceComponent component : components.values()) {
-        if (!component.canBeRemoved()) {
-          throw new AmbariException("Found non removable component when trying to"
-              + " delete all components from service"
-              + ", clusterName=" + cluster.getClusterName()
-              + ", serviceName=" + getName()
-              + ", componentName=" + component.getName());
+      readWriteLock.writeLock().lock();
+      try {
+        LOG.info("Deleting all components for service"
+            + ", clusterName=" + cluster.getClusterName()
+            + ", serviceName=" + getName());
+        // FIXME check dependencies from meta layer
+        for (ServiceComponent component : components.values()) {
+          if (!component.canBeRemoved()) {
+            throw new AmbariException("Found non removable component when trying to"
+                + " delete all components from service"
+                + ", clusterName=" + cluster.getClusterName()
+                + ", serviceName=" + getName()
+                + ", componentName=" + component.getName());
+          }
         }
-      }
 
-      for (ServiceComponent serviceComponent : components.values()) {
-        serviceComponent.delete();
-      }
+        for (ServiceComponent serviceComponent : components.values()) {
+          serviceComponent.delete();
+        }
 
-      components.clear();
+        components.clear();
+      } finally {
+        readWriteLock.writeLock().unlock();
+      }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
-
   }
 
   @Override
   public void deleteServiceComponent(String componentName)
       throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      ServiceComponent component = getServiceComponent(componentName);
-      LOG.info("Deleting servicecomponent for cluster"
-          + ", clusterName=" + cluster.getClusterName()
-          + ", serviceName=" + getName()
-          + ", componentName=" + componentName);
-      // FIXME check dependencies from meta layer
-      if (!component.canBeRemoved()) {
-        throw new AmbariException("Could not delete component from cluster"
+      readWriteLock.writeLock().lock();
+      try {
+        ServiceComponent component = getServiceComponent(componentName);
+        LOG.info("Deleting servicecomponent for cluster"
             + ", clusterName=" + cluster.getClusterName()
             + ", serviceName=" + getName()
             + ", componentName=" + componentName);
-      }
+        // FIXME check dependencies from meta layer
+        if (!component.canBeRemoved()) {
+          throw new AmbariException("Could not delete component from cluster"
+              + ", clusterName=" + cluster.getClusterName()
+              + ", serviceName=" + getName()
+              + ", componentName=" + componentName);
+        }
 
-      component.delete();
-      components.remove(componentName);
+        component.delete();
+        components.remove(componentName);
+      } finally {
+        readWriteLock.writeLock().unlock();
+      }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
 
+
   }
 
   @Override
@@ -597,20 +717,26 @@ public class ServiceImpl implements Service {
   @Override
   @Transactional
   public void delete() throws AmbariException {
-    readWriteLock.writeLock().lock();
+    clusterGlobalLock.writeLock().lock();
     try {
-      deleteAllComponents();
+      readWriteLock.writeLock().lock();
+      try {
+        deleteAllComponents();
 
-      if (persisted) {
-        removeEntities();
-        persisted = false;
-      }
+        if (persisted) {
+          removeEntities();
+          persisted = false;
+        }
 
-      desiredConfigs.clear();
+        desiredConfigs.clear();
+      } finally {
+        readWriteLock.writeLock().unlock();
+      }
     } finally {
-      readWriteLock.writeLock().unlock();
+      clusterGlobalLock.writeLock().unlock();
     }
 
+
   }
 
   @Transactional