You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by nc...@apache.org on 2018/06/15 18:06:45 UTC
[ambari] branch branch-feature-AMBARI-14714 updated:
[AMBARI-24119]. TaskWrapper should only wrap a single task (#1554)
This is an automated email from the ASF dual-hosted git repository.
ncole pushed a commit to branch branch-feature-AMBARI-14714
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/branch-feature-AMBARI-14714 by this push:
new e50a72c [AMBARI-24119]. TaskWrapper should only wrap a single task (#1554)
e50a72c is described below
commit e50a72c83a01f3a3438598f19dbefde010dcf316
Author: ncole <nc...@hortonworks.com>
AuthorDate: Fri Jun 15 14:06:22 2018 -0400
[AMBARI-24119]. TaskWrapper should only wrap a single task (#1554)
---
.../internal/UpgradeResourceProvider.java | 34 +++++++--------
.../apache/ambari/server/state/UpgradeHelper.java | 23 +++++-----
.../state/stack/upgrade/ColocatedGrouping.java | 9 +---
.../server/state/stack/upgrade/Grouping.java | 40 ++++--------------
.../server/state/stack/upgrade/StageWrapper.java | 6 +--
.../server/state/stack/upgrade/TaskWrapper.java | 49 ++++++----------------
.../stack/upgrade/StageWrapperBuilderTest.java | 2 +-
7 files changed, 53 insertions(+), 110 deletions(-)
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 f01547e..99bbcac 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
@@ -786,23 +786,23 @@ public class UpgradeResourceProvider extends AbstractControllerResourceProvider
// is a bug that prevents one stage with multiple tasks assigned for
// the same host, break them out into individual stages.
for (TaskWrapper taskWrapper : wrapper.getTasks()) {
- for (Task task : taskWrapper.getTasks()) {
- if (upgradeContext.isManualVerificationAutoSkipped()
- && task.getType() == Task.Type.MANUAL) {
- continue;
- }
-
- UpgradeItemEntity itemEntity = new UpgradeItemEntity();
-
- itemEntity.setText(wrapper.getText());
- itemEntity.setTasks(wrapper.getTasksJson());
- itemEntity.setHosts(wrapper.getHostsJson());
-
- injectVariables(configHelper, cluster, itemEntity);
- if (makeServerSideStage(group, upgradeContext, null, req,
- itemEntity, (ServerSideActionTask) task, configUpgradePack)) {
- itemEntities.add(itemEntity);
- }
+ Task task = taskWrapper.getTask();
+
+ if (upgradeContext.isManualVerificationAutoSkipped()
+ && task.getType() == Task.Type.MANUAL) {
+ continue;
+ }
+
+ UpgradeItemEntity itemEntity = new UpgradeItemEntity();
+
+ itemEntity.setText(wrapper.getText());
+ itemEntity.setTasks(wrapper.getTasksJson());
+ itemEntity.setHosts(wrapper.getHostsJson());
+
+ injectVariables(configHelper, cluster, itemEntity);
+ if (makeServerSideStage(group, upgradeContext, null, req,
+ itemEntity, (ServerSideActionTask) task, configUpgradePack)) {
+ itemEntities.add(itemEntity);
}
}
} else {
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
index 89cadbb..cedab34 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
@@ -616,19 +616,18 @@ public class UpgradeHelper {
}
for (TaskWrapper taskWrapper : stageWrapper.getTasks()) {
- for (Task task : taskWrapper.getTasks()) {
- if (null != task.summary) {
- task.summary = tokenReplace(ctx, task.summary, null, null);
- }
+ Task task = taskWrapper.getTask();
+ if (null != task.summary) {
+ task.summary = tokenReplace(ctx, task.summary, null, null);
+ }
- if (task.getType() == Type.MANUAL) {
- ManualTask mt = (ManualTask) task;
- if(null != mt.messages && !mt.messages.isEmpty()){
- for(int i = 0; i < mt.messages.size(); i++){
- String message = mt.messages.get(i);
- message = tokenReplace(ctx, message, taskWrapper.getService(), taskWrapper.getComponent());
- mt.messages.set(i, message);
- }
+ if (task.getType() == Type.MANUAL) {
+ ManualTask mt = (ManualTask) task;
+ if(null != mt.messages && !mt.messages.isEmpty()){
+ for(int i = 0; i < mt.messages.size(); i++){
+ String message = mt.messages.get(i);
+ message = tokenReplace(ctx, message, taskWrapper.getService(), taskWrapper.getComponent());
+ mt.messages.set(i, message);
}
}
}
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ColocatedGrouping.java b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ColocatedGrouping.java
index 0bb6029..9e3f21a 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ColocatedGrouping.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/ColocatedGrouping.java
@@ -18,7 +18,6 @@
package org.apache.ambari.server.state.stack.upgrade;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -36,14 +35,12 @@ import org.apache.ambari.server.stack.HostsType;
import org.apache.ambari.server.state.UpgradeContext;
import org.apache.ambari.server.state.stack.UpgradePack.ProcessingComponent;
import org.apache.ambari.server.state.stack.upgrade.StageWrapper.Type;
-import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.ArrayUtils;
import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Predicate;
-import com.google.common.collect.Collections2;
import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
import com.google.gson.JsonPrimitive;
@@ -380,7 +377,7 @@ public class ColocatedGrouping extends Grouping {
public String toString() {
String s = "";
for (TaskWrapper t : tasks) {
- s += component + "/" + t.getTasks() + " ";
+ s += component + "/" + t.getTask() + " ";
}
return s;
@@ -400,9 +397,7 @@ public class ColocatedGrouping extends Grouping {
List<TaskWrapper> interim = new ArrayList<>();
for (TaskWrapper wrapper : tasks) {
- Collection<Task> filtered = Collections2.filter(wrapper.getTasks(), predicate);
-
- if (CollectionUtils.isNotEmpty(filtered)) {
+ if (predicate.apply(wrapper.getTask())) {
interim.add(wrapper);
}
}
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
index 29d6e77..d3112e5 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/Grouping.java
@@ -40,7 +40,7 @@ import org.apache.ambari.server.state.stack.UpgradePack.ProcessingComponent;
import org.apache.ambari.server.utils.SetUtils;
import org.apache.commons.lang.StringUtils;
-import com.google.common.base.Objects;
+import com.google.common.base.MoreObjects;
/**
*
@@ -156,7 +156,7 @@ public class Grouping {
// Add the processing component
Task t = resolveTask(context, pc);
if (null != t) {
- TaskWrapper tw = new TaskWrapper(service, pc.name, hostsType.getHosts(), params, Collections.singletonList(t));
+ TaskWrapper tw = new TaskWrapper(service, pc.name, hostsType.getHosts(), params, t);
addTasksToStageInBatches(Collections.singletonList(tw), t.getActionVerb(), context, service, pc, params);
}
@@ -188,7 +188,7 @@ public class Grouping {
List<TaskWrapper> subTasks = new ArrayList<>();
for (TaskWrapper tw : tasks) {
// If an of this TaskWrapper's tasks must be on its own stage, write out the previous subtasks if possible into one complete stage.
- if (tw.isAnyTaskSequential()) {
+ if (tw.isSequential()) {
if (!subTasks.isEmpty()) {
groupedTasks.add(subTasks);
subTasks = new ArrayList<>();
@@ -216,12 +216,12 @@ public class Grouping {
* @param params Params to add to the stage.
*/
private void addTasksToStageInBatches(List<TaskWrapper> tasks, String verb, UpgradeContext ctx, String service, ProcessingComponent pc, Map<String, String> params) {
- if (tasks == null || tasks.isEmpty() || tasks.get(0).getTasks() == null || tasks.get(0).getTasks().isEmpty()) {
+ if (tasks == null || tasks.isEmpty() || tasks.get(0).getTask() == null) {
return;
}
// Our assumption is that all of the tasks in the StageWrapper are of the same type.
- StageWrapper.Type type = tasks.get(0).getTasks().get(0).getStageWrapperType();
+ StageWrapper.Type type = tasks.get(0).getTask().getStageWrapperType();
// Expand some of the TaskWrappers into multiple based on the batch size.
for (TaskWrapper tw : tasks) {
@@ -249,7 +249,7 @@ public class Grouping {
type,
stageText,
params,
- new TaskWrapper(service, pc.name, hostSubset, params, tw.getTasks()));
+ new TaskWrapper(service, pc.name, hostSubset, params, tw.getTask()));
m_stages.add(stage);
}
}
@@ -322,34 +322,8 @@ public class Grouping {
}
private static class TaskBucket {
- private StageWrapper.Type type;
private List<Task> tasks = new ArrayList<>();
private TaskBucket(Task initial) {
- switch (initial.getType()) {
- case CONFIGURE:
- case SERVER_ACTION:
- case MANUAL:
- type = StageWrapper.Type.SERVER_SIDE_ACTION;
- break;
- case EXECUTE:
- type = StageWrapper.Type.UPGRADE_TASKS;
- break;
- case CONFIGURE_FUNCTION:
- type = StageWrapper.Type.CONFIGURE;
- break;
- case RESTART:
- type = StageWrapper.Type.RESTART;
- break;
- case START:
- type = StageWrapper.Type.START;
- break;
- case STOP:
- type = StageWrapper.Type.STOP;
- break;
- case SERVICE_CHECK:
- type = StageWrapper.Type.SERVICE_CHECK;
- break;
- }
tasks.add(initial);
}
}
@@ -418,7 +392,7 @@ public class Grouping {
*/
@Override
public String toString() {
- return Objects.toStringHelper(this)
+ return MoreObjects.toStringHelper(this)
.add("name", name)
.add("title", title)
.toString();
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapper.java b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapper.java
index 79147aa..6df3b63 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/StageWrapper.java
@@ -31,7 +31,7 @@ import org.apache.commons.lang.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.base.Objects;
+import com.google.common.base.MoreObjects;
import com.google.gson.Gson;
/**
@@ -95,7 +95,7 @@ public class StageWrapper {
public String getTasksJson() {
List<Task> realTasks = new ArrayList<>();
for (TaskWrapper tw : tasks) {
- realTasks.addAll(tw.getTasks());
+ realTasks.add(tw.getTask());
}
return gson.toJson(realTasks);
@@ -167,7 +167,7 @@ public class StageWrapper {
*/
@Override
public String toString() {
- return Objects.toStringHelper(this).add("type", type)
+ return MoreObjects.toStringHelper(this).add("type", type)
.add("text",text)
.omitNullValues().toString();
}
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TaskWrapper.java b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TaskWrapper.java
index 56047d7..9b2a15c 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TaskWrapper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/stack/upgrade/TaskWrapper.java
@@ -17,16 +17,14 @@
*/
package org.apache.ambari.server.state.stack.upgrade;
-import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.commons.lang.StringUtils;
-import com.google.common.base.Objects;
+import com.google.common.base.MoreObjects;
/**
* Aggregates all upgrade tasks for a HostComponent into one wrapper.
@@ -37,8 +35,7 @@ public class TaskWrapper {
private String component;
private Set<String> hosts; // all the hosts that all the tasks must run
private Map<String, String> params;
- /* FIXME a TaskWrapper really should be wrapping ONLY ONE task */
- private List<Task> tasks; // all the tasks defined for the hostcomponent
+ private Task task;
private Set<String> timeoutKeys = new HashSet<>();
/**
@@ -51,38 +48,22 @@ public class TaskWrapper {
this(s, c, hosts, null, task);
}
-
- /**
- * @param s the service name for the tasks
- * @param c the component name for the tasks
- * @param hosts the set of hosts that the tasks are for
- * @param params additional command parameters
- * @param tasks an array of tasks as a convenience
- */
- public TaskWrapper(String s, String c, Set<String> hosts, Map<String, String> params, Task... tasks) {
- this(s, c, hosts, params, Arrays.asList(tasks));
- }
-
-
/**
* @param s the service name for the tasks
* @param c the component name for the tasks
* @param hosts the set of hosts for the
* @param tasks the list of tasks
*/
- public TaskWrapper(String s, String c, Set<String> hosts, Map<String, String> params, List<Task> tasks) {
+ public TaskWrapper(String s, String c, Set<String> hosts, Map<String, String> params, Task task) {
service = s;
component = c;
this.hosts = hosts;
this.params = (params == null) ? new HashMap<>() : params;
- this.tasks = tasks;
+ this.task = task;
- // !!! FIXME there should only be one task
- for (Task task : tasks) {
- if (StringUtils.isNotBlank(task.timeoutConfig)) {
- timeoutKeys.add(task.timeoutConfig);
- }
+ if (StringUtils.isNotBlank(task.timeoutConfig)) {
+ timeoutKeys.add(task.timeoutConfig);
}
}
@@ -96,8 +77,8 @@ public class TaskWrapper {
/**
* @return the tasks associated with this wrapper
*/
- public List<Task> getTasks() {
- return tasks;
+ public Task getTask() {
+ return task;
}
/**
@@ -112,9 +93,9 @@ public class TaskWrapper {
*/
@Override
public String toString() {
- return Objects.toStringHelper(this).add("service", service)
+ return MoreObjects.toStringHelper(this).add("service", service)
.add("component", component)
- .add("tasks", tasks)
+ .add("task", task)
.add("hosts", hosts)
.omitNullValues().toString();
}
@@ -136,14 +117,8 @@ public class TaskWrapper {
/**
* @return true if any task is sequential, otherwise, return false.
*/
- public boolean isAnyTaskSequential() {
- for (Task t : getTasks()) {
- if (t.isSequential) {
- return true;
- }
- }
-
- return false;
+ public boolean isSequential() {
+ return getTask().isSequential;
}
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilderTest.java
index a9a6de4..5872281 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/state/stack/upgrade/StageWrapperBuilderTest.java
@@ -114,7 +114,7 @@ public class StageWrapperBuilderTest extends EasyMockSupport {
StageWrapper skipSummaryWrapper = stageWrappers.get(1);
Assert.assertEquals(StageWrapper.Type.SERVER_SIDE_ACTION, skipSummaryWrapper.getType());
- ServerActionTask task = (ServerActionTask)(skipSummaryWrapper.getTasks().get(0).getTasks().get(0));
+ ServerActionTask task = (ServerActionTask)(skipSummaryWrapper.getTasks().get(0).getTask());
Assert.assertEquals(AutoSkipFailedSummaryAction.class.getName(), task.implClass);
Assert.assertEquals(1, task.messages.size());
Assert.assertTrue(task.messages.get(0).contains("There are failures that were automatically skipped"));
--
To stop receiving notification emails like this one, please contact
ncole@apache.org.