AMBARI-9918 - Caching of JPA entities causes SECONDARY_NAMENODE component to be re-persisted after NN HA enabling (tbeerbower)

ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java

@@ -105,8 +105,10 @@ public class HostImpl implements Host {
   private final Lock readLock;
   private final Lock writeLock;
 
+  // TODO : caching the JPA entities here causes issues if they become stale and get re-merged.
   private HostEntity hostEntity;
   private HostStateEntity hostStateEntity;
+
   private HostDAO hostDAO;
   private HostStateDAO hostStateDAO;
   private HostVersionDAO hostVersionDAO;
@@ -374,7 +376,7 @@ public class HostImpl implements Host {
   }
 
   /**
-   * @param hostInfo
+   * @param hostInfo  the host information
    */
   @Override
   public void importHostInfo(HostInfo hostInfo) {
@@ -520,6 +522,9 @@ public class HostImpl implements Host {
     try {
       writeLock.lock();
       stateMachine.setCurrentState(state);
+
+      HostStateEntity hostStateEntity = getHostStateEntity();
+
       hostStateEntity.setCurrentState(state);
       hostStateEntity.setTimeInState(System.currentTimeMillis());
       saveIfPersisted();
@@ -611,7 +616,7 @@ public class HostImpl implements Host {
   public void setPublicHostName(String hostName) {
     try {
       writeLock.lock();
-      hostEntity.setPublicHostName(hostName);
+      getHostEntity().setPublicHostName(hostName);
       saveIfPersisted();
     }
     finally {
@@ -623,7 +628,7 @@ public class HostImpl implements Host {
   public String getPublicHostName() {
     try {
       readLock.lock();
-      return hostEntity.getPublicHostName();
+      return getHostEntity().getPublicHostName();
     }
     finally {
       readLock.unlock();
@@ -634,7 +639,7 @@ public class HostImpl implements Host {
   public String getIPv4() {
     try {
       readLock.lock();
-      return hostEntity.getIpv4();
+      return getHostEntity().getIpv4();
     } finally {
       readLock.unlock();
     }
@@ -644,7 +649,7 @@ public class HostImpl implements Host {
   public void setIPv4(String ip) {
     try {
       writeLock.lock();
-      hostEntity.setIpv4(ip);
+      getHostEntity().setIpv4(ip);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -655,7 +660,7 @@ public class HostImpl implements Host {
   public String getIPv6() {
     try {
       readLock.lock();
-      return hostEntity.getIpv6();
+      return getHostEntity().getIpv6();
     } finally {
       readLock.unlock();
     }
@@ -665,7 +670,7 @@ public class HostImpl implements Host {
   public void setIPv6(String ip) {
     try {
       writeLock.lock();
-      hostEntity.setIpv6(ip);
+      getHostEntity().setIpv6(ip);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -676,7 +681,7 @@ public class HostImpl implements Host {
   public int getCpuCount() {
     try {
       readLock.lock();
-      return hostEntity.getCpuCount();
+      return getHostEntity().getCpuCount();
     } finally {
       readLock.unlock();
     }
@@ -686,7 +691,7 @@ public class HostImpl implements Host {
   public void setCpuCount(int cpuCount) {
     try {
       writeLock.lock();
-      hostEntity.setCpuCount(cpuCount);
+      getHostEntity().setCpuCount(cpuCount);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -697,7 +702,7 @@ public class HostImpl implements Host {
   public int getPhCpuCount() {
     try {
       readLock.lock();
-      return hostEntity.getPhCpuCount();
+      return getHostEntity().getPhCpuCount();
     } finally {
       readLock.unlock();
     }
@@ -707,7 +712,7 @@ public class HostImpl implements Host {
   public void setPhCpuCount(int phCpuCount) {
     try {
       writeLock.lock();
-      hostEntity.setPhCpuCount(phCpuCount);
+      getHostEntity().setPhCpuCount(phCpuCount);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -719,7 +724,7 @@ public class HostImpl implements Host {
   public long getTotalMemBytes() {
     try {
       readLock.lock();
-      return hostEntity.getTotalMem();
+      return getHostEntity().getTotalMem();
     } finally {
       readLock.unlock();
     }
@@ -729,7 +734,7 @@ public class HostImpl implements Host {
   public void setTotalMemBytes(long totalMemBytes) {
     try {
       writeLock.lock();
-      hostEntity.setTotalMem(totalMemBytes);
+      getHostEntity().setTotalMem(totalMemBytes);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -740,7 +745,7 @@ public class HostImpl implements Host {
   public long getAvailableMemBytes() {
     try {
       readLock.lock();
-      return hostStateEntity.getAvailableMem();
+      return getHostStateEntity().getAvailableMem();
     }
     finally {
       readLock.unlock();
@@ -751,7 +756,7 @@ public class HostImpl implements Host {
   public void setAvailableMemBytes(long availableMemBytes) {
     try {
       writeLock.lock();
-      hostStateEntity.setAvailableMem(availableMemBytes);
+      getHostStateEntity().setAvailableMem(availableMemBytes);
       saveIfPersisted();
     }
     finally {
@@ -763,7 +768,7 @@ public class HostImpl implements Host {
   public String getOsArch() {
     try {
       readLock.lock();
-      return hostEntity.getOsArch();
+      return getHostEntity().getOsArch();
     } finally {
       readLock.unlock();
     }
@@ -773,7 +778,7 @@ public class HostImpl implements Host {
   public void setOsArch(String osArch) {
     try {
       writeLock.lock();
-      hostEntity.setOsArch(osArch);
+      getHostEntity().setOsArch(osArch);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -784,7 +789,7 @@ public class HostImpl implements Host {
   public String getOsInfo() {
     try {
       readLock.lock();
-      return hostEntity.getOsInfo();
+      return getHostEntity().getOsInfo();
     } finally {
       readLock.unlock();
     }
@@ -794,7 +799,7 @@ public class HostImpl implements Host {
   public void setOsInfo(String osInfo) {
     try {
       writeLock.lock();
-      hostEntity.setOsInfo(osInfo);
+      getHostEntity().setOsInfo(osInfo);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -805,7 +810,7 @@ public class HostImpl implements Host {
   public String getOsType() {
     try {
       readLock.lock();
-      return hostEntity.getOsType();
+      return getHostEntity().getOsType();
     } finally {
       readLock.unlock();
     }
@@ -815,7 +820,7 @@ public class HostImpl implements Host {
   public void setOsType(String osType) {
     try {
       writeLock.lock();
-      hostEntity.setOsType(osType);
+      getHostEntity().setOsType(osType);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -852,7 +857,7 @@ public class HostImpl implements Host {
   public HostHealthStatus getHealthStatus() {
     try {
       readLock.lock();
-      return gson.fromJson(hostStateEntity.getHealthStatus(),
+      return gson.fromJson(getHostStateEntity().getHealthStatus(),
           HostHealthStatus.class);
     } finally {
       readLock.unlock();
@@ -863,7 +868,7 @@ public class HostImpl implements Host {
   public void setHealthStatus(HostHealthStatus healthStatus) {
     try {
       writeLock.lock();
-      hostStateEntity.setHealthStatus(gson.toJson(healthStatus));
+      getHostStateEntity().setHealthStatus(gson.toJson(healthStatus));
 
       if (healthStatus.getHealthStatus().equals(HealthStatus.UNKNOWN)) {
         setStatus(HealthStatus.UNKNOWN.name());
@@ -894,7 +899,7 @@ public class HostImpl implements Host {
   public Map<String, String> getHostAttributes() {
     try {
       readLock.lock();
-      return gson.<Map<String, String>>fromJson(hostEntity.getHostAttributes(),
+      return gson.fromJson(getHostEntity().getHostAttributes(),
           hostAttributesType);
     } finally {
       readLock.unlock();
@@ -905,14 +910,13 @@ public class HostImpl implements Host {
   public void setHostAttributes(Map<String, String> hostAttributes) {
     try {
       writeLock.lock();
-      Map<String, String> hostAttrs = gson.<Map<String, String>>
-          fromJson(hostEntity.getHostAttributes(), hostAttributesType);
+      HostEntity hostEntity = getHostEntity();
+      Map<String, String> hostAttrs = gson.fromJson(hostEntity.getHostAttributes(), hostAttributesType);
       if (hostAttrs == null) {
         hostAttrs = new HashMap<String, String>();
       }
       hostAttrs.putAll(hostAttributes);
-      hostEntity.setHostAttributes(gson.toJson(hostAttrs,
-          hostAttributesType));
+      hostEntity.setHostAttributes(gson.toJson(hostAttrs,hostAttributesType));
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -923,7 +927,7 @@ public class HostImpl implements Host {
   public String getRackInfo() {
     try {
       readLock.lock();
-      return hostEntity.getRackInfo();
+      return getHostEntity().getRackInfo();
     } finally {
       readLock.unlock();
     }
@@ -933,7 +937,7 @@ public class HostImpl implements Host {
   public void setRackInfo(String rackInfo) {
     try {
       writeLock.lock();
-      hostEntity.setRackInfo(rackInfo);
+      getHostEntity().setRackInfo(rackInfo);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -944,7 +948,7 @@ public class HostImpl implements Host {
   public long getLastRegistrationTime() {
     try {
       readLock.lock();
-      return hostEntity.getLastRegistrationTime();
+      return getHostEntity().getLastRegistrationTime();
     } finally {
       readLock.unlock();
     }
@@ -954,7 +958,7 @@ public class HostImpl implements Host {
   public void setLastRegistrationTime(long lastRegistrationTime) {
     try {
       writeLock.lock();
-      hostEntity.setLastRegistrationTime(lastRegistrationTime);
+      getHostEntity().setLastRegistrationTime(lastRegistrationTime);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -987,7 +991,7 @@ public class HostImpl implements Host {
   public AgentVersion getAgentVersion() {
     try {
       readLock.lock();
-      return gson.fromJson(hostStateEntity.getAgentVersion(),
+      return gson.fromJson(getHostStateEntity().getAgentVersion(),
           AgentVersion.class);
     }
     finally {
@@ -999,7 +1003,7 @@ public class HostImpl implements Host {
   public void setAgentVersion(AgentVersion agentVersion) {
     try {
       writeLock.lock();
-      hostStateEntity.setAgentVersion(gson.toJson(agentVersion));
+      getHostStateEntity().setAgentVersion(gson.toJson(agentVersion));
       saveIfPersisted();
     }
     finally {
@@ -1009,14 +1013,14 @@ public class HostImpl implements Host {
 
   @Override
   public long getTimeInState() {
-    return hostStateEntity.getTimeInState();
+    return getHostStateEntity().getTimeInState();
   }
 
   @Override
   public void setTimeInState(long timeInState) {
     try {
       writeLock.lock();
-      hostStateEntity.setTimeInState(timeInState);
+      getHostStateEntity().setTimeInState(timeInState);
       saveIfPersisted();
     }
     finally {
@@ -1135,12 +1139,7 @@ public class HostImpl implements Host {
   public void refresh() {
     writeLock.lock();
     try {
-      if (isPersisted()) {
-        hostEntity = hostDAO.findByName(hostEntity.getHostName());
-        hostStateEntity = hostEntity.getHostStateEntity();
-        hostDAO.refresh(hostEntity);
-        hostStateDAO.refresh(hostStateEntity);
-      }
+      getHostEntity();
     } finally {
       writeLock.unlock();
     }
@@ -1172,6 +1171,8 @@ public class HostImpl implements Host {
 
     writeLock.lock();
 
+    HostEntity hostEntity = getHostEntity();
+
     try {
       // set all old mappings for this type to empty
       for (HostConfigMapping e : hostConfigMappingDAO.findByType(clusterId,
@@ -1181,8 +1182,8 @@ public class HostImpl implements Host {
       }
 
       HostConfigMapping hostConfigMapping = new HostConfigMappingImpl();
-      hostConfigMapping.setClusterId(Long.valueOf(clusterId));
-      hostConfigMapping.setCreateTimestamp(Long.valueOf(System.currentTimeMillis()));
+      hostConfigMapping.setClusterId(clusterId);
+      hostConfigMapping.setCreateTimestamp(System.currentTimeMillis());
       hostConfigMapping.setHostName(hostEntity.getHostName());
       hostConfigMapping.setSelected(1);
       hostConfigMapping.setUser(user);
@@ -1220,7 +1221,8 @@ public class HostImpl implements Host {
   /**
    * Get a map of configType with all applicable config tags.
    *
-   * @param cluster
+   * @param cluster  the cluster
+   *
    * @return Map of configType -> HostConfig
    */
   @Override
@@ -1263,16 +1265,12 @@ public class HostImpl implements Host {
   }
 
   private HostConfigMapping getDesiredConfigEntity(long clusterId, String type) {
-    HostConfigMapping findSelectedByType = hostConfigMappingDAO.findSelectedByType(clusterId,
-        hostEntity.getHostName(), type);
-
-
-    return findSelectedByType;
+    return hostConfigMappingDAO.findSelectedByType(clusterId, hostEntity.getHostName(), type);
   }
 
   private void ensureMaintMap() {
     if (null == maintMap) {
-      String entity = hostStateEntity.getMaintenanceState();
+      String entity = getHostStateEntity().getMaintenanceState();
       if (null == entity) {
         maintMap = new HashMap<Long, MaintenanceState>();
       } else {
@@ -1292,10 +1290,10 @@ public class HostImpl implements Host {
 
       ensureMaintMap();
 
-      maintMap.put(Long.valueOf(clusterId), state);
+      maintMap.put(clusterId, state);
       String json = gson.toJson(maintMap, maintMapType);
 
-      hostStateEntity.setMaintenanceState(json);
+      getHostStateEntity().setMaintenanceState(json);
       saveIfPersisted();
 
       // broadcast the maintenance mode change
@@ -1313,13 +1311,11 @@ public class HostImpl implements Host {
 
       ensureMaintMap();
 
-      Long id = Long.valueOf(clusterId);
-
-      if (!maintMap.containsKey(id)) {
-        maintMap.put(id, MaintenanceState.OFF);
+      if (!maintMap.containsKey(clusterId)) {
+        maintMap.put(clusterId, MaintenanceState.OFF);
       }
 
-      return maintMap.get(id);
+      return maintMap.get(clusterId);
     } finally {
       readLock.unlock();
     }
@@ -1327,13 +1323,29 @@ public class HostImpl implements Host {
 
   /**
    * Get all of the HostVersionEntity objects for the host.
-   * @return
+   *
+   * @return all of the HostVersionEntity objects for the host
    */
   @Override
   public List<HostVersionEntity> getAllHostVersions() {
     return hostVersionDAO.findByHost(this.getHostName());
   }
 
+  // Get the cached host entity or load it fresh through the DAO.
+  public HostEntity getHostEntity() {
+    if (isPersisted()) {
+      hostEntity = hostDAO.findByName(hostEntity.getHostName());
+    }
+    return hostEntity;
+  }
+
+  // Get the cached host state entity or load it fresh through the DAO.
+  public HostStateEntity getHostStateEntity() {
+    if (isPersisted()) {
+      hostStateEntity = getHostEntity().getHostStateEntity();
+    }
+    return hostStateEntity;
+  }
 }
 
 

ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java

@@ -38,11 +38,9 @@ import org.apache.ambari.server.events.MaintenanceModeEvent;
 import org.apache.ambari.server.events.ServiceComponentInstalledEvent;
 import org.apache.ambari.server.events.ServiceComponentUninstalledEvent;
 import org.apache.ambari.server.events.publishers.AmbariEventPublisher;
-import org.apache.ambari.server.orm.dao.ClusterVersionDAO;
 import org.apache.ambari.server.orm.dao.HostComponentDesiredStateDAO;
 import org.apache.ambari.server.orm.dao.HostComponentStateDAO;
 import org.apache.ambari.server.orm.dao.HostDAO;
-import org.apache.ambari.server.orm.dao.HostVersionDAO;
 import org.apache.ambari.server.orm.dao.RepositoryVersionDAO;
 import org.apache.ambari.server.orm.dao.ServiceComponentDesiredStateDAO;
 import org.apache.ambari.server.orm.entities.HostComponentDesiredStateEntity;
@@ -106,16 +104,12 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   @Inject
   Gson gson;
   @Inject
-  ClusterVersionDAO clusterVersionDAO;
-  @Inject
   HostComponentStateDAO hostComponentStateDAO;
   @Inject
   HostComponentDesiredStateDAO hostComponentDesiredStateDAO;
   @Inject
   HostDAO hostDAO;
   @Inject
-  HostVersionDAO hostVersionDAO;
-  @Inject
   RepositoryVersionDAO repositoryVersionDAO;
   @Inject
   ServiceComponentDesiredStateDAO serviceComponentDesiredStateDAO;
@@ -141,9 +135,20 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   @Inject
   private AmbariEventPublisher eventPublisher;
 
+  // TODO : caching the JPA entities here causes issues if they become stale and get re-merged.
   private HostComponentStateEntity stateEntity;
   private HostComponentDesiredStateEntity desiredStateEntity;
 
+  /**
+   * The component state entity PK.
+   */
+  private final HostComponentStateEntityPK stateEntityPK;
+
+  /**
+   * The desired component state entity PK.
+   */
+  private final HostComponentDesiredStateEntityPK desiredStateEntityPK;
+
   private long lastOpStartTime;
   private long lastOpEndTime;
   private long lastOpLastUpdateTime;
@@ -563,7 +568,6 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
       impl.updateLastOpInfo(event.getType(), event.getOpTimestamp());
 
     }
-
   }
 
   /**
@@ -696,6 +700,8 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
     stateEntity.setUpgradeState(UpgradeState.NONE);
     stateEntity.setCurrentStackVersion(gson.toJson(new StackId()));
 
+    stateEntityPK = getHostComponentStateEntityPK(stateEntity);
+
     desiredStateEntity = new HostComponentDesiredStateEntity();
     desiredStateEntity.setClusterId(serviceComponent.getClusterId());
     desiredStateEntity.setComponentName(serviceComponent.getName());
@@ -710,6 +716,8 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
       desiredStateEntity.setAdminState(null);
     }
 
+    desiredStateEntityPK = getHostComponentDesiredStateEntityPK(desiredStateEntity);
+
     try {
       host = clusters.getHost(hostName);
     } catch (AmbariException e) {
@@ -733,6 +741,9 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
     this.desiredStateEntity = desiredStateEntity;
     this.stateEntity = stateEntity;
 
+    desiredStateEntityPK = getHostComponentDesiredStateEntityPK(desiredStateEntity);
+    stateEntityPK = getHostComponentStateEntityPK(stateEntity);
+
     //TODO implement State Machine init as now type choosing is hardcoded in above code
     if (serviceComponent.isClientComponent()) {
       stateMachine = clientStateMachineFactory.make(this);
@@ -764,7 +775,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
     writeLock.lock();
     try {
       stateMachine.setCurrentState(state);
-      stateEntity.setCurrentState(state);
+      getStateEntity().setCurrentState(state);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -775,7 +786,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public String getVersion() {
     readLock.lock();
     try {
-      return stateEntity.getVersion();
+      return getStateEntity().getVersion();
     } finally {
       readLock.unlock();
     }
@@ -785,7 +796,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public void setVersion(String version) {
     writeLock.lock();
     try {
-      stateEntity.setVersion(version);
+      getStateEntity().setVersion(version);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -796,7 +807,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public SecurityState getSecurityState() {
     readLock.lock();
     try {
-      return stateEntity.getSecurityState();
+      return getStateEntity().getSecurityState();
     } finally {
       readLock.unlock();
     }
@@ -806,7 +817,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public void setSecurityState(SecurityState securityState) {
     writeLock.lock();
     try {
-      stateEntity.setSecurityState(securityState);
+      getStateEntity().setSecurityState(securityState);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -817,7 +828,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public SecurityState getDesiredSecurityState() {
     readLock.lock();
     try {
-      return desiredStateEntity.getSecurityState();
+      return getDesiredStateEntity().getSecurityState();
     } finally {
       readLock.unlock();
     }
@@ -831,7 +842,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
 
     writeLock.lock();
     try {
-      desiredStateEntity.setSecurityState(securityState);
+      getDesiredStateEntity().setSecurityState(securityState);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -844,13 +855,14 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
    * If the upgrade completes successfully, the upgradeState should be set back to NONE.
    * If the upgrade fails, then the user can retry to set it back into PENDING or IN_PROGRESS.
    * If the upgrade is aborted, then the upgradeState should be set back to NONE.
-   * @param upgradeState
+   *
+   * @param upgradeState  the upgrade state
    */
   @Override
   public void setUpgradeState(UpgradeState upgradeState) {
     writeLock.lock();
     try {
-      stateEntity.setUpgradeState(upgradeState);
+      getStateEntity().setUpgradeState(upgradeState);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -873,7 +885,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
         writeLock.lock();
         try {
           stateMachine.doTransition(event.getType(), event);
-          stateEntity.setCurrentState(stateMachine.getCurrentState());
+          getStateEntity().setCurrentState(stateMachine.getCurrentState());
           saveIfPersisted();
           // TODO Audit logs
         } catch (InvalidStateTransitionException e) {
@@ -1002,7 +1014,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public StackId getStackVersion() {
     readLock.lock();
     try {
-      return gson.fromJson(stateEntity.getCurrentStackVersion(), StackId.class);
+      return gson.fromJson(getStateEntity().getCurrentStackVersion(), StackId.class);
     } finally {
       readLock.unlock();
     }
@@ -1012,7 +1024,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public void setStackVersion(StackId stackVersion) {
     writeLock.lock();
     try {
-      stateEntity.setCurrentStackVersion(gson.toJson(stackVersion));
+      getStateEntity().setCurrentStackVersion(gson.toJson(stackVersion));
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -1023,7 +1035,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public State getDesiredState() {
     readLock.lock();
     try {
-      return desiredStateEntity.getDesiredState();
+      return getDesiredStateEntity().getDesiredState();
     } finally {
       readLock.unlock();
     }
@@ -1033,7 +1045,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public void setDesiredState(State state) {
     writeLock.lock();
     try {
-      desiredStateEntity.setDesiredState(state);
+      getDesiredStateEntity().setDesiredState(state);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -1044,7 +1056,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public StackId getDesiredStackVersion() {
     readLock.lock();
     try {
-      return gson.fromJson(desiredStateEntity.getDesiredStackVersion(),
+      return gson.fromJson(getDesiredStateEntity().getDesiredStackVersion(),
           StackId.class);
     } finally {
       readLock.unlock();
@@ -1055,7 +1067,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public void setDesiredStackVersion(StackId stackVersion) {
     writeLock.lock();
     try {
-      desiredStateEntity.setDesiredStackVersion(gson.toJson(stackVersion));
+      getDesiredStateEntity().setDesiredStackVersion(gson.toJson(stackVersion));
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -1066,7 +1078,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public HostComponentAdminState getComponentAdminState() {
     readLock.lock();
     try {
-      HostComponentAdminState adminState = desiredStateEntity.getAdminState();
+      HostComponentAdminState adminState = getDesiredStateEntity().getAdminState();
       if (adminState == null && !serviceComponent.isClientComponent()
           && !serviceComponent.isMasterComponent()) {
         adminState = HostComponentAdminState.INSERVICE;
@@ -1081,7 +1093,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public void setComponentAdminState(HostComponentAdminState attribute) {
     writeLock.lock();
     try {
-      desiredStateEntity.setAdminState(attribute);
+      getDesiredStateEntity().setAdminState(attribute);
       saveIfPersisted();
     } finally {
       writeLock.unlock();
@@ -1165,7 +1177,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
           clusterGlobalLock.writeLock().unlock();
           clusterWriteLockAcquired = false;
 
-          // these shoudl still be done with the internal lock
+          // these should still be done with the internal lock
           refresh();
           host.refresh();
           serviceComponent.refresh();
@@ -1219,26 +1231,11 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   }
 
   @Override
-  @Transactional
   public void refresh() {
     writeLock.lock();
     try {
-      if (isPersisted()) {
-        HostComponentStateEntityPK pk = new HostComponentStateEntityPK();
-        HostComponentDesiredStateEntityPK dpk = new HostComponentDesiredStateEntityPK();
-        pk.setClusterId(getClusterId());
-        pk.setComponentName(getServiceComponentName());
-        pk.setServiceName(getServiceName());
-        pk.setHostName(getHostName());
-        dpk.setClusterId(getClusterId());
-        dpk.setComponentName(getServiceComponentName());
-        dpk.setServiceName(getServiceName());
-        dpk.setHostName(getHostName());
-        stateEntity = hostComponentStateDAO.findByPK(pk);
-        desiredStateEntity = hostComponentDesiredStateDAO.findByPK(dpk);
-        hostComponentStateDAO.refresh(stateEntity);
-        hostComponentDesiredStateDAO.refresh(desiredStateEntity);
-      }
+      getDesiredStateEntity();
+      getStateEntity();
     } finally {
       writeLock.unlock();
     }
@@ -1259,11 +1256,9 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
     try {
       // if unable to read, then writers are writing; cannot remove SCH
       schLockAcquired = readLock.tryLock();
-      if (!schLockAcquired) {
-        return false;
-      }
 
-      return (getState().isRemovableState());
+      return schLockAcquired && (getState().isRemovableState());
+
     } finally {
       if (schLockAcquired) {
         readLock.unlock();
@@ -1407,7 +1402,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public void setMaintenanceState(MaintenanceState state) {
     writeLock.lock();
     try {
-      desiredStateEntity.setMaintenanceState(state);
+      getDesiredStateEntity().setMaintenanceState(state);
       saveIfPersisted();
 
       // broadcast the maintenance mode change
@@ -1423,7 +1418,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public MaintenanceState getMaintenanceState() {
     readLock.lock();
     try {
-      return desiredStateEntity.getMaintenanceState();
+      return getDesiredStateEntity().getMaintenanceState();
     } finally {
       readLock.unlock();
     }
@@ -1453,7 +1448,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public boolean isRestartRequired() {
     readLock.lock();
     try {
-      return desiredStateEntity.isRestartRequired();
+      return getDesiredStateEntity().isRestartRequired();
     } finally {
       readLock.unlock();
     }
@@ -1463,7 +1458,7 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   public void setRestartRequired(boolean restartRequired) {
     writeLock.lock();
     try {
-      desiredStateEntity.setRestartRequired(restartRequired);
+      getDesiredStateEntity().setRestartRequired(restartRequired);
       saveIfPersisted();
       helper.invalidateStaleConfigsCache(this);
     } finally {
@@ -1475,10 +1470,9 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
   private RepositoryVersionEntity createRepositoryVersion(String version, final StackId stackId, final StackInfo stackInfo) throws AmbariException {
     // During an Ambari Upgrade from 1.7.0 -> 2.0.0, the Repo Version will not exist, so bootstrap it.
     LOG.info("Creating new repository version " + stackId.getStackName() + "-" + version);
-    RepositoryVersionEntity repositoryVersion = repositoryVersionDAO.create(stackId.getStackId(), version, stackId.getStackName() + "-" + version,
+    return repositoryVersionDAO.create(stackId.getStackId(), version, stackId.getStackName() + "-" + version,
         repositoryVersionHelper.getUpgradePackageNameSafe(stackId.getStackName(), stackId.getStackVersion(), version),
         repositoryVersionHelper.serializeOperatingSystems(stackInfo.getRepositories()));
-    return repositoryVersion;
   }
 
   /**
@@ -1519,4 +1513,42 @@ public class ServiceComponentHostImpl implements ServiceComponentHost {
     }
     return version;
   }
+
+  // Get the cached desired state entity or load it fresh through the DAO.
+  private HostComponentDesiredStateEntity getDesiredStateEntity() {
+    if (isPersisted()) {
+      desiredStateEntity = hostComponentDesiredStateDAO.findByPK(desiredStateEntityPK);
+    }
+    return desiredStateEntity;
+  }
+
+  // Get the cached state entity or load it fresh through the DAO.
+  private HostComponentStateEntity getStateEntity() {
+    if (isPersisted()) {
+      stateEntity = hostComponentStateDAO.findByPK(stateEntityPK);
+    }
+    return stateEntity;
+  }
+
+  // create a PK object from the given desired component state entity.
+  private static HostComponentDesiredStateEntityPK getHostComponentDesiredStateEntityPK(
+      HostComponentDesiredStateEntity desiredStateEntity) {
+
+    HostComponentDesiredStateEntityPK dpk = new HostComponentDesiredStateEntityPK();
+    dpk.setClusterId(desiredStateEntity.getClusterId());
+    dpk.setComponentName(desiredStateEntity.getComponentName());
+    dpk.setServiceName(desiredStateEntity.getServiceName());
+    dpk.setHostName(desiredStateEntity.getHostName());
+    return dpk;
+  }
+
+  // create a PK object from the given component state entity.
+  private static HostComponentStateEntityPK getHostComponentStateEntityPK(HostComponentStateEntity stateEntity) {
+    HostComponentStateEntityPK pk = new HostComponentStateEntityPK();
+    pk.setClusterId(stateEntity.getClusterId());
+    pk.setComponentName(stateEntity.getComponentName());
+    pk.setServiceName(stateEntity.getServiceName());
+    pk.setHostName(stateEntity.getHostName());
+    return pk;
+  }
 }

ambari-server/src/test/java/org/apache/ambari/server/state/host/HostImplTest.java

@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ambari.server.state.host;
+
+import com.google.gson.Gson;
+import com.google.inject.Injector;
+import org.apache.ambari.server.orm.dao.HostDAO;
+import org.apache.ambari.server.orm.entities.HostEntity;
+import org.apache.ambari.server.orm.entities.HostStateEntity;
+import org.apache.ambari.server.state.Clusters;
+import org.apache.ambari.server.state.HostHealthStatus;
+import org.junit.Test;
+
+import java.util.Map;
+
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.Assert.*;
+
+public class HostImplTest {
+
+  @Test
+  public void testGetHostAttributes() throws Exception {
+
+    HostEntity hostEntity = createNiceMock(HostEntity.class);
+    HostStateEntity hostStateEntity = createNiceMock(HostStateEntity.class);
+    HostDAO hostDAO  = createNiceMock(HostDAO.class);
+    Injector injector = createNiceMock(Injector.class);
+
+    Gson gson = new Gson();
+
+    expect(injector.getInstance(Gson.class)).andReturn(gson).anyTimes();
+    expect(injector.getInstance(HostDAO.class)).andReturn(hostDAO).anyTimes();
+    expect(hostEntity.getHostAttributes()).andReturn("{\"foo\": \"aaa\", \"bar\":\"bbb\"}").anyTimes();
+    expect(hostEntity.getHostName()).andReturn("host1").anyTimes();
+    expect(hostEntity.getHostStateEntity()).andReturn(hostStateEntity).anyTimes();
+    expect(hostDAO.findByName("host1")).andReturn(hostEntity);
+
+    replay(hostEntity, hostStateEntity, injector, hostDAO);
+    HostImpl host = new HostImpl(hostEntity, false, injector);
+
+    Map<String, String> hostAttributes = host.getHostAttributes();
+    assertEquals("aaa", hostAttributes.get("foo"));
+    assertEquals("bbb", hostAttributes.get("bar"));
+
+    host = new HostImpl(hostEntity, true, injector);
+
+    hostAttributes = host.getHostAttributes();
+    assertEquals("aaa", hostAttributes.get("foo"));
+    assertEquals("bbb", hostAttributes.get("bar"));
+
+    verify(hostEntity, hostStateEntity, injector, hostDAO);
+  }
+
+  @Test
+  public void testGetHealthStatus() throws Exception {
+
+    HostEntity hostEntity = createNiceMock(HostEntity.class);
+    HostStateEntity hostStateEntity = createNiceMock(HostStateEntity.class);
+    HostDAO hostDAO  = createNiceMock(HostDAO.class);
+    Injector injector = createNiceMock(Injector.class);
+
+    Gson gson = new Gson();
+
+    expect(injector.getInstance(Gson.class)).andReturn(gson).anyTimes();
+    expect(injector.getInstance(HostDAO.class)).andReturn(hostDAO).anyTimes();
+    expect(hostEntity.getHostAttributes()).andReturn("{\"foo\": \"aaa\", \"bar\":\"bbb\"}").anyTimes();
+    expect(hostEntity.getHostName()).andReturn("host1").anyTimes();
+    expect(hostEntity.getHostStateEntity()).andReturn(hostStateEntity).anyTimes();
+    expect(hostDAO.findByName("host1")).andReturn(hostEntity);
+
+    replay(hostEntity, hostStateEntity, injector, hostDAO);
+    HostImpl host = new HostImpl(hostEntity, false, injector);
+
+    host.getHealthStatus();
+
+    host = new HostImpl(hostEntity, true, injector);
+
+    host.getHealthStatus();
+
+    verify(hostEntity, hostStateEntity, injector, hostDAO);
+  }
+}