AMBARI-9368 - Deadlock Between Dependent Cluster/Service/Component/Host Implementations (jonathanhurley)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/dd572d35 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/dd572d35 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/dd572d35 Branch: refs/heads/branch-1.7.0 Commit: dd572d3544eb6a36c78155ae88ac423a16922d00 Parents: 7566e57 Author: Jonathan Hurley <[email protected]> Authored: Wed Mar 4 20:24:50 2015 -0500 Committer: Jonathan Hurley <[email protected]> Committed: Thu Mar 5 09:24:50 2015 -0500 ---------------------------------------------------------------------- .../server/state/ServiceComponentImpl.java | 302 ++--- .../apache/ambari/server/state/ServiceImpl.java | 263 ++-- .../server/state/cluster/ClusterImpl.java | 1272 +++++++----------- .../svccomphost/ServiceComponentHostImpl.java | 652 ++++----- .../state/cluster/ClusterDeadlockTest.java | 357 +++++ ambari-web/package.json | 2 +- 6 files changed, 1292 insertions(+), 1556 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/dd572d35/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java index 45ea1f9..de0943b 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentImpl.java @@ -18,12 +18,12 @@ package org.apache.ambari.server.state; -import com.google.gson.Gson; -import com.google.inject.Inject; -import com.google.inject.Injector; -import com.google.inject.assistedinject.Assisted; -import com.google.inject.assistedinject.AssistedInject; -import com.google.inject.persist.Transactional; +import java.util.HashMap; +import java.util.Map; +import java.util.Map.Entry; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; + import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.ServiceComponentHostNotFoundException; import org.apache.ambari.server.api.services.AmbariMetaInfo; @@ -42,11 +42,12 @@ import org.apache.ambari.server.state.cluster.ClusterImpl; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.HashMap; -import java.util.Map; -import java.util.Map.Entry; -import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock; +import com.google.gson.Gson; +import com.google.inject.Inject; +import com.google.inject.Injector; +import com.google.inject.assistedinject.Assisted; +import com.google.inject.assistedinject.AssistedInject; +import com.google.inject.persist.Transactional; public class ServiceComponentImpl implements ServiceComponent { @@ -77,15 +78,15 @@ public class ServiceComponentImpl implements ServiceComponent { public ServiceComponentImpl(@Assisted Service service, @Assisted String componentName, Injector injector) throws AmbariException { injector.injectMembers(this); - this.clusterGlobalLock = service.getClusterGlobalLock(); + clusterGlobalLock = service.getClusterGlobalLock(); this.service = service; - this.desiredStateEntity = new ServiceComponentDesiredStateEntity(); + desiredStateEntity = new ServiceComponentDesiredStateEntity(); desiredStateEntity.setComponentName(componentName); desiredStateEntity.setDesiredState(State.INIT); setDesiredStackVersion(service.getDesiredStackVersion()); - this.hostComponents = new HashMap<String, ServiceComponentHost>(); + hostComponents = new HashMap<String, ServiceComponentHost>(); StackId stackId = service.getDesiredStackVersion(); ComponentInfo compInfo = ambariMetaInfo.getComponentCategory( @@ -99,10 +100,8 @@ public class ServiceComponentImpl implements ServiceComponent { + ", componentName=" + componentName + ", stackInfo=" + stackId.getStackId()); } - this.isClientComponent = compInfo.isClient(); - this.isMasterComponent = compInfo.isMaster(); - - init(); + isClientComponent = compInfo.isClient(); + isMasterComponent = compInfo.isMaster(); } @AssistedInject @@ -110,11 +109,11 @@ public class ServiceComponentImpl implements ServiceComponent { @Assisted ServiceComponentDesiredStateEntity serviceComponentDesiredStateEntity, Injector injector) throws AmbariException { injector.injectMembers(this); - this.clusterGlobalLock = service.getClusterGlobalLock(); + clusterGlobalLock = service.getClusterGlobalLock(); this.service = service; - this.desiredStateEntity = serviceComponentDesiredStateEntity; + desiredStateEntity = serviceComponentDesiredStateEntity; - this.hostComponents = new HashMap<String, ServiceComponentHost>(); + hostComponents = new HashMap<String, ServiceComponentHost>(); for (HostComponentStateEntity hostComponentStateEntity : desiredStateEntity.getHostComponentStateEntities()) { HostComponentDesiredStateEntityPK pk = new HostComponentDesiredStateEntityPK(); pk.setClusterId(hostComponentStateEntity.getClusterId()); @@ -141,17 +140,12 @@ public class ServiceComponentImpl implements ServiceComponent { + ", componentName=" + getName() + ", stackInfo=" + stackId.getStackId()); } - this.isClientComponent = compInfo.isClient(); - this.isMasterComponent = compInfo.isMaster(); + isClientComponent = compInfo.isClient(); + isMasterComponent = compInfo.isMaster(); persisted = true; } - private void init() { - // TODO load during restart - // initialize from DB - } - @Override public ReadWriteLock getClusterGlobalLock() { return clusterGlobalLock; @@ -159,47 +153,17 @@ public class ServiceComponentImpl implements ServiceComponent { @Override public String getName() { - clusterGlobalLock.readLock().lock(); - try { - readWriteLock.readLock().lock(); - try { - return desiredStateEntity.getComponentName(); - } finally { - readWriteLock.readLock().unlock(); - } - } finally { - clusterGlobalLock.readLock().unlock(); - } + return desiredStateEntity.getComponentName(); } @Override public String getServiceName() { - clusterGlobalLock.readLock().lock(); - try { - readWriteLock.readLock().lock(); - try { - return service.getName(); - } finally { - readWriteLock.readLock().unlock(); - } - } finally { - clusterGlobalLock.readLock().unlock(); - } + return service.getName(); } @Override public long getClusterId() { - clusterGlobalLock.readLock().lock(); - try { - readWriteLock.readLock().lock(); - try { - return this.service.getClusterId(); - } finally { - readWriteLock.readLock().unlock(); - } - } finally { - clusterGlobalLock.readLock().unlock(); - } + return service.getClusterId(); } @Override @@ -271,7 +235,7 @@ public class ServiceComponentImpl implements ServiceComponent { // FIXME need a better approach of caching components by host ClusterImpl clusterImpl = (ClusterImpl) service.getCluster(); clusterImpl.addServiceComponentHost(hostComponent); - this.hostComponents.put(hostComponent.getHostName(), hostComponent); + hostComponents.put(hostComponent.getHostName(), hostComponent); } finally { readWriteLock.writeLock().unlock(); } @@ -310,7 +274,7 @@ public class ServiceComponentImpl implements ServiceComponent { ClusterImpl clusterImpl = (ClusterImpl) service.getCluster(); clusterImpl.addServiceComponentHost(hostComponent); - this.hostComponents.put(hostComponent.getHostName(), hostComponent); + hostComponents.put(hostComponent.getHostName(), hostComponent); return hostComponent; } finally { @@ -332,7 +296,7 @@ public class ServiceComponentImpl implements ServiceComponent { throw new ServiceComponentHostNotFoundException(getClusterName(), getServiceName(), getName(), hostname); } - return this.hostComponents.get(hostname); + return hostComponents.get(hostname); } finally { readWriteLock.readLock().unlock(); } @@ -343,149 +307,105 @@ public class ServiceComponentImpl implements ServiceComponent { @Override public State getDesiredState() { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - readWriteLock.readLock().lock(); - try { - return desiredStateEntity.getDesiredState(); - } finally { - readWriteLock.readLock().unlock(); - } + return desiredStateEntity.getDesiredState(); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } } @Override public void setDesiredState(State state) { - clusterGlobalLock.readLock().lock(); + readWriteLock.writeLock().lock(); try { - 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(); + 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 { - clusterGlobalLock.readLock().unlock(); + readWriteLock.writeLock().unlock(); } } @Override public StackId getDesiredStackVersion() { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - readWriteLock.readLock().lock(); - try { - return gson.fromJson(desiredStateEntity.getDesiredStackVersion(), StackId.class); - } finally { - readWriteLock.readLock().unlock(); - } + return gson.fromJson(desiredStateEntity.getDesiredStackVersion(), + StackId.class); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } } @Override public void setDesiredStackVersion(StackId stackVersion) { - clusterGlobalLock.readLock().lock(); + readWriteLock.writeLock().lock(); try { - 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(); + 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 { - clusterGlobalLock.readLock().unlock(); + readWriteLock.writeLock().unlock(); } } @Override public ServiceComponentResponse convertToResponse() { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - readWriteLock.readLock().lock(); - try { - ServiceComponentResponse r = new ServiceComponentResponse( - getClusterId(), service.getCluster().getClusterName(), - service.getName(), getName(), - getDesiredStackVersion().getStackId(), - getDesiredState().toString(), getTotalCount(), getStartedCount(), - getInstalledCount()); - return r; - } finally { - readWriteLock.readLock().unlock(); - } + ServiceComponentResponse r = new ServiceComponentResponse(getClusterId(), + service.getCluster().getClusterName(), service.getName(), getName(), + getDesiredStackVersion().getStackId(), getDesiredState().toString(), + getTotalCount(), getStartedCount(), getInstalledCount()); + return r; } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } } @Override public String getClusterName() { - clusterGlobalLock.readLock().lock(); - try { - readWriteLock.readLock().lock(); - try { - return service.getCluster().getClusterName(); - } finally { - readWriteLock.readLock().unlock(); - } - } finally { - clusterGlobalLock.readLock().unlock(); - } + return service.getCluster().getClusterName(); } @Override public void debugDump(StringBuilder sb) { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - 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("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(" ] }"); - } finally { - readWriteLock.readLock().unlock(); + sb.append("\n "); + sch.debugDump(sb); + sb.append(" "); } + sb.append(" ] }"); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } } @@ -506,12 +426,22 @@ public class ServiceComponentImpl implements ServiceComponent { @Override public void persist() { - clusterGlobalLock.readLock().lock(); + boolean clusterWriteLockAcquired = false; + if (!persisted) { + clusterGlobalLock.writeLock().lock(); + clusterWriteLockAcquired = true; + } + try { readWriteLock.writeLock().lock(); try { if (!persisted) { + // persist the new cluster topology and then release the cluster lock + // as it has no more bearing on the rest of this persist() method persistEntities(); + clusterGlobalLock.writeLock().unlock(); + clusterWriteLockAcquired = false; + refresh(); service.refresh(); persisted = true; @@ -522,7 +452,9 @@ public class ServiceComponentImpl implements ServiceComponent { readWriteLock.writeLock().unlock(); } } finally { - clusterGlobalLock.readLock().unlock(); + if (clusterWriteLockAcquired) { + clusterGlobalLock.writeLock().unlock(); + } } } @@ -541,52 +473,42 @@ public class ServiceComponentImpl implements ServiceComponent { @Override @Transactional public void refresh() { - clusterGlobalLock.readLock().lock(); + readWriteLock.writeLock().lock(); try { - 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(); + 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 { - clusterGlobalLock.readLock().unlock(); + readWriteLock.writeLock().unlock(); } } @Transactional private void saveIfPersisted() { - clusterGlobalLock.readLock().lock(); + readWriteLock.writeLock().lock(); try { - readWriteLock.writeLock().lock(); - try { - if (isPersisted()) { - serviceComponentDesiredStateDAO.merge(desiredStateEntity); - } - } finally { - readWriteLock.writeLock().unlock(); + if (isPersisted()) { + serviceComponentDesiredStateDAO.merge(desiredStateEntity); } } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.writeLock().unlock(); } } @Override public boolean isClientComponent() { - return this.isClientComponent; + return isClientComponent; } @Override public boolean isMasterComponent() { - return this.isMasterComponent; + return isMasterComponent; } @Override http://git-wip-us.apache.org/repos/asf/ambari/blob/dd572d35/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java index 6279c2b..b110014 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java @@ -18,7 +18,6 @@ package org.apache.ambari.server.state; -import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.concurrent.locks.ReadWriteLock; @@ -94,14 +93,14 @@ public class ServiceImpl implements Service { this.cluster = cluster; - this.components = new HashMap<String, ServiceComponent>(); + components = new HashMap<String, ServiceComponent>(); StackId stackId = cluster.getDesiredStackVersion(); setDesiredStackVersion(stackId); ServiceInfo sInfo = ambariMetaInfo.getServiceInfo(stackId.getStackName(), stackId.getStackVersion(), serviceName); - this.isClientOnlyService = sInfo.isClientOnlyService(); + isClientOnlyService = sInfo.isClientOnlyService(); init(); } @@ -115,9 +114,9 @@ public class ServiceImpl implements Service { this.cluster = cluster; //TODO check for null states? - this.serviceDesiredStateEntity = serviceEntity.getServiceDesiredStateEntity(); + serviceDesiredStateEntity = serviceEntity.getServiceDesiredStateEntity(); - this.components = new HashMap<String, ServiceComponent>(); + components = new HashMap<String, ServiceComponent>(); if (!serviceEntity.getServiceComponentDesiredStateEntities().isEmpty()) { for (ServiceComponentDesiredStateEntity serviceComponentDesiredStateEntity @@ -139,7 +138,7 @@ public class ServiceImpl implements Service { StackId stackId = getDesiredStackVersion(); ServiceInfo sInfo = ambariMetaInfo.getServiceInfo(stackId.getStackName(), stackId.getStackVersion(), getName()); - this.isClientOnlyService = sInfo.isClientOnlyService(); + isClientOnlyService = sInfo.isClientOnlyService(); persisted = true; } @@ -151,46 +150,21 @@ public class ServiceImpl implements Service { @Override public String getName() { - clusterGlobalLock.readLock().lock(); - try { - readWriteLock.readLock().lock(); - try { - return serviceEntity.getServiceName(); - } finally { - readWriteLock.readLock().unlock(); - } - } finally { - clusterGlobalLock.readLock().unlock(); - } + return serviceEntity.getServiceName(); } @Override public long getClusterId() { - clusterGlobalLock.readLock().lock(); - try { - readWriteLock.readLock().lock(); - try { - return cluster.getClusterId(); - } finally { - readWriteLock.readLock().unlock(); - } - } finally { - clusterGlobalLock.readLock().unlock(); - } + return cluster.getClusterId(); } @Override public Map<String, ServiceComponent> getServiceComponents() { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - readWriteLock.readLock().lock(); - try { - return new HashMap<String, ServiceComponent>(components); - } finally { - readWriteLock.readLock().unlock(); - } + return new HashMap<String, ServiceComponent>(components); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } } @@ -234,7 +208,7 @@ public class ServiceImpl implements Service { + ", serviceName=" + getName() + ", serviceComponentName=" + component.getName()); } - this.components.put(component.getName(), component); + components.put(component.getName(), component); } finally { readWriteLock.writeLock().unlock(); } @@ -265,7 +239,7 @@ public class ServiceImpl implements Service { + ", serviceComponentName=" + serviceComponentName); } ServiceComponent component = serviceComponentFactory.createNew(this, serviceComponentName); - this.components.put(component.getName(), component); + components.put(component.getName(), component); return component; } finally { readWriteLock.writeLock().unlock(); @@ -273,134 +247,92 @@ public class ServiceImpl implements Service { } finally { clusterGlobalLock.writeLock().unlock(); } - - } @Override public ServiceComponent getServiceComponent(String componentName) throws AmbariException { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - readWriteLock.readLock().lock(); - try { - if (!components.containsKey(componentName)) { - throw new ServiceComponentNotFoundException(cluster.getClusterName(), - getName(), - componentName); - } - return this.components.get(componentName); - } finally { - readWriteLock.readLock().unlock(); + if (!components.containsKey(componentName)) { + throw new ServiceComponentNotFoundException(cluster.getClusterName(), + getName(), componentName); } + return components.get(componentName); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } - - } @Override public State getDesiredState() { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - readWriteLock.readLock().lock(); - try { - return this.serviceDesiredStateEntity.getDesiredState(); - } finally { - readWriteLock.readLock().unlock(); - } + return serviceDesiredStateEntity.getDesiredState(); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } - - } @Override public void setDesiredState(State state) { - clusterGlobalLock.readLock().lock(); + readWriteLock.writeLock().lock(); try { - 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(); + if (LOG.isDebugEnabled()) { + LOG.debug("Setting DesiredState of Service" + ", clusterName=" + + cluster.getClusterName() + ", clusterId=" + + cluster.getClusterId() + ", serviceName=" + getName() + + ", oldDesiredState=" + getDesiredState() + ", newDesiredState=" + + state); } + serviceDesiredStateEntity.setDesiredState(state); + saveIfPersisted(); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.writeLock().unlock(); } } @Override public StackId getDesiredStackVersion() { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - readWriteLock.readLock().lock(); - try { - return gson.fromJson(serviceDesiredStateEntity.getDesiredStackVersion(), StackId.class); - } finally { - readWriteLock.readLock().unlock(); - } + return gson.fromJson(serviceDesiredStateEntity.getDesiredStackVersion(), + StackId.class); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } } @Override public void setDesiredStackVersion(StackId stackVersion) { - clusterGlobalLock.readLock().lock(); + readWriteLock.writeLock().lock(); try { - 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(); + 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 { - clusterGlobalLock.readLock().unlock(); + readWriteLock.writeLock().unlock(); } - - } @Override public ServiceResponse convertToResponse() { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - readWriteLock.readLock().lock(); - try { - ServiceResponse r = new ServiceResponse(cluster.getClusterId(), - cluster.getClusterName(), - getName(), - getDesiredStackVersion().getStackId(), - getDesiredState().toString()); - - r.setMaintenanceState(getMaintenanceState().name()); - return r; - } finally { - readWriteLock.readLock().unlock(); - } + ServiceResponse r = new ServiceResponse(cluster.getClusterId(), + cluster.getClusterName(), getName(), + getDesiredStackVersion().getStackId(), getDesiredState().toString()); + + r.setMaintenanceState(getMaintenanceState().name()); + return r; } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } } @@ -411,32 +343,26 @@ public class ServiceImpl implements Service { @Override public void debugDump(StringBuilder sb) { - clusterGlobalLock.readLock().lock(); + readWriteLock.readLock().lock(); try { - readWriteLock.readLock().lock(); - try { - sb.append("Service={ serviceName=" + getName() - + ", clusterName=" + cluster.getClusterName() - + ", clusterId=" + cluster.getClusterId() - + ", desiredStackVersion=" + getDesiredStackVersion() - + ", desiredState=" + getDesiredState().toString() - + ", components=[ "); - boolean first = true; - for (ServiceComponent sc : components.values()) { - if (!first) { - sb.append(" , "); - } - first = false; - sb.append("\n "); - sc.debugDump(sb); - sb.append(" "); + sb.append("Service={ serviceName=" + getName() + ", clusterName=" + + cluster.getClusterName() + ", clusterId=" + cluster.getClusterId() + + ", desiredStackVersion=" + getDesiredStackVersion() + + ", desiredState=" + getDesiredState().toString() + + ", components=[ "); + boolean first = true; + for (ServiceComponent sc : components.values()) { + if (!first) { + sb.append(" , "); } - sb.append(" ] }"); - } finally { - readWriteLock.readLock().unlock(); + first = false; + sb.append("\n "); + sc.debugDump(sb); + sb.append(" "); } + sb.append(" ] }"); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.readLock().unlock(); } } @@ -453,12 +379,11 @@ public class ServiceImpl implements Service { } finally { clusterGlobalLock.readLock().unlock(); } - } @Override public void persist() { - clusterGlobalLock.readLock().lock(); + clusterGlobalLock.writeLock().lock(); try { readWriteLock.writeLock().lock(); try { @@ -474,9 +399,8 @@ public class ServiceImpl implements Service { readWriteLock.writeLock().unlock(); } } finally { - clusterGlobalLock.readLock().unlock(); + clusterGlobalLock.writeLock().unlock(); } - } @Transactional @@ -504,27 +428,20 @@ public class ServiceImpl implements Service { @Override @Transactional public void refresh() { - clusterGlobalLock.readLock().lock(); + readWriteLock.writeLock().lock(); try { - 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(); + 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 { - clusterGlobalLock.readLock().unlock(); + readWriteLock.writeLock().unlock(); } - - } @Override @@ -659,26 +576,20 @@ public class ServiceImpl implements Service { clusterServiceDAO.removeByPK(pk); } - + @Override public void setMaintenanceState(MaintenanceState state) { - clusterGlobalLock.readLock().lock(); + readWriteLock.writeLock().lock(); try { - try { - readWriteLock.writeLock().lock(); - serviceDesiredStateEntity.setMaintenanceState(state); - saveIfPersisted(); - } finally { - readWriteLock.writeLock().unlock(); - } + serviceDesiredStateEntity.setMaintenanceState(state); + saveIfPersisted(); } finally { - clusterGlobalLock.readLock().unlock(); + readWriteLock.writeLock().unlock(); } } - + @Override public MaintenanceState getMaintenanceState() { return serviceDesiredStateEntity.getMaintenanceState(); } - }
