Repository: ambari Updated Branches: refs/heads/trunk be444cc14 -> 54cbe9e7d
AMBARI-15569. Ambari calculates stack downgrade as ABORTED under incorrect conditions. UI shows 'Downgrade paused' and button to resume downgrade even when progress is happening (alejandro) Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/54cbe9e7 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/54cbe9e7 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/54cbe9e7 Branch: refs/heads/trunk Commit: 54cbe9e7d1d0c057bb7c6dd099dda6ad9ef13c5b Parents: be444cc Author: Alejandro Fernandez <afernan...@hortonworks.com> Authored: Wed Mar 23 18:34:12 2016 -0700 Committer: Alejandro Fernandez <afernan...@hortonworks.com> Committed: Mon Mar 28 16:12:02 2016 -0700 ---------------------------------------------------------------------- .../controller/internal/CalculatedStatus.java | 73 ++++++++++++++------ .../internal/UpgradeResourceProvider.java | 24 ++++++- .../server/orm/entities/UpgradeEntity.java | 7 ++ .../internal/CalculatedStatusTest.java | 28 ++++++++ .../internal/RequestResourceProviderTest.java | 2 +- .../internal/UpgradeResourceProviderTest.java | 3 +- 6 files changed, 111 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/54cbe9e7/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 1d8df9b..985e9ad 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 @@ -137,7 +137,7 @@ public class CalculatedStatus { Map<HostRoleStatus, Integer> taskStatusCounts = CalculatedStatus.calculateTaskEntityStatusCounts(tasks); - HostRoleStatus status = calculateSummaryStatus(taskStatusCounts, size, skippable); + HostRoleStatus status = calculateSummaryStatusOfStage(taskStatusCounts, size, skippable); double progressPercent = calculateProgressPercent(taskStatusCounts, size); @@ -153,7 +153,6 @@ public class CalculatedStatus { * @return a calculated status */ public static CalculatedStatus statusFromStageEntities(Collection<StageEntity> stages) { - Collection<HostRoleStatus> stageStatuses = new HashSet<HostRoleStatus>(); Collection<HostRoleCommandEntity> tasks = new HashSet<HostRoleCommandEntity>(); @@ -163,7 +162,7 @@ public class CalculatedStatus { // calculate the stage status from the task status counts HostRoleStatus stageStatus = - calculateSummaryStatus(calculateTaskEntityStatusCounts(stageTasks), stageTasks.size(), stage.isSkippable()); + calculateSummaryStatusOfStage(calculateTaskEntityStatusCounts(stageTasks), stageTasks.size(), stage.isSkippable()); stageStatuses.add(stageStatus); @@ -172,7 +171,7 @@ public class CalculatedStatus { } // calculate the overall status from the stage statuses - HostRoleStatus status = calculateSummaryStatus(calculateStatusCounts(stageStatuses), stageStatuses.size(), false); + HostRoleStatus status = calculateSummaryStatusOfUpgrade(calculateStatusCounts(stageStatuses), stageStatuses.size()); // calculate the progress from the task status counts double progressPercent = calculateProgressPercent(calculateTaskEntityStatusCounts(tasks), tasks.size()); @@ -199,7 +198,7 @@ public class CalculatedStatus { // calculate the stage status from the task status counts HostRoleStatus stageStatus = - calculateSummaryStatus(calculateTaskStatusCounts(stageTasks), stageTasks.size(), stage.isSkippable()); + calculateSummaryStatusOfStage(calculateTaskStatusCounts(stageTasks), stageTasks.size(), stage.isSkippable()); stageStatuses.add(stageStatus); @@ -208,7 +207,7 @@ public class CalculatedStatus { } // calculate the overall status from the stage statuses - HostRoleStatus status = calculateSummaryStatus(calculateStatusCounts(stageStatuses), stageStatuses.size(), false); + HostRoleStatus status = calculateSummaryStatusOfUpgrade(calculateStatusCounts(stageStatuses), stageStatuses.size()); // calculate the progress from the task status counts double progressPercent = calculateProgressPercent(calculateTaskStatusCounts(tasks), tasks.size()); @@ -292,7 +291,7 @@ public class CalculatedStatus { } /** - * Calculates the overall status + * 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 * @return the calculated status @@ -304,7 +303,6 @@ public class CalculatedStatus { Collection<HostRoleStatus> stageDisplayStatuses = new HashSet<>(); Collection<HostRoleStatus> taskStatuses = new ArrayList<>(); - for (Long stageId : stageIds) { if (!stageDto.containsKey(stageId)) { continue; @@ -315,7 +313,7 @@ public class CalculatedStatus { int total = summary.getTaskTotal(); boolean skip = summary.isStageSkippable(); Map<HostRoleStatus, Integer> counts = calculateStatusCounts(summary.getTaskStatuses()); - HostRoleStatus stageStatus = calculateSummaryStatus(counts, total, skip); + HostRoleStatus stageStatus = calculateSummaryStatusOfStage(counts, total, skip); HostRoleStatus stageDisplayStatus = calculateSummaryDisplayStatus(counts, total, skip); stageStatuses.add(stageStatus); @@ -327,7 +325,7 @@ public class CalculatedStatus { Map<HostRoleStatus, Integer> counts = calculateStatusCounts(stageStatuses); Map<HostRoleStatus, Integer> displayCounts = calculateStatusCounts(stageDisplayStatuses); - HostRoleStatus status = calculateSummaryStatus(counts, stageStatuses.size(), false); + HostRoleStatus status = calculateSummaryStatusOfUpgrade(counts, stageStatuses.size()); HostRoleStatus displayStatus = calculateSummaryDisplayStatus(displayCounts, stageDisplayStatuses.size(), false); double progressPercent = calculateProgressPercent(calculateStatusCounts(taskStatuses), taskStatuses.size()); @@ -370,24 +368,55 @@ public class CalculatedStatus { } /** - * Calculate an overall status based on the given status counts. + * Calculate overall status of a stage or upgrade based on the given status counts. * * @param counters counts of resources that are in various states - * @param total total number of resources in request + * @param total total number of resources in request. NOTE, completed tasks may be counted twice. * @param skippable true if a single failed status should NOT result in an overall failed status return * * @return summary request status based on statuses of tasks in different states. */ - private static HostRoleStatus calculateSummaryStatus(Map<HostRoleStatus, Integer> counters, + private static HostRoleStatus calculateSummaryStatusOfStage(Map<HostRoleStatus, Integer> counters, int total, boolean skippable) { - return counters.get(HostRoleStatus.PENDING) == total ? HostRoleStatus.PENDING : - counters.get(HostRoleStatus.HOLDING) > 0 ? HostRoleStatus.HOLDING : - counters.get(HostRoleStatus.HOLDING_FAILED) > 0 ? HostRoleStatus.HOLDING_FAILED : - counters.get(HostRoleStatus.HOLDING_TIMEDOUT) > 0 ? HostRoleStatus.HOLDING_TIMEDOUT : - counters.get(HostRoleStatus.FAILED) > 0 && !skippable ? HostRoleStatus.FAILED : - counters.get(HostRoleStatus.ABORTED) > 0 ? HostRoleStatus.ABORTED: - counters.get(HostRoleStatus.TIMEDOUT) > 0 && !skippable ? HostRoleStatus.TIMEDOUT : - counters.get(HostRoleStatus.COMPLETED) == total ? HostRoleStatus.COMPLETED : HostRoleStatus.IN_PROGRESS; + 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; + } + + int numActiveTasks = counters.get(HostRoleStatus.PENDING) + counters.get(HostRoleStatus.QUEUED) + counters.get(HostRoleStatus.IN_PROGRESS); + // ABORTED only if there are no other active tasks + if (counters.get(HostRoleStatus.ABORTED) > 0 && numActiveTasks == 0) { + return HostRoleStatus.ABORTED; + } + if (counters.get(HostRoleStatus.COMPLETED) == total) { + return HostRoleStatus.COMPLETED; + } + return HostRoleStatus.IN_PROGRESS; + } + + /** + * Calculate overall status of an upgrade. + * + * @param counters counts of resources that are in various states + * @param total total number of resources in request + * + * @return summary request status based on statuses of tasks in different states. + */ + private static HostRoleStatus calculateSummaryStatusOfUpgrade(Map<HostRoleStatus, Integer> counters, int total) { + return calculateSummaryStatusOfStage(counters, total, false); } /** @@ -403,6 +432,6 @@ public class CalculatedStatus { int total, boolean skippable) { return counters.get(HostRoleStatus.SKIPPED_FAILED) > 0 ? HostRoleStatus.SKIPPED_FAILED : counters.get(HostRoleStatus.FAILED) > 0 ? HostRoleStatus.FAILED: - calculateSummaryStatus(counters, total, skippable); + calculateSummaryStatusOfStage(counters, total, skippable); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/54cbe9e7/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java index d43daca..e69bc90 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java @@ -464,6 +464,17 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider status, UPGRADE_SUSPENDED )); } else { suspended = Boolean.valueOf((String) propertyMap.get(UPGRADE_SUSPENDED)); + // If either the status is ABORTED or request to be suspended, then both should be set. + if ((status == HostRoleStatus.ABORTED || suspended)) { + if (status == HostRoleStatus.ABORTED && !suspended) { + throw new IllegalArgumentException(String.format( + "If the status is set to ABORTED, must also set property %s to true", UPGRADE_SUSPENDED)); + } + if (status != HostRoleStatus.ABORTED && suspended) { + throw new IllegalArgumentException(String.format( + "If property %s is set to true, must also set the status to ABORTED", UPGRADE_SUSPENDED)); + } + } } setUpgradeRequestStatus(requestIdProperty, status, propertyMap); @@ -495,7 +506,7 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider upgradeEntity.setAutoSkipComponentFailures(skipFailures); upgradeEntity.setAutoSkipServiceCheckFailures(skipServiceCheckFailures); - upgradeEntity = s_upgradeDAO.merge(upgradeEntity); + s_upgradeDAO.merge(upgradeEntity); } @@ -523,6 +534,7 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider setResourceProperty(resource, UPGRADE_FROM_VERSION, entity.getFromVersion(), requestedIds); setResourceProperty(resource, UPGRADE_TO_VERSION, entity.getToVersion(), requestedIds); setResourceProperty(resource, UPGRADE_DIRECTION, entity.getDirection(), requestedIds); + setResourceProperty(resource, UPGRADE_SUSPENDED, entity.getSuspended(), requestedIds); setResourceProperty(resource, UPGRADE_DOWNGRADE_ALLOWED, entity.isDowngradeAllowed(), requestedIds); setResourceProperty(resource, UPGRADE_SKIP_FAILURES, entity.isComponentFailureAutoSkipped(), requestedIds); setResourceProperty(resource, UPGRADE_SKIP_SC_FAILURES, entity.isServiceCheckFailureAutoSkipped(), requestedIds); @@ -1642,7 +1654,7 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider HostRoleStatus internalStatus = CalculatedStatus.statusFromStages( internalRequest.getStages()).getStatus(); - if (HostRoleStatus.PENDING == status && internalStatus != HostRoleStatus.ABORTED) { + if (HostRoleStatus.PENDING == status && !(internalStatus == HostRoleStatus.ABORTED || internalStatus == HostRoleStatus.IN_PROGRESS)) { throw new IllegalArgumentException( String.format("Can only set status to %s when the upgrade is %s (currently %s)", status, HostRoleStatus.ABORTED, internalStatus)); @@ -1655,12 +1667,17 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider // Remove relevant upgrade entity try { Cluster cluster = clusters.get().getClusterById(clusterId); + UpgradeEntity lastUpgradeItemForCluster = s_upgradeDAO.findLastUpgradeForCluster(cluster.getClusterId()); + lastUpgradeItemForCluster.setSuspended(true); + s_upgradeDAO.merge(lastUpgradeItemForCluster); + cluster.setUpgradeEntity(null); } catch (AmbariException e) { LOG.warn("Could not clear upgrade entity for cluster with id {}", clusterId, e); } } } else { + // Status must be PENDING. List<Long> taskIds = new ArrayList<Long>(); for (HostRoleCommand hrc : internalRequest.getCommands()) { if (HostRoleStatus.ABORTED == hrc.getStatus() @@ -1674,6 +1691,9 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider try { Cluster cluster = clusters.get().getClusterById(clusterId); UpgradeEntity lastUpgradeItemForCluster = s_upgradeDAO.findLastUpgradeForCluster(cluster.getClusterId()); + lastUpgradeItemForCluster.setSuspended(false); + s_upgradeDAO.merge(lastUpgradeItemForCluster); + cluster.setUpgradeEntity(lastUpgradeItemForCluster); } catch (AmbariException e) { LOG.warn("Could not clear upgrade entity for cluster with id {}", clusterId, e); http://git-wip-us.apache.org/repos/asf/ambari/blob/54cbe9e7/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java index fd866a1..b47860f 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java @@ -198,6 +198,13 @@ public class UpgradeEntity { } /** + * @return if the upgrade is suspended + */ + public boolean getSuspended() { + return suspended == 1; + } + + /** * @param direction the direction of the upgrade */ public void setDirection(Direction direction) { http://git-wip-us.apache.org/repos/asf/ambari/blob/54cbe9e7/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 8d80fa6..46c0b03 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 @@ -441,6 +441,34 @@ public class CalculatedStatusTest { assertEquals(HostRoleStatus.HOLDING, status.getStatus()); assertNull(status.getDisplayStatus()); assertEquals(47.5, status.getPercent(), 0.1); + + // aborted + stages = getStages( + getTaskEntities(HostRoleStatus.COMPLETED, HostRoleStatus.COMPLETED, HostRoleStatus.COMPLETED), + getTaskEntities(HostRoleStatus.COMPLETED, HostRoleStatus.COMPLETED, HostRoleStatus.ABORTED), + getTaskEntities(HostRoleStatus.ABORTED, HostRoleStatus.ABORTED, HostRoleStatus.ABORTED), + getTaskEntities(HostRoleStatus.ABORTED, HostRoleStatus.ABORTED, HostRoleStatus.ABORTED) + ); + + status = CalculatedStatus.statusFromStages(stages); + + assertEquals(HostRoleStatus.ABORTED, status.getStatus()); + assertNull(status.getDisplayStatus()); + assertEquals(100.0, status.getPercent(), 0.1); + + // in-progress even though there are aborted tasks in the middle + stages = getStages( + getTaskEntities(HostRoleStatus.COMPLETED, HostRoleStatus.COMPLETED, HostRoleStatus.COMPLETED), + getTaskEntities(HostRoleStatus.COMPLETED, HostRoleStatus.COMPLETED, HostRoleStatus.COMPLETED), + getTaskEntities(HostRoleStatus.ABORTED, HostRoleStatus.ABORTED, HostRoleStatus.PENDING), + getTaskEntities(HostRoleStatus.PENDING, HostRoleStatus.PENDING, HostRoleStatus.PENDING) + ); + + status = CalculatedStatus.statusFromStages(stages); + + assertEquals(HostRoleStatus.IN_PROGRESS, status.getStatus()); + assertNull(status.getDisplayStatus()); + assertEquals(66.6, status.getPercent(), 0.1); } @Test http://git-wip-us.apache.org/repos/asf/ambari/blob/54cbe9e7/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 c18a8b6..22921fb 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 @@ -734,7 +734,7 @@ public class RequestResourceProviderTest { Assert.assertEquals(0, resource.getPropertyValue(RequestResourceProvider.REQUEST_ABORTED_TASK_CNT_ID)); Assert.assertEquals(0, resource.getPropertyValue(RequestResourceProvider.REQUEST_TIMED_OUT_TASK_CNT_ID)); } else { - Assert.assertEquals("ABORTED", resource.getPropertyValue(RequestResourceProvider.REQUEST_STATUS_PROPERTY_ID)); + Assert.assertEquals("TIMEDOUT", resource.getPropertyValue(RequestResourceProvider.REQUEST_STATUS_PROPERTY_ID)); Assert.assertEquals(0, resource.getPropertyValue(RequestResourceProvider.REQUEST_FAILED_TASK_CNT_ID)); Assert.assertEquals(1, resource.getPropertyValue(RequestResourceProvider.REQUEST_ABORTED_TASK_CNT_ID)); Assert.assertEquals(1, resource.getPropertyValue(RequestResourceProvider.REQUEST_TIMED_OUT_TASK_CNT_ID)); http://git-wip-us.apache.org/repos/asf/ambari/blob/54cbe9e7/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java index d37f3ba..64a8852 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java @@ -785,7 +785,7 @@ public class UpgradeResourceProviderTest { Map<String, Object> requestProps = new HashMap<String, Object>(); requestProps.put(UpgradeResourceProvider.UPGRADE_REQUEST_ID, id.toString()); requestProps.put(UpgradeResourceProvider.UPGRADE_REQUEST_STATUS, "ABORTED"); - requestProps.put(UpgradeResourceProvider.UPGRADE_SUSPENDED, "false"); + requestProps.put(UpgradeResourceProvider.UPGRADE_SUSPENDED, "true"); UpgradeResourceProvider urp = createProvider(amc); @@ -841,6 +841,7 @@ public class UpgradeResourceProviderTest { requestProps = new HashMap<String, Object>(); requestProps.put(UpgradeResourceProvider.UPGRADE_REQUEST_ID, id.toString()); requestProps.put(UpgradeResourceProvider.UPGRADE_REQUEST_STATUS, "PENDING"); + requestProps.put(UpgradeResourceProvider.UPGRADE_SUSPENDED, "false"); // !!! make sure we can. actual reset is tested elsewhere req = PropertyHelper.getUpdateRequest(requestProps, null);