Repository: ambari Updated Branches: refs/heads/trunk 5ec73bce0 -> 12a3cd871
AMBARI-19055 - Removing Tasks From host_role_command Causes Upgrades To Show As PENDING (jonathanhurley) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/12a3cd87 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/12a3cd87 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/12a3cd87 Branch: refs/heads/trunk Commit: 12a3cd87184167cadf3cd12eb9b3c9d67dd3907a Parents: 5ec73bc Author: Jonathan Hurley <jhur...@hortonworks.com> Authored: Sat Dec 3 08:35:12 2016 -0500 Committer: Jonathan Hurley <jhur...@hortonworks.com> Committed: Sat Dec 3 14:09:44 2016 -0500 ---------------------------------------------------------------------- .../controller/internal/CalculatedStatus.java | 52 +++++++++++++++----- .../internal/RequestResourceProvider.java | 34 ++++++++----- .../internal/StageResourceProvider.java | 2 +- .../ambari/server/topology/TopologyManager.java | 19 ++++++- .../internal/CalculatedStatusTest.java | 31 ++++++++++++ .../internal/RequestResourceProviderTest.java | 12 +++-- 6 files changed, 121 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/12a3cd87/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java index 3a86aef..3c415df 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/CalculatedStatus.java @@ -53,6 +53,17 @@ public class CalculatedStatus { */ private final double percent; + /** + * A status which represents a COMPLETED state at 0% + */ + public static final CalculatedStatus COMPLETED = new CalculatedStatus(HostRoleStatus.COMPLETED, + HostRoleStatus.COMPLETED, 100.0); + + /** + * A status which represents a PENDING state at 0% + */ + public static final CalculatedStatus PENDING = new CalculatedStatus(HostRoleStatus.PENDING, + HostRoleStatus.PENDING, 0.0); // ----- Constructors ------------------------------------------------------ @@ -79,12 +90,6 @@ public class CalculatedStatus { this.percent = percent; } - /** - * Static factory method to get Status that represents a Completed state - */ - public static CalculatedStatus getCompletedStatus() { - return new CalculatedStatus(HostRoleStatus.COMPLETED, HostRoleStatus.COMPLETED, 100.0); - } // ----- CalculatedStatus -------------------------------------------------- @@ -291,14 +296,25 @@ public class CalculatedStatus { } /** - * Calculates the overall status of an upgrade. - * @param stageDto the map of stage-to-summary value objects - * @param stageIds the stage ids to consider from the value objects + * Calculates the overall status of an upgrade. If there are no tasks, then a + * status of {@link HostRoleStatus#COMPLETED} is returned. + * + * @param stageDto + * the map of stage-to-summary value objects + * @param stageIds + * the stage ids to consider from the value objects * @return the calculated status */ public static CalculatedStatus statusFromStageSummary(Map<Long, HostRoleCommandStatusSummaryDTO> stageDto, Set<Long> stageIds) { + // if either are empty, then we have no tasks and therefore no status - we + // should return COMPLETED. This can happen if someone removes all tasks but + // leaves the stages and request + if (stageDto.isEmpty() || stageIds.isEmpty()) { + return COMPLETED; + } + Collection<HostRoleStatus> stageStatuses = new HashSet<>(); Collection<HostRoleStatus> stageDisplayStatuses = new HashSet<>(); Collection<HostRoleStatus> taskStatuses = new ArrayList<>(); @@ -378,19 +394,28 @@ public class CalculatedStatus { */ public static HostRoleStatus calculateSummaryStatusOfStage(Map<HostRoleStatus, Integer> counters, int total, boolean skippable) { + + // when there are 0 tasks, return COMPLETED + if (total == 0) { + return HostRoleStatus.COMPLETED; + } + if (counters.get(HostRoleStatus.PENDING) == total) { return HostRoleStatus.PENDING; } + // By definition, any tasks in a future stage must be held in a PENDING status. if (counters.get(HostRoleStatus.HOLDING) > 0 || counters.get(HostRoleStatus.HOLDING_FAILED) > 0 || counters.get(HostRoleStatus.HOLDING_TIMEDOUT) > 0) { return counters.get(HostRoleStatus.HOLDING) > 0 ? HostRoleStatus.HOLDING : counters.get(HostRoleStatus.HOLDING_FAILED) > 0 ? HostRoleStatus.HOLDING_FAILED : HostRoleStatus.HOLDING_TIMEDOUT; } + // Because tasks are not skippable, guaranteed to be FAILED if (counters.get(HostRoleStatus.FAILED) > 0 && !skippable) { return HostRoleStatus.FAILED; } + // Because tasks are not skippable, guaranteed to be TIMEDOUT if (counters.get(HostRoleStatus.TIMEDOUT) > 0 && !skippable) { return HostRoleStatus.TIMEDOUT; @@ -401,9 +426,11 @@ public class CalculatedStatus { if (counters.get(HostRoleStatus.ABORTED) > 0 && numActiveTasks == 0) { return HostRoleStatus.ABORTED; } + if (counters.get(HostRoleStatus.COMPLETED) == total) { return HostRoleStatus.COMPLETED; } + return HostRoleStatus.IN_PROGRESS; } @@ -415,7 +442,8 @@ public class CalculatedStatus { * * @return summary request status based on statuses of tasks in different states. */ - private static HostRoleStatus calculateSummaryStatusOfUpgrade(Map<HostRoleStatus, Integer> counters, int total) { + protected static HostRoleStatus calculateSummaryStatusOfUpgrade( + Map<HostRoleStatus, Integer> counters, int total) { return calculateSummaryStatusOfStage(counters, total, false); } @@ -428,8 +456,8 @@ public class CalculatedStatus { * * @return summary request status based on statuses of tasks in different states. */ - private static HostRoleStatus calculateSummaryDisplayStatus(Map<HostRoleStatus, Integer> counters, - int total, boolean skippable) { + protected static HostRoleStatus calculateSummaryDisplayStatus( + Map<HostRoleStatus, Integer> counters, int total, boolean skippable) { return counters.get(HostRoleStatus.SKIPPED_FAILED) > 0 ? HostRoleStatus.SKIPPED_FAILED : counters.get(HostRoleStatus.FAILED) > 0 ? HostRoleStatus.FAILED: calculateSummaryStatusOfStage(counters, total, skippable); http://git-wip-us.apache.org/repos/asf/ambari/blob/12a3cd87/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java index 8c1bc57..0690ee7 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RequestResourceProvider.java @@ -69,10 +69,10 @@ import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.topology.LogicalRequest; import org.apache.ambari.server.topology.TopologyManager; +import org.apache.commons.lang.StringUtils; import com.google.common.collect.Sets; import com.google.inject.Inject; -import org.apache.commons.lang.StringUtils; /** * Resource provider for request resources. @@ -566,13 +566,18 @@ public class RequestResourceProvider extends AbstractControllerResourceProvider @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } ServiceComponentTuple that = (ServiceComponentTuple) o; - if (serviceName != null ? !serviceName.equals(that.serviceName) : that.serviceName != null) + if (serviceName != null ? !serviceName.equals(that.serviceName) : that.serviceName != null) { return false; + } return !(componentName != null ? !componentName.equals(that.componentName) : that.componentName != null); } @@ -739,15 +744,20 @@ public class RequestResourceProvider extends AbstractControllerResourceProvider // get summaries from TopologyManager for logical requests summary.putAll(topologyManager.getStageSummaries(entity.getRequestId())); + // summary might be empty due to delete host have cleared all + // HostRoleCommands or due to hosts haven't registered yet with the cluster + // when the cluster is provisioned with a Blueprint + final CalculatedStatus status; LogicalRequest logicalRequest = topologyManager.getRequest(entity.getRequestId()); - - CalculatedStatus status = CalculatedStatus.statusFromStageSummary(summary, summary.keySet()); - if (summary.isEmpty() && logicalRequest == null) { - - // summary might be empty due to delete host have cleared all HostRoleCommands - // or due to hosts haven't registered yet with the cluster when the cluster is provisioned - // with a Blueprint - status = CalculatedStatus.getCompletedStatus(); + if (summary.isEmpty() && null != logicalRequest) { + // in this case, it appears that there are no tasks but this is a logical + // topology request, so it's a matter of hosts simply not registering yet + // for tasks to be created + status = CalculatedStatus.PENDING; + } else { + // there are either tasks or this is not a logical request, so do normal + // status calculations + status = CalculatedStatus.statusFromStageSummary(summary, summary.keySet()); } setResourceProperty(resource, REQUEST_STATUS_PROPERTY_ID, status.getStatus().toString(), requestedPropertyIds); http://git-wip-us.apache.org/repos/asf/ambari/blob/12a3cd87/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 59dd9d9..a778882 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 @@ -339,7 +339,7 @@ public class StageResourceProvider extends AbstractControllerResourceProvider im CalculatedStatus status; if (summary.isEmpty()) { // Delete host might have cleared all HostRoleCommands - status = CalculatedStatus.getCompletedStatus(); + status = CalculatedStatus.COMPLETED; } else { status = CalculatedStatus.statusFromStageSummary(summary, Collections.singleton(entity.getStageId())); } http://git-wip-us.apache.org/repos/asf/ambari/blob/12a3cd87/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 d6a4bdd..d527b2d 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 @@ -596,6 +596,14 @@ public class TopologyManager { return clusterTopologyMap.get(clusterId); } + /** + * Gets a map of components keyed by host which have operations in the + * {@link HostRoleStatus#PENDING} state. This could either be because hosts + * have not registered or becuase the operations are actually waiting to be + * queued. + * + * @return a mapping of host with pending components. + */ public Map<String, Collection<String>> getPendingHostComponents() { ensureInitialized(); Map<String, Collection<String>> hostComponentMap = new HashMap<String, Collection<String>>(); @@ -603,7 +611,16 @@ public class TopologyManager { for (LogicalRequest logicalRequest : allRequests.values()) { Map<Long, HostRoleCommandStatusSummaryDTO> summary = logicalRequest.getStageSummaries(); final CalculatedStatus status = CalculatedStatus.statusFromStageSummary(summary, summary.keySet()); - if (status.getStatus().isInProgress()) { + + // either use the calculated status of the stage or the fact that there + // are no tasks and the request has no end time to determine if the + // request is still in progress + boolean logicalRequestInProgress = false; + if (status.getStatus().isInProgress() || (summary.isEmpty() && logicalRequest.getEndTime() <= 0) ) { + logicalRequestInProgress = true; + } + + if (logicalRequestInProgress) { Map<String, Collection<String>> requestTopology = logicalRequest.getProjectedTopology(); for (Map.Entry<String, Collection<String>> entry : requestTopology.entrySet()) { String host = entry.getKey(); http://git-wip-us.apache.org/repos/asf/ambari/blob/12a3cd87/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java index 6f592cd..a96f395 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CalculatedStatusTest.java @@ -657,6 +657,37 @@ public class CalculatedStatusTest { assertEquals(HostRoleStatus.IN_PROGRESS, calc.getStatus()); } + /** + * Tests that when there are no tasks and all counts are 0, that the returned + * status is {@link HostRoleStatus#COMPLETED}. + * + * @throws Exception + */ + @Test + public void testGetCompletedStatusForNoTasks() throws Exception { + // no status / no tasks + CalculatedStatus status = CalculatedStatus.statusFromTaskEntities( + new ArrayList<HostRoleCommandEntity>(), false); + + assertEquals(HostRoleStatus.COMPLETED, status.getStatus()); + + // empty summaries + status = CalculatedStatus.statusFromStageSummary( + new HashMap<Long, HostRoleCommandStatusSummaryDTO>(), new HashSet<Long>()); + + assertEquals(HostRoleStatus.COMPLETED, status.getStatus()); + + // generate a map of 0's - COMPLETED=0, IN_PROGRESS=0, etc + Map<HostRoleStatus, Integer> counts = CalculatedStatus.calculateStatusCounts(new ArrayList<HostRoleStatus>()); + Map<HostRoleStatus, Integer> displayCounts = CalculatedStatus.calculateStatusCounts(new ArrayList<HostRoleStatus>()); + + HostRoleStatus hostRoleStatus = CalculatedStatus.calculateSummaryStatusOfUpgrade(counts, 0); + HostRoleStatus hostRoleDisplayStatus = CalculatedStatus.calculateSummaryDisplayStatus(displayCounts, 0, false); + + assertEquals(HostRoleStatus.COMPLETED, hostRoleStatus); + assertEquals(HostRoleStatus.COMPLETED, hostRoleDisplayStatus); + } + private Collection<HostRoleCommandEntity> getTaskEntities(HostRoleStatus... statuses) { Collection<HostRoleCommandEntity> entities = new LinkedList<HostRoleCommandEntity>(); http://git-wip-us.apache.org/repos/asf/ambari/blob/12a3cd87/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java index 5dfc74d..f7dff11 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RequestResourceProviderTest.java @@ -20,11 +20,10 @@ package org.apache.ambari.server.controller.internal; import static org.apache.ambari.server.controller.internal.HostComponentResourceProvider.HOST_COMPONENT_STALE_CONFIGS_PROPERTY_ID; -import org.apache.ambari.server.topology.Blueprint; import static org.easymock.EasyMock.anyObject; import static org.easymock.EasyMock.capture; -import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.eq; +import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.newCapture; import static org.powermock.api.easymock.PowerMock.createMock; import static org.powermock.api.easymock.PowerMock.createNiceMock; @@ -77,6 +76,7 @@ import org.apache.ambari.server.security.authorization.AuthorizationHelperInitia import org.apache.ambari.server.security.authorization.RoleAuthorization; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; +import org.apache.ambari.server.topology.Blueprint; import org.apache.ambari.server.topology.ClusterTopology; import org.apache.ambari.server.topology.HostGroupInfo; import org.apache.ambari.server.topology.LogicalRequest; @@ -1200,7 +1200,7 @@ public class RequestResourceProviderTest { } } Assert.assertNotNull(propertyIdToAssert); - Assert.assertEquals("true", (String) propertyValueToAssert); + Assert.assertEquals("true", propertyValueToAssert); } @Test @@ -1624,6 +1624,12 @@ public class RequestResourceProviderTest { verify(managementController, actionManager, clusters, requestMock, requestDAO, hrcDAO); } + /** + * Tests that topology requests return different status (PENDING) if there are + * no tasks. Normal requests should return COMPLETED. + * + * @throws Exception + */ @Test @PrepareForTest(AmbariServer.class) public void testGetLogicalRequestStatusWithNoTasks() throws Exception {