Repository: ambari Updated Branches: refs/heads/trunk a394f2f7a -> 7b19db6b2
AMBARI-12178 - Memory Exhausted During Upgrade Of Large Cluster (jonathanhurley) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/7b19db6b Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/7b19db6b Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/7b19db6b Branch: refs/heads/trunk Commit: 7b19db6b2d2c68f13f4fcc94ac6396533c6cdf43 Parents: a394f2f Author: Jonathan Hurley <[email protected]> Authored: Fri Jun 26 17:52:00 2015 -0400 Committer: Jonathan Hurley <[email protected]> Committed: Sat Jun 27 00:45:26 2015 -0400 ---------------------------------------------------------------------- .../server/actionmanager/HostRoleCommand.java | 67 ++++++++++++++------ .../internal/StageResourceProvider.java | 64 ++++++++++--------- .../internal/UpgradeGroupResourceProvider.java | 7 -- .../apache/ambari/server/orm/dao/StageDAO.java | 10 +-- .../ambari/server/orm/entities/HostEntity.java | 3 +- .../ambari/server/orm/entities/StageEntity.java | 36 ++++------- .../ambari/server/topology/HostRequest.java | 48 +++++++------- .../ambari/server/topology/TopologyManager.java | 42 +++++++----- 8 files changed, 149 insertions(+), 128 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/7b19db6b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java index 20ec9ea..85e9135 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java @@ -17,7 +17,6 @@ */ package org.apache.ambari.server.actionmanager; -import com.google.inject.Inject; import org.apache.ambari.server.Role; import org.apache.ambari.server.RoleCommand; import org.apache.ambari.server.orm.dao.ExecutionCommandDAO; @@ -28,15 +27,20 @@ import org.apache.ambari.server.orm.entities.HostRoleCommandEntity; import org.apache.ambari.server.state.Host; import org.apache.ambari.server.state.ServiceComponentHostEvent; -import com.google.inject.Injector; +import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; /** * This class encapsulates the information for an task on a host for a - * particular role which action manager needs. It doesn't capture actual - * command and parameters, but just the stuff enough for action manager to - * track the request. + * particular role which action manager needs. It doesn't capture actual command + * and parameters, but just the stuff enough for action manager to track the + * request. + * <p/> + * Since this class is cached by the {@link ActionDBAccessor}, it should not + * hold references to JPA entities. It's possible that by holding onto JPA + * entities, they will inadvertently hold onto the entire cache of entities in + * the L1 cache. */ public class HostRoleCommand { private final Role role; @@ -44,7 +48,8 @@ public class HostRoleCommand { private long taskId = -1; private long stageId = -1; private long requestId = -1; - private HostEntity hostEntity; + private long hostId = -1; + private String hostName; private HostRoleStatus status = HostRoleStatus.PENDING; private String stdout = ""; private String stderr = ""; @@ -87,34 +92,41 @@ public class HostRoleCommand { * @param hostName Host name * @param role Action to run * @param event Event on the host and component - * @param command Type of command + * @param roleCommand Type of command * @param retryAllowed Whether the command can be repeated * @param hostDAO {@link org.apache.ambari.server.orm.dao.HostDAO} instance being injected */ @AssistedInject public HostRoleCommand(String hostName, Role role, - ServiceComponentHostEvent event, RoleCommand command, boolean retryAllowed, HostDAO hostDAO, ExecutionCommandDAO executionCommandDAO) { + ServiceComponentHostEvent event, RoleCommand roleCommand, boolean retryAllowed, HostDAO hostDAO, ExecutionCommandDAO executionCommandDAO) { this.hostDAO = hostDAO; this.executionCommandDAO = executionCommandDAO; this.role = role; this.event = new ServiceComponentHostEventWrapper(event); - this.roleCommand = command; + this.roleCommand = roleCommand; this.retryAllowed = retryAllowed; - this.hostEntity = this.hostDAO.findByName(hostName); + this.hostName = hostName; + + HostEntity hostEntity = this.hostDAO.findByName(hostName); + if (null != hostEntity) { + hostId = hostEntity.getHostId(); + } } @AssistedInject - public HostRoleCommand(Host host, Role role, ServiceComponentHostEvent event, RoleCommand command, + public HostRoleCommand(Host host, Role role, ServiceComponentHostEvent event, + RoleCommand roleCommand, boolean retryAllowed, HostDAO hostDAO, ExecutionCommandDAO executionCommandDAO) { this.hostDAO = hostDAO; this.executionCommandDAO = executionCommandDAO; this.role = role; this.event = new ServiceComponentHostEventWrapper(event); - this.roleCommand = command; + this.roleCommand = roleCommand; this.retryAllowed = retryAllowed; - this.hostEntity = hostDAO.findById(host.getHostId()); + hostId = host.getHostId(); + hostName = host.getHostName(); } @AssistedInject @@ -125,7 +137,8 @@ public class HostRoleCommand { taskId = hostRoleCommandEntity.getTaskId(); stageId = hostRoleCommandEntity.getStage().getStageId(); requestId = hostRoleCommandEntity.getStage().getRequestId(); - this.hostEntity = hostRoleCommandEntity.getHostEntity(); + hostId = hostRoleCommandEntity.getHostId(); + hostName = hostRoleCommandEntity.getHostName(); role = hostRoleCommandEntity.getRole(); status = hostRoleCommandEntity.getStatus(); stdout = hostRoleCommandEntity.getStdOut() != null ? new String(hostRoleCommandEntity.getStdOut()) : ""; @@ -149,7 +162,6 @@ public class HostRoleCommand { //todo: why are we only setting some fields in this constructor, 8 fields missing????? public HostRoleCommandEntity constructNewPersistenceEntity() { HostRoleCommandEntity hostRoleCommandEntity = new HostRoleCommandEntity(); - hostRoleCommandEntity.setHostEntity(hostEntity); hostRoleCommandEntity.setRole(role); hostRoleCommandEntity.setStatus(status); hostRoleCommandEntity.setStdError(stderr.getBytes()); @@ -165,6 +177,11 @@ public class HostRoleCommand { hostRoleCommandEntity.setCommandDetail(commandDetail); hostRoleCommandEntity.setCustomCommandName(customCommandName); + HostEntity hostEntity = hostDAO.findById(hostId); + if (null != hostEntity) { + hostRoleCommandEntity.setHostEntity(hostEntity); + } + hostRoleCommandEntity.setEvent(event.getEventJson()); return hostRoleCommandEntity; @@ -202,13 +219,23 @@ public class HostRoleCommand { } } + /** + * Sets the host ID and name for the host associated with this command. + * + * @param hostId + * @param hostName + */ + public void setHost(long hostId, String hostName) { + this.hostId = hostId; + this.hostName = hostName; + } + public String getHostName() { - return hostEntity != null ? hostEntity.getHostName() : null; + return hostName; } - public void setHostEntity(HostEntity entity) { - //todo: initial entity id and name may be null in case of 'logical' tasks - hostEntity = entity; + public long getHostId() { + return hostId; } public Role getRole() { @@ -300,7 +327,7 @@ public class HostRoleCommand { } public void incrementAttemptCount() { - this.attemptCount++; + attemptCount++; } public boolean isRetryAllowed() { http://git-wip-us.apache.org/repos/asf/ambari/blob/7b19db6b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java index 664fae3..67379ee 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StageResourceProvider.java @@ -74,11 +74,6 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im @Inject private static HostRoleCommandDAO hostRoleCommandDAO = null; - /** - * Used to get cluster information. - */ - private static Clusters clusters = null; - @Inject private static Provider<Clusters> clustersProvider = null; @@ -188,8 +183,6 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im Map<String,Object> updateProperties = iterator.next(); - ensureClusters(); - List<StageEntity> entities = dao.findAll(request, predicate); for (StageEntity entity : entities) { @@ -217,8 +210,6 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im throws SystemException, UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException { - ensureClusters(); - Set<Resource> results = new LinkedHashSet<Resource>(); Set<String> propertyIds = getRequestPropertyIds(request, predicate); @@ -231,6 +222,7 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im for (StageEntity entity : entities) { results.add(toResource(cache, entity, propertyIds)); } + cache.clear(); Collection<StageEntity> topologyManagerStages = topologyManager.getStages(); @@ -336,7 +328,7 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im Long clusterId = entity.getClusterId(); if (clusterId != null && !clusterId.equals(Long.valueOf(-1L))) { try { - Cluster cluster = clusters.getClusterById(clusterId); + Cluster cluster = clustersProvider.get().getClusterById(clusterId); setResourceProperty(resource, STAGE_CLUSTER_NAME, cluster.getClusterName(), requestedIds); } catch (Exception e) { @@ -353,9 +345,22 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im setResourceProperty(resource, STAGE_STAGE_ID, entity.getStageId(), requestedIds); setResourceProperty(resource, STAGE_REQUEST_ID, entity.getRequestId(), requestedIds); setResourceProperty(resource, STAGE_CONTEXT, entity.getRequestContext(), requestedIds); - setResourceProperty(resource, STAGE_CLUSTER_HOST_INFO, entity.getClusterHostInfo(), requestedIds); - setResourceProperty(resource, STAGE_COMMAND_PARAMS, entity.getCommandParamsStage(), requestedIds); - setResourceProperty(resource, STAGE_HOST_PARAMS, entity.getHostParamsStage(), requestedIds); + + // this property is lazy loaded in JPA; don't use it unless requested + if (isPropertyRequested(STAGE_CLUSTER_HOST_INFO, requestedIds)) { + resource.setProperty(STAGE_CLUSTER_HOST_INFO, entity.getClusterHostInfo()); + } + + // this property is lazy loaded in JPA; don't use it unless requested + if (isPropertyRequested(STAGE_COMMAND_PARAMS, requestedIds)) { + resource.setProperty(STAGE_COMMAND_PARAMS, entity.getCommandParamsStage()); + } + + // this property is lazy loaded in JPA; don't use it unless requested + if (isPropertyRequested(STAGE_HOST_PARAMS, requestedIds)) { + resource.setProperty(STAGE_HOST_PARAMS, entity.getHostParamsStage()); + } + setResourceProperty(resource, STAGE_SKIPPABLE, entity.isSkippable(), requestedIds); Long startTime = Long.MAX_VALUE; @@ -393,7 +398,7 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im Long clusterId = entity.getClusterId(); if (clusterId != null && !clusterId.equals(Long.valueOf(-1L))) { try { - Cluster cluster = clusters.getClusterById(clusterId); + Cluster cluster = clustersProvider.get().getClusterById(clusterId); setResourceProperty(resource, STAGE_CLUSTER_NAME, cluster.getClusterName(), requestedIds); } catch (Exception e) { @@ -407,9 +412,22 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im setResourceProperty(resource, STAGE_STAGE_ID, entity.getStageId(), requestedIds); setResourceProperty(resource, STAGE_REQUEST_ID, entity.getRequestId(), requestedIds); setResourceProperty(resource, STAGE_CONTEXT, entity.getRequestContext(), requestedIds); - setResourceProperty(resource, STAGE_CLUSTER_HOST_INFO, entity.getClusterHostInfo(), requestedIds); - setResourceProperty(resource, STAGE_COMMAND_PARAMS, entity.getCommandParamsStage(), requestedIds); - setResourceProperty(resource, STAGE_HOST_PARAMS, entity.getHostParamsStage(), requestedIds); + + // this property is lazy loaded in JPA; don't use it unless requested + if (isPropertyRequested(STAGE_CLUSTER_HOST_INFO, requestedIds)) { + resource.setProperty(STAGE_CLUSTER_HOST_INFO, entity.getClusterHostInfo()); + } + + // this property is lazy loaded in JPA; don't use it unless requested + if (isPropertyRequested(STAGE_COMMAND_PARAMS, requestedIds)) { + resource.setProperty(STAGE_COMMAND_PARAMS, entity.getCommandParamsStage()); + } + + // this property is lazy loaded in JPA; don't use it unless requested + if (isPropertyRequested(STAGE_HOST_PARAMS, requestedIds)) { + resource.setProperty(STAGE_HOST_PARAMS, entity.getHostParamsStage()); + } + setResourceProperty(resource, STAGE_SKIPPABLE, entity.isSkippable(), requestedIds); Long startTime = Long.MAX_VALUE; @@ -431,18 +449,6 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im } /** - * Ensure that cluster information is available. - * - * @return the clusters information - */ - private synchronized Clusters ensureClusters() { - if (clusters == null) { - clusters = clustersProvider.get(); - } - return clusters; - } - - /** * Determine whether or not it is valid to transition from this stage status to the given status. * * @param status the stage status being transitioned to http://git-wip-us.apache.org/repos/asf/ambari/blob/7b19db6b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeGroupResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeGroupResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeGroupResourceProvider.java index eb34d63..468315b 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeGroupResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeGroupResourceProvider.java @@ -39,7 +39,6 @@ import org.apache.ambari.server.controller.spi.SystemException; import org.apache.ambari.server.controller.spi.UnsupportedPropertyException; import org.apache.ambari.server.orm.dao.HostRoleCommandDAO; import org.apache.ambari.server.orm.dao.HostRoleCommandStatusSummaryDTO; -import org.apache.ambari.server.orm.dao.StageDAO; import org.apache.ambari.server.orm.dao.UpgradeDAO; import org.apache.ambari.server.orm.entities.UpgradeEntity; import org.apache.ambari.server.orm.entities.UpgradeGroupEntity; @@ -75,12 +74,6 @@ public class UpgradeGroupResourceProvider extends AbstractControllerResourceProv @Inject private static UpgradeDAO m_dao = null; - /** - * Used for querying stage resources. - */ - @Inject - private static StageDAO stageDAO = null; - @Inject private static HostRoleCommandDAO s_hostRoleCommandDao; http://git-wip-us.apache.org/repos/asf/ambari/blob/7b19db6b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/StageDAO.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/StageDAO.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/StageDAO.java index b354841..4b0056e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/StageDAO.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/dao/StageDAO.java @@ -42,8 +42,6 @@ import org.apache.ambari.server.orm.entities.StageEntity; import org.apache.ambari.server.orm.entities.StageEntityPK; import org.apache.ambari.server.orm.entities.StageEntity_; import org.apache.ambari.server.utils.StageUtils; -import org.eclipse.persistence.config.HintValues; -import org.eclipse.persistence.config.QueryHints; import com.google.inject.Inject; import com.google.inject.Provider; @@ -187,7 +185,7 @@ public class StageDAO { * @param request * @return */ - @Transactional + @RequiresSession public List<StageEntity> findAll(Request request, Predicate predicate) { EntityManager entityManager = entityManagerProvider.get(); @@ -208,12 +206,6 @@ public class StageDAO { query.orderBy(sortOrders); TypedQuery<StageEntity> typedQuery = entityManager.createQuery(query); - - // !!! https://bugs.eclipse.org/bugs/show_bug.cgi?id=398067 - // ensure that an associated entity with a JOIN is not stale; this causes - // the associated StageEntity to be stale - typedQuery.setHint(QueryHints.REFRESH, HintValues.TRUE); - return daoUtils.selectList(typedQuery); } http://git-wip-us.apache.org/repos/asf/ambari/blob/7b19db6b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostEntity.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostEntity.java index 9f3f70c..42f7777 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostEntity.java @@ -39,10 +39,11 @@ import javax.persistence.NamedQueries; import javax.persistence.NamedQuery; import javax.persistence.OneToMany; import javax.persistence.OneToOne; +import javax.persistence.Table; import javax.persistence.TableGenerator; [email protected](name = "hosts") @Entity +@Table(name = "hosts") @TableGenerator(name = "host_id_generator", table = "ambari_sequences", pkColumnName = "sequence_name", valueColumnName = "sequence_value" , pkColumnValue = "host_id_seq" http://git-wip-us.apache.org/repos/asf/ambari/blob/7b19db6b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/StageEntity.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/StageEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/StageEntity.java index c2b97d6..e801233 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/StageEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/StageEntity.java @@ -65,12 +65,24 @@ public class StageEntity { @Basic private String requestContext = ""; + /** + * On large clusters, this value can be in the 10,000's of kilobytes. During + * an upgrade, all stages are loaded in memory for every request, which can + * lead to an OOM. As a result, lazy load this since it's barely ever + * requested or used. + */ @Column(name = "cluster_host_info") - @Basic + @Basic(fetch = FetchType.LAZY) private byte[] clusterHostInfo; + /** + * On large clusters, this value can be in the 10,000's of kilobytes. During + * an upgrade, all stages are loaded in memory for every request, which can + * lead to an OOM. As a result, lazy load this since it's barely ever + * requested or used. + */ @Column(name = "command_params") - @Basic + @Basic(fetch = FetchType.LAZY) private byte[] commandParamsStage; @Column(name = "host_params") @@ -178,22 +190,6 @@ public class StageEntity { return false; } - if (logInfo != null ? !logInfo.equals(that.logInfo) : that.logInfo != null) { - return false; - } - - if (clusterHostInfo != null ? !clusterHostInfo.equals(that.clusterHostInfo) : that.clusterHostInfo != null) { - return false; - } - - if (commandParamsStage != null ? !commandParamsStage.equals(that.commandParamsStage) : that.commandParamsStage != null) { - return false; - } - - if (hostParamsStage != null ? !hostParamsStage.equals(that.hostParamsStage) : that.hostParamsStage != null) { - return false; - } - return !(requestContext != null ? !requestContext.equals(that.requestContext) : that.requestContext != null); } @@ -202,10 +198,6 @@ public class StageEntity { int result = clusterId != null ? clusterId.hashCode() : 0; result = 31 * result + (requestId != null ? requestId.hashCode() : 0); result = 31 * result + (stageId != null ? stageId.hashCode() : 0); - result = 31 * result + (logInfo != null ? logInfo.hashCode() : 0); - result = 31 * result + (clusterHostInfo != null ? clusterHostInfo.hashCode() : 0); - result = 31 * result + (commandParamsStage != null ? commandParamsStage.hashCode() : 0); - result = 31 * result + (hostParamsStage != null ? hostParamsStage.hashCode() : 0); result = 31 * result + (requestContext != null ? requestContext.hashCode() : 0); return result; } http://git-wip-us.apache.org/repos/asf/ambari/blob/7b19db6b/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java index f63ba3f..5d76f7a 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java @@ -18,6 +18,13 @@ package org.apache.ambari.server.topology; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; + import org.apache.ambari.server.actionmanager.HostRoleCommand; import org.apache.ambari.server.api.predicate.InvalidQueryException; import org.apache.ambari.server.api.predicate.PredicateCompiler; @@ -36,13 +43,6 @@ import org.apache.ambari.server.state.host.HostImpl; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; - /** * Represents a set of requests to a single host such as install, start, etc. */ @@ -79,11 +79,11 @@ public class HostRequest implements Comparable<HostRequest> { this.requestId = requestId; this.id = id; this.cluster = cluster; - this.blueprint = blueprintName; + blueprint = blueprintName; this.hostGroup = hostGroup; - this.hostgroupName = hostGroup.getName(); + hostgroupName = hostGroup.getName(); this.predicate = predicate; - this.containsMaster = hostGroup.containsMasterComponent(); + containsMaster = hostGroup.containsMasterComponent(); this.topology = topology; createTasks(); @@ -105,13 +105,13 @@ public class HostRequest implements Comparable<HostRequest> { this.requestId = requestId; this.id = id; - this.cluster = topology.getClusterName(); - this.blueprint = topology.getBlueprint().getName(); - this.hostgroupName = entity.getTopologyHostGroupEntity().getName(); - this.hostGroup = topology.getBlueprint().getHostGroup(hostgroupName); - this.hostname = entity.getHostName(); + cluster = topology.getClusterName(); + blueprint = topology.getBlueprint().getName(); + hostgroupName = entity.getTopologyHostGroupEntity().getName(); + hostGroup = topology.getBlueprint().getHostGroup(hostgroupName); + hostname = entity.getHostName(); this.predicate = toPredicate(predicate); - this.containsMaster = hostGroup.containsMasterComponent(); + containsMaster = hostGroup.containsMasterComponent(); this.topology = topology; createTasksForReplay(entity); @@ -140,7 +140,7 @@ public class HostRequest implements Comparable<HostRequest> { } public void setHostName(String hostName) { - this.hostname = hostName; + hostname = hostName; } public long getRequestId() { @@ -260,7 +260,7 @@ public class HostRequest implements Comparable<HostRequest> { private void setHostOnTasks(HostImpl host) { for (HostRoleCommand task : getLogicalTasks()) { - task.setHostEntity(host.getHostEntity()); + task.setHost(host.getHostId(), host.getHostName()); } } @@ -382,7 +382,9 @@ public class HostRequest implements Comparable<HostRequest> { return other.containsMaster() ? hashCode() - other.hashCode() : -1; } else if (other.containsMaster()) { return 1; - } else return hashCode() - other.hashCode(); + } else { + return hashCode() - other.hashCode(); + } } //todo: once we have logical tasks, move tracking of physical tasks there @@ -441,7 +443,7 @@ public class HostRequest implements Comparable<HostRequest> { @Override public void init(ClusterTopology topology, AmbariContext ambariContext) { - this.clusterTopology = topology; + clusterTopology = topology; this.ambariContext = ambariContext; } @@ -462,7 +464,7 @@ public class HostRequest implements Comparable<HostRequest> { @Override public void init(ClusterTopology topology, AmbariContext ambariContext) { - this.clusterTopology = topology; + clusterTopology = topology; } @Override @@ -501,7 +503,7 @@ public class HostRequest implements Comparable<HostRequest> { @Override public void init(ClusterTopology topology, AmbariContext ambariContext) { - this.clusterTopology = topology; + clusterTopology = topology; } @Override @@ -596,6 +598,6 @@ public class HostRequest implements Comparable<HostRequest> { host.getDisksInfo()); hostResource.setProperty(HostResourceProvider.HOST_STATE_PROPERTY_ID, host.getState()); - } + } } } http://git-wip-us.apache.org/repos/asf/ambari/blob/7b19db6b/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java index 31363b4..b799d27 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java @@ -18,18 +18,6 @@ package org.apache.ambari.server.topology; -import com.google.inject.Singleton; -import org.apache.ambari.server.AmbariException; -import org.apache.ambari.server.actionmanager.HostRoleCommand; -import org.apache.ambari.server.actionmanager.Request; -import org.apache.ambari.server.controller.RequestStatusResponse; -import org.apache.ambari.server.controller.internal.Stack; -import org.apache.ambari.server.orm.dao.HostRoleCommandStatusSummaryDTO; -import org.apache.ambari.server.orm.entities.StageEntity; -import org.apache.ambari.server.state.host.HostImpl; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -42,6 +30,19 @@ import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import org.apache.ambari.server.AmbariException; +import org.apache.ambari.server.actionmanager.HostRoleCommand; +import org.apache.ambari.server.actionmanager.Request; +import org.apache.ambari.server.controller.RequestStatusResponse; +import org.apache.ambari.server.controller.internal.Stack; +import org.apache.ambari.server.orm.dao.HostRoleCommandStatusSummaryDTO; +import org.apache.ambari.server.orm.entities.StageEntity; +import org.apache.ambari.server.state.host.HostImpl; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.inject.Singleton; + /** * Manages all cluster provisioning actions on the cluster topology. */ @@ -68,7 +69,12 @@ public class TopologyManager { private static AmbariContext ambariContext = new AmbariContext(); private final Object initializationLock = new Object(); - private boolean isInitialized; + + /** + * A boolean not cached thread-local (volatile) to prevent double-checked + * locking on the synchronized keyword. + */ + private volatile boolean isInitialized; private final static Logger LOG = LoggerFactory.getLogger(TopologyManager.class); @@ -79,10 +85,12 @@ public class TopologyManager { //todo: can't call in constructor. //todo: Very important that this occurs prior to any usage private void ensureInitialized() { - synchronized(initializationLock) { - if (! isInitialized) { - replayRequests(persistedState.getAllRequests()); - isInitialized = true; + if (!isInitialized) { + synchronized (initializationLock) { + if (!isInitialized) { + replayRequests(persistedState.getAllRequests()); + isInitialized = true; + } } } }
