You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by al...@apache.org on 2016/03/29 01:18:52 UTC
ambari git commit: 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)
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 <af...@hortonworks.com>
Authored: Wed Mar 23 18:34:12 2016 -0700
Committer: Alejandro Fernandez <af...@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);