Repository: aurora Updated Branches: refs/heads/master ae8a9f726 -> b6e2bc34f
Suppressing "Unregistered executor" status message. Bugs closed: AURORA-1193 Reviewed at https://reviews.apache.org/r/37483/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/b6e2bc34 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/b6e2bc34 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/b6e2bc34 Branch: refs/heads/master Commit: b6e2bc34fe0785270700cf39e6d45af86b414c5a Parents: ae8a9f7 Author: Maxim Khutornenko <[email protected]> Authored: Fri Aug 14 13:18:14 2015 -0700 Committer: Maxim Khutornenko <[email protected]> Committed: Fri Aug 14 13:18:14 2015 -0700 ---------------------------------------------------------------------- .../aurora/scheduler/TaskStatusHandlerImpl.java | 39 +++++++--- .../scheduler/TaskStatusHandlerImplTest.java | 78 +++++++++++++++----- 2 files changed, 86 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/b6e2bc34/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java b/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java index 24a7a2c..423765f 100644 --- a/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java +++ b/src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java @@ -153,22 +153,12 @@ public class TaskStatusHandlerImpl extends AbstractExecutionThreadService for (TaskStatus status : updates) { ScheduleStatus translatedState = Conversions.convertProtoState(status.getState()); - Optional<String> message = Optional.absent(); - if (status.hasMessage()) { - message = Optional.of(status.getMessage()); - } - - if (translatedState == ScheduleStatus.FAILED && status.hasReason() - && status.getReason() == TaskStatus.Reason.REASON_MEMORY_LIMIT) { - message = Optional.of(MEMORY_LIMIT_DISPLAY); - } - StateChangeResult result = stateManager.changeState( storeProvider, status.getTaskId().getValue(), Optional.absent(), translatedState, - message); + formatMessage(status)); if (status.hasReason()) { counters.get(statName(status, result)).incrementAndGet(); @@ -190,4 +180,31 @@ public class TaskStatusHandlerImpl extends AbstractExecutionThreadService static String statName(TaskStatus status, StateChangeResult result) { return String.format(STATUS_STAT_FORMAT, status.getReason(), result); } + + private static Optional<String> formatMessage(TaskStatus status) { + Optional<String> message = Optional.absent(); + if (status.hasMessage()) { + message = Optional.of(status.getMessage()); + } + + if (status.hasReason()) { + switch (status.getReason()) { + case REASON_MEMORY_LIMIT: + // Add a failure explanation to the user + message = Optional.of(MEMORY_LIMIT_DISPLAY); + break; + + case REASON_EXECUTOR_UNREGISTERED: + // Suppress "Unregistered executor" message as it bears no meaning to the user. + message = Optional.absent(); + break; + + default: + // Message is already populated above. + break; + } + } + + return message; + } } http://git-wip-us.apache.org/repos/asf/aurora/blob/b6e2bc34/src/test/java/org/apache/aurora/scheduler/TaskStatusHandlerImplTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/TaskStatusHandlerImplTest.java b/src/test/java/org/apache/aurora/scheduler/TaskStatusHandlerImplTest.java index 83dcb4b..937cadc 100644 --- a/src/test/java/org/apache/aurora/scheduler/TaskStatusHandlerImplTest.java +++ b/src/test/java/org/apache/aurora/scheduler/TaskStatusHandlerImplTest.java @@ -32,11 +32,13 @@ import org.apache.mesos.Protos.TaskID; import org.apache.mesos.Protos.TaskState; import org.apache.mesos.Protos.TaskStatus; import org.easymock.EasyMock; +import org.easymock.IAnswer; import org.junit.After; import org.junit.Before; import org.junit.Test; import static org.apache.aurora.gen.ScheduleStatus.FAILED; +import static org.apache.aurora.gen.ScheduleStatus.KILLED; import static org.apache.aurora.gen.ScheduleStatus.RUNNING; import static org.apache.aurora.scheduler.TaskStatusHandlerImpl.statName; import static org.easymock.EasyMock.expect; @@ -102,10 +104,7 @@ public class TaskStatusHandlerImplTest extends EasyMockTest { final CountDownLatch latch = new CountDownLatch(1); driver.acknowledgeStatusUpdate(status); - expectLastCall().andAnswer(() -> { - latch.countDown(); - return null; - }); + waitAndAnswer(latch); control.replay(); @@ -126,10 +125,13 @@ public class TaskStatusHandlerImplTest extends EasyMockTest { Optional.absent(), RUNNING, Optional.of("fake message"))) - .andAnswer(() -> { + .andAnswer(new IAnswer<StateChangeResult>() { + @Override + public StateChangeResult answer() throws Throwable { latch.countDown(); throw new StorageException("Injected error"); - }); + } + }); control.replay(); @@ -166,10 +168,38 @@ public class TaskStatusHandlerImplTest extends EasyMockTest { final CountDownLatch latch = new CountDownLatch(1); driver.acknowledgeStatusUpdate(status); - expectLastCall().andAnswer(() -> { - latch.countDown(); - return null; - }); + waitAndAnswer(latch); + + control.replay(); + + statusHandler.statusUpdate(status); + + assertTrue(latch.await(5L, TimeUnit.SECONDS)); + } + + @Test + public void testSuppressUnregisteredExecutorMessage() throws Exception { + storageUtil.expectWrite(); + + TaskStatus status = TaskStatus.newBuilder() + .setState(TaskState.TASK_KILLED) + .setTaskId(TaskID.newBuilder().setValue(TASK_ID_A)) + .setReason(TaskStatus.Reason.REASON_EXECUTOR_UNREGISTERED) + .setMessage("Unregistered executor") + .build(); + + expect(stateManager.changeState( + storageUtil.mutableStoreProvider, + TASK_ID_A, + Optional.absent(), + KILLED, + Optional.absent())) + .andReturn(StateChangeResult.SUCCESS); + + final CountDownLatch latch = new CountDownLatch(1); + + driver.acknowledgeStatusUpdate(status); + waitAndAnswer(latch); control.replay(); @@ -197,21 +227,19 @@ public class TaskStatusHandlerImplTest extends EasyMockTest { 1000, new CachedCounters(stats)); - expect(queue.add(EasyMock.anyObject())) - .andReturn(true); + expect(queue.add(EasyMock.anyObject())).andReturn(true); - expect(queue.take()) - .andAnswer(() -> { - throw new RuntimeException(); - }); + expect(queue.take()).andAnswer(new IAnswer<TaskStatus>() { + @Override + public TaskStatus answer() throws Throwable { + throw new RuntimeException(); + } + }); final CountDownLatch latch = new CountDownLatch(1); driver.abort(); - expectLastCall().andAnswer(() -> { - latch.countDown(); - return null; - }); + waitAndAnswer(latch); control.replay(); @@ -227,4 +255,14 @@ public class TaskStatusHandlerImplTest extends EasyMockTest { assertTrue(latch.await(5L, TimeUnit.SECONDS)); } + + private static void waitAndAnswer(CountDownLatch latch) { + expectLastCall().andAnswer(new IAnswer<StateChangeResult>() { + @Override + public StateChangeResult answer() { + latch.countDown(); + return null; + } + }); + } }
