Repository: aurora Updated Branches: refs/heads/master 07065b502 -> c385b63f4
Fix pendingTasks endpoint in case of multiple TaskGroups per job. Central idea of this patch is to change the return value of `getPendingReasons` from a map keyed by JobKey to a map keyed by `TaskGroupKey`. This prevents the `IllegalArgumentException` during the map construction. Bugs closed: AURORA-1879 Reviewed at https://reviews.apache.org/r/56058/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/c385b63f Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/c385b63f Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/c385b63f Branch: refs/heads/master Commit: c385b63f43b84519f7bee74178906bc76a8c8cfb Parents: 07065b5 Author: Stephan Erb <s...@apache.org> Authored: Mon Jan 30 21:37:59 2017 +0100 Committer: Stephan Erb <s...@apache.org> Committed: Mon Jan 30 21:37:59 2017 +0100 ---------------------------------------------------------------------- .../aurora/scheduler/http/PendingTasks.java | 24 ++++++++--------- .../aurora/scheduler/metadata/NearestFit.java | 7 ++--- .../aurora/scheduler/http/PendingTasksTest.java | 28 ++++++++++++++++++-- .../scheduler/metadata/NearestFitTest.java | 8 +++--- 4 files changed, 45 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/c385b63f/src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java b/src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java index 4571070..142acac 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java +++ b/src/main/java/org/apache/aurora/scheduler/http/PendingTasks.java @@ -14,7 +14,6 @@ package org.apache.aurora.scheduler.http; import java.io.IOException; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -26,10 +25,12 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import org.apache.aurora.scheduler.base.TaskGroupKey; import org.apache.aurora.scheduler.metadata.NearestFit; +import org.apache.aurora.scheduler.scheduling.TaskGroup; import org.apache.aurora.scheduler.scheduling.TaskGroups; -import org.codehaus.jackson.JsonNode; import org.codehaus.jackson.map.ObjectMapper; +import org.codehaus.jackson.node.ArrayNode; import org.codehaus.jackson.node.ObjectNode; /** @@ -55,21 +56,18 @@ public class PendingTasks { @GET @Produces(MediaType.APPLICATION_JSON) public Response getOffers() throws IOException { - // Adding reason, received from NearestFit#getPendingReasons() to the JSON Object. - Map<String, List<String>> taskGroupReasonMap = + Map<TaskGroupKey, List<String>> taskGroupReasonMap = nearestFit.getPendingReasons(taskGroups.getGroups()); - // Adding the attribute "reason" to each of the JSON Objects in the JsonNode. + ObjectMapper mapper = new ObjectMapper(); - JsonNode jsonNode = mapper.valueToTree(taskGroups.getGroups()); - Iterator<JsonNode> jsonNodeIterator = jsonNode.iterator(); + ArrayNode jsonNode = mapper.createArrayNode(); - while (jsonNodeIterator.hasNext()) { - JsonNode pendingTask = jsonNodeIterator.next(); + // Add the attribute "reason" to each serialized taskgroup + for (TaskGroup group : taskGroups.getGroups()) { + ObjectNode pendingTask = (ObjectNode) mapper.valueToTree(group); - // Retrieving the reasons corresponding to this pendingTask. - List<String> reasons = taskGroupReasonMap.get(pendingTask.get("name").asText()); - // Adding the reasons corresponding to the pendingTask. - ((ObjectNode) pendingTask).put("reason", reasons.toString()); + pendingTask.put("reason", taskGroupReasonMap.get(group.getKey()).toString()); + jsonNode.add(pendingTask); } return Response.ok(jsonNode).build(); } http://git-wip-us.apache.org/repos/asf/aurora/blob/c385b63f/src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java b/src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java index 1c015a2..44cac6f 100644 --- a/src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java +++ b/src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java @@ -128,13 +128,14 @@ public class NearestFit implements EventSubscriber { * Determine the pending reason, for each of the given tasks in taskGroups. * * @param taskGroups Group of pending tasks. - * @return A map with key=String (the taskgroup key) and value=List of reasons. + * @return A map with key=TaskGroupKey and value=List of reasons. */ - public synchronized Map<String, List<String>> getPendingReasons(Iterable<TaskGroup> taskGroups) { + public synchronized Map<TaskGroupKey, List<String>> getPendingReasons( + Iterable<TaskGroup> taskGroups) { return StreamSupport.stream(taskGroups.spliterator(), false).map(t -> { List<String> reasons = getNearestFit(t.getKey()).stream() .map(Veto::getReason).collect(Collectors.toList()); - return new HashMap.SimpleEntry<>(t.getKey().toString(), reasons); + return new HashMap.SimpleEntry<>(t.getKey(), reasons); }).collect(GuavaUtils.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); } http://git-wip-us.apache.org/repos/asf/aurora/blob/c385b63f/src/test/java/org/apache/aurora/scheduler/http/PendingTasksTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/http/PendingTasksTest.java b/src/test/java/org/apache/aurora/scheduler/http/PendingTasksTest.java index b71669f..96bdded 100644 --- a/src/test/java/org/apache/aurora/scheduler/http/PendingTasksTest.java +++ b/src/test/java/org/apache/aurora/scheduler/http/PendingTasksTest.java @@ -114,28 +114,38 @@ public class PendingTasksTest extends EasyMockTest { // Making pending tasks. IJobKey jobKey0 = IJobKey.build(new JobKey("role", "test", "jobA")); IJobKey jobKey1 = IJobKey.build(new JobKey("role", "test", "jobB")); + // Task of jobA IScheduledTask task0 = TestUtils.makeTask(jobKey0, "task0", 0, ScheduleStatus.PENDING, 1000, 1000000, 10); + // Tasks of jobB with two different TaskConfigs and thus two different TaskGroupKeys IScheduledTask task1 = TestUtils.makeTask(jobKey1, "task1", 0, ScheduleStatus.PENDING, 1000, 10, 1000000); + IScheduledTask task2 = TestUtils.makeTask(jobKey1, "task2", 1, + ScheduleStatus.PENDING, 100, 1, 100000); PubsubEvent.TaskStateChange taskStateChange0 = PubsubEvent.TaskStateChange.transition( task0, ScheduleStatus.INIT); PubsubEvent.TaskStateChange taskStateChange1 = PubsubEvent.TaskStateChange.transition( task1, ScheduleStatus.INIT); + PubsubEvent.TaskStateChange taskStateChange2 = PubsubEvent.TaskStateChange.transition( + task2, ScheduleStatus.INIT); pendingTaskGroups.taskChangedState(taskStateChange0); pendingTaskGroups.taskChangedState(taskStateChange1); + pendingTaskGroups.taskChangedState(taskStateChange2); expectLastCall(); // Recording the return value of pendingTaskGroups.getGroups(). TaskGroupKey taskGroupKey0 = TaskGroupKey.from(task0.getAssignedTask().getTask()); TaskGroupKey taskGroupKey1 = TaskGroupKey.from(task1.getAssignedTask().getTask()); + TaskGroupKey taskGroupKey2 = TaskGroupKey.from(task2.getAssignedTask().getTask()); TaskGroup taskGroup0 = new TaskGroup(taskGroupKey0, "task0"); TaskGroup taskGroup1 = new TaskGroup(taskGroupKey1, "task1"); + TaskGroup taskGroup2 = new TaskGroup(taskGroupKey2, "task2"); List<TaskGroup> taskGroupList = new ArrayList<>(); taskGroupList.add(taskGroup0); taskGroupList.add(taskGroup1); + taskGroupList.add(taskGroup2); expect(pendingTaskGroups.getGroups()).andReturn(taskGroupList).anyTimes(); // Creating vetoes for CPU and RAM, corresponding to task0. @@ -148,25 +158,39 @@ public class PendingTasksTest extends EasyMockTest { .add(Veto.insufficientResources("CPU", 1)) .add(Veto.insufficientResources("DISK", 1)).build(); nearestFit.vetoed(new PubsubEvent.Vetoed(taskGroupKey1, vetoes1)); + // Creating vetoes for CPU, corresponding to task2. + ImmutableSet<Veto> vetoes2 = ImmutableSet.<Veto>builder() + .add(Veto.insufficientResources("CPU", 1)).build(); + nearestFit.vetoed(new PubsubEvent.Vetoed(taskGroupKey2, vetoes2)); replay(pendingTaskGroups); // Testing. pendingTaskGroups.taskChangedState(taskStateChange0); pendingTaskGroups.taskChangedState(taskStateChange1); - PendingTasks pendingTasks0 = new PendingTasks(pendingTaskGroups, nearestFit); + pendingTaskGroups.taskChangedState(taskStateChange2); + + PendingTasks pendingTasks = new PendingTasks(pendingTaskGroups, nearestFit); String[] taskIds0 = {"task0"}; String[] taskIds1 = {"task1"}; + String[] taskIds2 = {"task2"}; String[] reasonsArr0 = {"Insufficient: CPU", "Insufficient: RAM"}; String[] reasonsArr1 = {"Insufficient: CPU", "Insufficient: DISK"}; + String[] reasonsArr2 = {"Insufficient: CPU"}; List<String> reasons0 = Arrays.stream(reasonsArr0).collect(Collectors.toList()); List<String> reasons1 = Arrays.stream(reasonsArr1).collect(Collectors.toList()); + List<String> reasons2 = Arrays.stream(reasonsArr2).collect(Collectors.toList()); + JsonNode mimicResponseTwoPendingTasksJson = new ObjectMapper().createArrayNode(); JsonNode mimicJson0 = getMimicResponseJson(0, taskIds0, "role/test/jobA", reasons0); JsonNode mimicJson1 = getMimicResponseJson(0, taskIds1, "role/test/jobB", reasons1); + JsonNode mimicJson2 = getMimicResponseJson(0, taskIds2, "role/test/jobB", reasons2); ((ArrayNode) mimicResponseTwoPendingTasksJson).add(mimicJson0); ((ArrayNode) mimicResponseTwoPendingTasksJson).add(mimicJson1); + ((ArrayNode) mimicResponseTwoPendingTasksJson).add(mimicJson2); + JsonNode actualResponseJson = new ObjectMapper().valueToTree( - pendingTasks0.getOffers().getEntity()); + pendingTasks.getOffers().getEntity()); + assertEquals(mimicResponseTwoPendingTasksJson, actualResponseJson); } } http://git-wip-us.apache.org/repos/asf/aurora/blob/c385b63f/src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java b/src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java index 195d083..f14d971 100644 --- a/src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java +++ b/src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java @@ -139,11 +139,11 @@ public class NearestFitTest { nearest.vetoed(new Vetoed(taskGroupKey, vetoes(SEVERITY_4_CPU, SEVERITY_4_RAM))); // Testing. - Map<String, List<String>> mimicPendingReasons = new LinkedHashMap<>(); + Map<TaskGroupKey, List<String>> mimicPendingReasons = new LinkedHashMap<>(); List<String> reasons = Arrays.stream( new String[]{SEVERITY_4_CPU.getReason(), SEVERITY_4_RAM.getReason()}) .collect(Collectors.toList()); - mimicPendingReasons.put("role/test/jobA", reasons); + mimicPendingReasons.put(taskGroupKey, reasons); assertPendingReasons(nearest.getPendingReasons(pendingTaskGroups), mimicPendingReasons); } @@ -159,8 +159,8 @@ public class NearestFitTest { assertEquals(vetoes(vetoes), nearest.getNearestFit(GROUP_KEY)); } - private void assertPendingReasons(Map<String, List<String>> actualPendingReasons, - Map<String, List<String>> mimicPendingReasons) { + private void assertPendingReasons(Map<TaskGroupKey, List<String>> actualPendingReasons, + Map<TaskGroupKey, List<String>> mimicPendingReasons) { assertEquals(actualPendingReasons, mimicPendingReasons); } }
